Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fbf150b1 authored by Pavel Skripkin's avatar Pavel Skripkin Committed by Greg Kroah-Hartman
Browse files

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: default avatar <syzbot+28b0702ada0bf7381f58@syzkaller.appspotmail.com>
Fixes: c6c8fea2 ("net: Add batman-adv meshing protocol")
Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
Acked-by: default avatarSven Eckelmann <sven@narfation.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 3dae1a4e

net/batman-adv/bridge_loop_avoidance.c

+6 −2
Original line number Diff line number Diff line
@@ -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);

net/batman-adv/main.c

+40 −16
Original line number Diff line number Diff line
@@ -187,29 +187,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);
@@ -219,8 +231,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;

}

net/batman-adv/network-coding.c

+3 −1
Original line number Diff line number Diff line
@@ -167,8 +167,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);

net/batman-adv/translation-table.c

+3 −1
Original line number Diff line number Diff line
@@ -4413,8 +4413,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,