* [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
@ 2016-05-06 20:27 Sven Eckelmann
2016-05-07 7:03 ` Sven Eckelmann
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sven Eckelmann @ 2016-05-06 20:27 UTC (permalink / raw)
To: b.a.t.m.a.n
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,
--
2.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
2016-05-06 20:27 [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update Sven Eckelmann
@ 2016-05-07 7:03 ` Sven Eckelmann
2016-05-07 9:15 ` Antonio Quartulli
2016-05-07 9:33 ` Antonio Quartulli
2016-05-07 12:07 ` Marek Lindner
2 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2016-05-07 7:03 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]
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");
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
2016-05-07 7:03 ` Sven Eckelmann
@ 2016-05-07 9:15 ` Antonio Quartulli
0 siblings, 0 replies; 7+ messages in thread
From: Antonio Quartulli @ 2016-05-07 9:15 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
2016-05-06 20:27 [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update Sven Eckelmann
2016-05-07 7:03 ` Sven Eckelmann
@ 2016-05-07 9:33 ` Antonio Quartulli
2016-05-07 12:07 ` Marek Lindner
2 siblings, 0 replies; 7+ messages in thread
From: Antonio Quartulli @ 2016-05-07 9:33 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking; +Cc: Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 752 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update
2016-05-06 20:27 [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update Sven Eckelmann
2016-05-07 7:03 ` Sven Eckelmann
2016-05-07 9:33 ` Antonio Quartulli
@ 2016-05-07 12:07 ` Marek Lindner
2016-05-07 12:29 ` [B.A.T.M.A.N.] no able to switch routing algo to V ? contact
2 siblings, 1 reply; 7+ messages in thread
From: Marek Lindner @ 2016-05-07 12:07 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] no able to switch routing algo to V ?
2016-05-07 12:07 ` Marek Lindner
@ 2016-05-07 12:29 ` contact
2016-05-07 13:14 ` Sven Eckelmann
0 siblings, 1 reply; 7+ messages in thread
From: contact @ 2016-05-07 12:29 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] no able to switch routing algo to V ?
2016-05-07 12:29 ` [B.A.T.M.A.N.] no able to switch routing algo to V ? contact
@ 2016-05-07 13:14 ` Sven Eckelmann
0 siblings, 0 replies; 7+ messages in thread
From: Sven Eckelmann @ 2016-05-07 13:14 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 615 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-07 13:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 20:27 [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix double neigh_node_put in batadv_v_ogm_route_update Sven Eckelmann
2016-05-07 7:03 ` Sven Eckelmann
2016-05-07 9:15 ` Antonio Quartulli
2016-05-07 9:33 ` Antonio Quartulli
2016-05-07 12:07 ` Marek Lindner
2016-05-07 12:29 ` [B.A.T.M.A.N.] no able to switch routing algo to V ? contact
2016-05-07 13:14 ` Sven Eckelmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).