Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix general protection fault in batadv_tt_global_del_orig()

[Antonio Quartulli <ordex@autistici.org>]

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

On Sat, Mar 30, 2013 at 02:16:02PM +0100, Antonio Quartulli wrote:
> On Sun, Mar 17, 2013 at 05:44:58AM +0100, Linus Lüssing wrote:
> > On shutdown a race condition where we access a just freed global TT hash
> > might occure. batadv_orig_node_free_rcu() callbacks might have been
> > scheduled (especially during the shutdown procedure) and unfortunately
> > batadv_tt_global_table_free() does not wait for them to finish first
> > before freeing the global TT hash.
> > 
> > This potentially results in a general protection fault in
> > batadv_tt_global_del_orig(), called via a batadv_orig_node_free_rcu()
> > callback, which tries to access the just freed global TT hash.
> > 
> > This patch tries to fix this by waiting for any just scheduled
> > batadv_orig_node_free_rcu() to finish via an extra rcu_barrier() call
> > before freeing the global TT hash. And by moving the TT freeing call to
> > the end of the batman cleanup routines.
> > 
> > Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> 
> Acked-by: Antonio Quartulli <ordex@autistici.org>

NACK.

This patch is solving one problem but creating a new one:
by using rcu_barrier we avoid the crash but we will leak memory, because
batadv_orig_node_free_rcu()->batadv_tt_global_del_orig() will access an empty
global table and so will not be able to free the global entries.


Patch
("batman-adv: avoid race conditions on TT global table by counting references")
is fixing the problem by redesigning the TT clean up routine.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]