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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 26 Jul 2008 15:47:17 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Pekka Enberg <penberg@...helsinki.fi>, Ingo Molnar <mingo@...e.hu>,
	David Miller <davem@...emloft.net>,
	herbert@...dor.apana.org.au, w@....eu, davidn@...idnewall.com,
	akpm@...ux-foundation.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, stefanr@...6.in-berlin.de,
	rjw@...k.pl, ilpo.jarvinen@...sinki.fi,
	Dave Jones <davej@...hat.com>
Subject: Re: [regression] nf_iterate(), BUG: unable to handle kernel NULL
 pointer dereference

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/