From 6d10a01dfa68eb5e25b84b33e33aefd312f46f4d Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Fri, 19 Nov 2021 17:05:03 +0100 Subject: [PATCH] batman-adv: Merge bugfixes from 2021.4 * fix error handling during interface initialization Signed-off-by: Sven Eckelmann --- .../0006-batman-adv-fix-error-handling.patch | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 batman-adv/patches/0006-batman-adv-fix-error-handling.patch diff --git a/batman-adv/patches/0006-batman-adv-fix-error-handling.patch b/batman-adv/patches/0006-batman-adv-fix-error-handling.patch new file mode 100644 index 0000000..f0a00d7 --- /dev/null +++ b/batman-adv/patches/0006-batman-adv-fix-error-handling.patch @@ -0,0 +1,162 @@ +From: Pavel Skripkin +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 +Signed-off-by: David S. Miller +Signed-off-by: Sven Eckelmann +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 +@@ -1556,10 +1556,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 +@@ -189,29 +189,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); +@@ -221,8 +233,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 +@@ -152,8 +152,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 +@@ -4193,8 +4193,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,