mirror of
https://git.openwrt.org/feed/routing.git
synced 2024-06-18 05:03:57 +02:00
6d10a01dfa
* fix error handling during interface initialization Signed-off-by: Sven Eckelmann <sven@narfation.org>
163 lines
4.9 KiB
Diff
163 lines
4.9 KiB
Diff
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
|
|
@@ -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,
|