Re: [regression] nf_iterate(), BUG: unable to handle kernel NULLpointer dereference

From: Patrick McHardy
Date: Sat Jul 26 2008 - 09:47:35 EST


Linus Torvalds wrote:

On Thu, 24 Jul 2008, Patrick McHardy wrote:
To fix this I think we need a __krealloc() that doesn't
free the old memory, especially since it must not be
freed immediately because it may still be used in a RCU
read side (see the last part in the patch attached to
this mail (based on a kernel without your patch)).

Hmm. Don't you need to fix some of the ordering of the initialization too?

If there are possible readers that happen in parallel with changing this thing, don't you need to protect the update of "ext->len" against the actual changes? And the readers should probably have a read barrier between checking "len" and actually looking at the values?

Extensions can only be added while the conntrack is "unconfirmed",
meaning its not in the hash tables yet and the reference is exclusive.
The reason why we need RCU at all is that the extension areas
might already visible and contained in RCU protected lists.

Finally, why do the "ct->ext" dereference thing, when we know it has to be equal to "new"?

Thats a relict of the old code, which allocated "new" conditionally.
I've taken that part from your patch without the smb_wmb(), which
shouldn't be necessary.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/