Messages in this thread | | | Date | Thu, 18 Jun 2009 18:13:41 +0200 | From | Pablo Neira Ayuso <> | Subject | Re: [bug] __nf_ct_refresh_acct(): WARNING: at lib/list_debug.c:30 __list_add+0x7d/0xad() |
| |
Patrick McHardy wrote: > 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.
Indeed. I didn't include user-space support for this yet in my tree, so this should not be the problem. Thanks for the catch anyway!
| |