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/ [1] 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>
This commit is contained in:
parent
fc081477b4
commit
07533f1a67
@ -1569,11 +1569,15 @@ 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 || !bat_priv->bla.backbone_hash)
|
||||
if (!bat_priv->bla.claim_hash)
|
||||
return -ENOMEM;
|
||||
|
||||
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);
|
||||
batadv_hash_set_lock_class(bat_priv->bla.backbone_hash,
|
||||
|
@ -177,29 +177,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);
|
||||
@ -209,8 +221,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;
|
||||
}
|
||||
|
||||
|
@ -166,8 +166,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);
|
||||
|
@ -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,
|
||||
|
Loading…
x
Reference in New Issue
Block a user