net: batman-adv: fix error handling

commit 6f68cd634856f8ca93bafd623ba5357e0f648c68 upstream.

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/
Reported-and-tested-by: syzbot+28b0702ada0bf7381f58@syzkaller.appspotmail.com
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Acked-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index ae1147b8710f3eddbbbe320bd8c1beb120ef398d..12ecde765003f1f1489c5333ec3b320a97438906 100644 (file)
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -1574,10 +1574,14 @@ int batadv_bla_init(struct batadv_priv *bat_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);

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 5762e52f1d1f6c3c1c05d175f8b0f6d2331566b5..c93a75f70f015551b2b7ea1ade7b8f594579ad83 100644 (file)
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -180,29 +180,41 @@ int batadv_mesh_init(struct net_device *soft_iface)
        INIT_HLIST_HEAD(&bat_priv->softif_vlan_list);
        INIT_HLIST_HEAD(&bat_priv->tp_list);
 
-       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);
@@ -212,8 +224,20 @@ int batadv_mesh_init(struct net_device *soft_iface)
 
        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;
 }
 

diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index 7aacec24958eb36e0bb4cf17b09265f280cda03b..400e31ace4d0e0da7789fd7c235b43c59d688124 100644 (file)
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -165,8 +165,10 @@ int batadv_nc_mesh_init(struct batadv_priv *bat_priv)
                                   &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);

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 9cef38f6cb4de0a7e4166bc177a48d76782236d9..47b19ad5a02e1aa43481a46f9daedbe41f8029a7 100644 (file)
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -4373,8 +4373,10 @@ int batadv_tt_init(struct batadv_priv *bat_priv)
                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,