Subject: Re: [regression] nf_iterate(), BUG: unable to handle kernel NULL pointer dereference

From: Patrick McHardy <[email-masked]>
Date: Sat, 26 Jul 2008 15:47:17 +0200

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.