Re: [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix potential synchronization issues in mcast tvlv handler

[Marek Lindner <mareklindner@neomailbox.ch>]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

On Monday, June 15, 2015 08:22:25 Linus Lüssing wrote:
> So far the mcast tvlv handler did not anticipate the processing of
> multiple incoming OGMs from the same originator at the same time. This
> can lead to various issues:
> 
> * Broken refcounting: For instance two mcast handlers might both assume
>   that an originator just got multicast capabilities and will together
>   wrongly decrease mcast.num_disabled by two, potentially leading to
>   an integer underflow.
> 
> * Potential kernel panic on hlist_del_rcu(): Two mcast handlers might
>   one after another try to do an
>   hlist_del_rcu(&orig->mcast_want_all_*_node). The second one will
>   cause memory corruption / crashes.
>   (Reported by: Sven Eckelmann <sven@narfation.org>)

As far as I can tell from looking at the code your patch does not address the 
issue raised by Sven.

The first problem is that the mcast code calls hlist_del_rcu() without 
verifying whether or not the element is still in the list. Adding a spinlock 
is not going to change that. You can still have a purge event going on while 
we just receive a new OGM because the purge caller does not need to hold the 
newly added lock.

Cheers,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]