[B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
The router is put down twice when it was non-NULL and either orig_ifinfo is NULL afterwards or batman-adv receives a packet with the same sequence number. This will end up in a use-after-free when the batadv_neigh_node is removed because the reference counter ended up too early at 0.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") Signed-off-by: Sven Eckelmann sven@narfation.org --- It looks like this bug was solved in next/master by refactoring in 30c96bc7870f ("batman-adv: move and restructure batadv_v_ogm_forward")
only compile tested. Please add so many reported by as you want - just don't know it it is *the* bug and who actually reported it. --- net/batman-adv/bat_v_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c index d9bcbe6..91df28a 100644 --- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -529,8 +529,10 @@ static void batadv_v_ogm_route_update(struct batadv_priv *bat_priv, goto out; }
- if (router) + if (router) { batadv_neigh_node_put(router); + router = NULL; + }
/* Update routes, and check if the OGM is from the best next hop */ batadv_v_ogm_orig_update(bat_priv, orig_node, neigh_node, ogm2,
On Friday 06 May 2016 22:27:09 Sven Eckelmann wrote:
The router is put down twice when it was non-NULL and either orig_ifinfo is NULL afterwards or batman-adv receives a packet with the same sequence number. This will end up in a use-after-free when the batadv_neigh_node is removed because the reference counter ended up too early at 0.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") Signed-off-by: Sven Eckelmann sven@narfation.org
[...]
There is a conflict with master. I hope that Antonio can share how it can be resolved when he submits following remaining fixes to David:
* batman-adv: Fix integer overflow in batadv_iv_ogm_calc_tq * batman-adv: Avoid duplicate neigh_node additions * batman-adv: make sure ELP/OGM orig MAC is updated on address change * batman-adv: Fix unexpected free of bcast_own on add_if error * batman-adv: Avoid nullptr derefence in batadv_v_neigh_is_sob * batman-adv: Fix refcnt leak in batadv_v_neigh_* * batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
The solution for the merge conflict with master is:
--- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -510,17 +510,10 @@ goto out; }
-<<<<<<< /* Mark the OGM to be considered for forwarding, and update routes * if needed. */ forward = true; -======= - if (router) { - batadv_neigh_node_put(router); - router = NULL; - } ->>>>>>>
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Searching and updating originator entry of received packet\n");
On Sat, May 07, 2016 at 09:03:13AM +0200, Sven Eckelmann wrote:
On Friday 06 May 2016 22:27:09 Sven Eckelmann wrote:
The router is put down twice when it was non-NULL and either orig_ifinfo is NULL afterwards or batman-adv receives a packet with the same sequence number. This will end up in a use-after-free when the batadv_neigh_node is removed because the reference counter ended up too early at 0.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") Signed-off-by: Sven Eckelmann sven@narfation.org
[...]
There is a conflict with master. I hope that Antonio can share how it can be resolved when he submits following remaining fixes to David:
- batman-adv: Fix integer overflow in batadv_iv_ogm_calc_tq
- batman-adv: Avoid duplicate neigh_node additions
- batman-adv: make sure ELP/OGM orig MAC is updated on address change
- batman-adv: Fix unexpected free of bcast_own on add_if error
- batman-adv: Avoid nullptr derefence in batadv_v_neigh_is_sob
- batman-adv: Fix refcnt leak in batadv_v_neigh_*
- batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
The solution for the merge conflict with master is:
--- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -510,17 +510,10 @@ goto out; }
-<<<<<<< /* Mark the OGM to be considered for forwarding, and update routes * if needed. */ forward = true; -=======
if (router) {batadv_neigh_node_put(router);router = NULL;}->>>>>>>
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Searching and updating originator entry of received packet\n");
Thanks a lot for this.
Cheers,
On Fri, May 06, 2016 at 10:27:09PM +0200, Sven Eckelmann wrote:
The router is put down twice when it was non-NULL and either orig_ifinfo is NULL afterwards or batman-adv receives a packet with the same sequence number. This will end up in a use-after-free when the batadv_neigh_node is removed because the reference counter ended up too early at 0.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") Signed-off-by: Sven Eckelmann sven@narfation.org
Tested-by: Antonio Quartulli a@unstable.cc
it fixes the crash reported in "Kernel panic by BATMAN_V @WBMv9" for me.
@Marek: can you also test this patch in your environment to see if you still see the crash?
Cheers,
On Friday, May 06, 2016 22:27:09 Sven Eckelmann wrote:
The router is put down twice when it was non-NULL and either orig_ifinfo is NULL afterwards or batman-adv receives a packet with the same sequence number. This will end up in a use-after-free when the batadv_neigh_node is removed because the reference counter ended up too early at 0.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") Signed-off-by: Sven Eckelmann sven@narfation.org
It looks like this bug was solved in next/master by refactoring in 30c96bc7870f ("batman-adv: move and restructure batadv_v_ogm_forward")
only compile tested. Please add so many reported by as you want - just don't know it it is *the* bug and who actually reported it.
net/batman-adv/bat_v_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Applied in revision fc3e79d.
Thanks, Marek
Hello,
I just got openwrt 15.05.1 with batman-adv 2016.1 on 3 picostation HP, this time a selected before the build of openwrt batman V option, I can see it when it ask for available routing algo ,
root@OpenWrt:/etc/init.d# cat /sys/kernel/debug/batman_adv/routing_algos Available routing algorithms: * BATMAN_IV * BATMAN_V
but when I do :
echo BATMAN_V > /sys/module/batman_adv/parameters/routing_algo
It does nothing, still on BATMAN_IV during session and on reboot ?
Best Regards,
Laurent
On Saturday 07 May 2016 14:29:17 contact wrote:
I just got openwrt 15.05.1 with batman-adv 2016.1 on 3 picostation HP, this time a selected before the build of openwrt batman V option, I can see it when it ask for available routing algo ,
root@OpenWrt:/etc/init.d# cat /sys/kernel/debug/batman_adv/routing_algos Available routing algorithms:
- BATMAN_IV
- BATMAN_V
but when I do :
echo BATMAN_V > /sys/module/batman_adv/parameters/routing_algo
It does nothing, still on BATMAN_IV during session and on reboot ?
Please don't hijack other (unrelated) threads,
Kind regards, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
contact -
Marek Lindner -
Sven Eckelmann