batman-adv: Merge bugfixes from 2021.4 752/head
authorSven Eckelmann <sven@narfation.org>
Fri, 19 Nov 2021 16:05:03 +0000 (17:05 +0100)
committerSven Eckelmann <sven@narfation.org>
Fri, 19 Nov 2021 16:20:13 +0000 (17:20 +0100)
* fix error handling during interface initialization

Signed-off-by: Sven Eckelmann <sven@narfation.org>
batman-adv/patches/0036-batman-adv-fix-error-handling.patch [new file with mode: 0644]

diff --git a/batman-adv/patches/0036-batman-adv-fix-error-handling.patch b/batman-adv/patches/0036-batman-adv-fix-error-handling.patch
new file mode 100644 (file)
index 0000000..0daaae4
--- /dev/null
@@ -0,0 +1,162 @@
+From: Pavel Skripkin <paskripkin@gmail.com>
+Date: Sun, 24 Oct 2021 16:13:56 +0300
+Subject: batman-adv: fix error handling
+
+Syzbot reported ODEBUG warning in batadv_nc_mesh_free(). The problem was
+in wrong error handling in batadv_mesh_init().
+
+Before this patch batadv_mesh_init() was calling batadv_mesh_free() in case
+of any batadv_*_init() calls failure. This approach may work well, when
+there is some kind of indicator, which can tell which parts of batadv are
+initialized; but there isn't any.
+
+All written above lead to cleaning up uninitialized fields. Even if we hide
+ODEBUG warning by initializing bat_priv->nc.work, syzbot was able to hit
+GPF in batadv_nc_purge_paths(), because hash pointer in still NULL. [1]
+
+To fix these bugs we can unwind batadv_*_init() calls one by one.
+It is good approach for 2 reasons: 1) It fixes bugs on error handling
+path 2) It improves the performance, since we won't call unneeded
+batadv_*_free() functions.
+
+So, this patch makes all batadv_*_init() clean up all allocated memory
+before returning with an error to no call correspoing batadv_*_free()
+and open-codes batadv_mesh_free() with proper order to avoid touching
+uninitialized fields.
+
+Link: https://lore.kernel.org/netdev/000000000000c87fbd05cef6bcb0@google.com/ [1]
+Reported-and-tested-by: syzbot+28b0702ada0bf7381f58@syzkaller.appspotmail.com
+Fixes: 21e838760727 ("[batman-adv] fix various race conditions during startup & shutdown")
+Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/0631e0825c8129cd3896926da62a09ac00bf13a0
+
+--- a/net/batman-adv/bridge_loop_avoidance.c
++++ b/net/batman-adv/bridge_loop_avoidance.c
+@@ -1561,10 +1561,14 @@ int batadv_bla_init(struct batadv_priv *
+               return 0;
+       bat_priv->bla.claim_hash = batadv_hash_new(128);
+-      bat_priv->bla.backbone_hash = batadv_hash_new(32);
++      if (!bat_priv->bla.claim_hash)
++              return -ENOMEM;
+-      if (!bat_priv->bla.claim_hash || !bat_priv->bla.backbone_hash)
++      bat_priv->bla.backbone_hash = batadv_hash_new(32);
++      if (!bat_priv->bla.backbone_hash) {
++              batadv_hash_destroy(bat_priv->bla.claim_hash);
+               return -ENOMEM;
++      }
+       batadv_hash_set_lock_class(bat_priv->bla.claim_hash,
+                                  &batadv_claim_hash_lock_class_key);
+--- a/net/batman-adv/main.c
++++ b/net/batman-adv/main.c
+@@ -197,29 +197,41 @@ int batadv_mesh_init(struct net_device *
+       bat_priv->gw.generation = 0;
+-      ret = batadv_v_mesh_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
+-
+       ret = batadv_originator_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_orig;
++      }
+       ret = batadv_tt_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_tt;
++      }
++
++      ret = batadv_v_mesh_init(bat_priv);
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_v;
++      }
+       ret = batadv_bla_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_bla;
++      }
+       ret = batadv_dat_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_dat;
++      }
+       ret = batadv_nc_mesh_init(bat_priv);
+-      if (ret < 0)
+-              goto err;
++      if (ret < 0) {
++              atomic_set(&bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
++              goto err_nc;
++      }
+       batadv_gw_init(bat_priv);
+       batadv_mcast_init(bat_priv);
+@@ -229,8 +241,20 @@ int batadv_mesh_init(struct net_device *
+       return 0;
+-err:
+-      batadv_mesh_free(soft_iface);
++err_nc:
++      batadv_dat_free(bat_priv);
++err_dat:
++      batadv_bla_free(bat_priv);
++err_bla:
++      batadv_v_mesh_free(bat_priv);
++err_v:
++      batadv_tt_free(bat_priv);
++err_tt:
++      batadv_originator_free(bat_priv);
++err_orig:
++      batadv_purge_outstanding_packets(bat_priv, NULL);
++      atomic_set(&bat_priv->mesh_state, BATADV_MESH_INACTIVE);
++
+       return ret;
+ }
+--- a/net/batman-adv/network-coding.c
++++ b/net/batman-adv/network-coding.c
+@@ -155,8 +155,10 @@ int batadv_nc_mesh_init(struct batadv_pr
+                                  &batadv_nc_coding_hash_lock_class_key);
+       bat_priv->nc.decoding_hash = batadv_hash_new(128);
+-      if (!bat_priv->nc.decoding_hash)
++      if (!bat_priv->nc.decoding_hash) {
++              batadv_hash_destroy(bat_priv->nc.coding_hash);
+               goto err;
++      }
+       batadv_hash_set_lock_class(bat_priv->nc.decoding_hash,
+                                  &batadv_nc_decoding_hash_lock_class_key);
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -4405,8 +4405,10 @@ int batadv_tt_init(struct batadv_priv *b
+               return ret;
+       ret = batadv_tt_global_init(bat_priv);
+-      if (ret < 0)
++      if (ret < 0) {
++              batadv_tt_local_table_free(bat_priv);
+               return ret;
++      }
+       batadv_tvlv_handler_register(bat_priv, batadv_tt_tvlv_ogm_handler_v1,
+                                    batadv_tt_tvlv_unicast_handler_v1,