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

[Marek Lindner <mareklindner@neomailbox.ch>]

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

On Tuesday, June 16, 2015 22:12:00 Marek Lindner wrote:
> On Tuesday, June 16, 2015 15:17:23 Linus Lüssing wrote:
> > +               /* flag checks above + mcast_handler_lock prevents this */
> > +               if (unlikely(!hlist_unhashed(node)))
> > +                       BUG();
> > +
> 
> I don't think this will work because hlist_unhashed() checks for node->pprev
> being NULL or not. hlist_del_rcu() sets node->pprev to LIST_POISON2.
> 
> We could also use BUG_ON() for readability. Something like:
> 
> BUG_ON(node->pprev == LIST_POISON2);
> 
> Though there are not many code sections working with LIST_POISON2 outside
> the list handling code.

Correction: Sven pointed out that we should not depend on LIST_POISON2 as it 
may point to some valid code or something utterly random depending on kernel 
configs.

I had overlooked that you had switched to hlist_del_init_rcu() already which 
makes your check valid. Still, would you consider using BUG_ON() ?

Cheers,
Marek

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