Messages in this thread | | | Date | Thu, 18 Jun 2009 18:09:48 +0200 | From | Patrick McHardy <> | Subject | Re: [bug] __nf_ct_refresh_acct(): WARNING: at lib/list_debug.c:30 __list_add+0x7d/0xad() |
| |
Eric Dumazet wrote: > In my own analysis, I found death_by_timeout() might be problematic, > with RCU and lockless lookups. > > static void death_by_timeout(unsigned long ul_conntrack) > { > struct nf_conn *ct = (void *)ul_conntrack; > > if (!test_bit(IPS_DYING_BIT, &ct->status) && > unlikely(nf_conntrack_event(IPCT_DESTROY, ct) < 0)) { > /* destroy event was not delivered */ > nf_ct_delete_from_lists(ct); > << HERE >> > > nf_ct_insert_dying_list(ct); > return; > } > set_bit(IPS_DYING_BIT, &ct->status); > nf_ct_delete_from_lists(ct); > nf_ct_put(ct); > } > > > We delete ct from a list and insert it in a new list. > > I believe a reader could "*catch*" ct while doing a lookup and miss the end > of its chain. (nulls algo check the null value at the end of lookup and can > decide to restart the lookup if the null value is not the expected one) > > We need to change nf_conntrack_init_net() and use a different "null" value, > guaranteed not being used in regular lists
Good catch. This is a new bug, but it shouldn't matter in this case since nf_conntrack_event() can't fail unless you have a userspace listener that makes use of reliable delivery, which I think hasn't even been released yet.
> Patch follows :
Looks good. If you send me a Signed-off-by: I'll already apply it.
| |