Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Muli Ben-Yehuda]

On Tue, Aug 08, 2006 at 07:32:51AM -0700, akpm@osdl.org wrote:
> 
> The patch titled
> 
>      sys_getppid() oopses on debug kernel
> 
> has been added to the -mm tree.  Its filename is
> 
>      sys_getppid-oopses-on-debug-kernel.patch
> 
> See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> out what to do about this
> 
> ------------------------------------------------------
> Subject: sys_getppid() oopses on debug kernel
> From: Kirill Korotaev <dev@sw.ru>
> 
> sys_getppid() optimization can access a freed memory.  On kernels with
> DEBUG_SLAB turned ON, this results in Oops.
> 
> Signed-off-by: Kirill Korotaev <dev@openvz.org>
> Cc: <stable@kernel.org>
> Signed-off-by: Andrew Morton <akpm@osdl.org>

I'm probably missing something, but is it really valid to access freed
kernel memory even if CONFIG_DEBUG_SLAB is off - as this patch does?

Cheers,
Muli

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Björn Steinbrink]

On 2006.08.08 17:39:37 +0300, Muli Ben-Yehuda wrote:
> On Tue, Aug 08, 2006 at 07:32:51AM -0700, akpm@osdl.org wrote:
> > 
> > The patch titled
> > 
> >      sys_getppid() oopses on debug kernel
> > 
> > has been added to the -mm tree.  Its filename is
> > 
> >      sys_getppid-oopses-on-debug-kernel.patch
> > 
> > See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> > out what to do about this
> > 
> > ------------------------------------------------------
> > Subject: sys_getppid() oopses on debug kernel
> > From: Kirill Korotaev <dev@sw.ru>
> > 
> > sys_getppid() optimization can access a freed memory.  On kernels with
> > DEBUG_SLAB turned ON, this results in Oops.
> > 
> > Signed-off-by: Kirill Korotaev <dev@openvz.org>
> > Cc: <stable@kernel.org>
> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> 
> I'm probably missing something, but is it really valid to access freed
> kernel memory even if CONFIG_DEBUG_SLAB is off - as this patch does?

There's a note right above the function that explains it:
 * NOTE! This depends on the fact that even if we _do_
 * get an old value of "parent", we can happily dereference
 * the pointer (it was and remains a dereferencable kernel pointer
 * no matter what): we just can't necessarily trust the result
 * until we know that the parent pointer is valid.

HTH
Björn

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Muli Ben-Yehuda]

On Tue, Aug 08, 2006 at 04:51:38PM +0200, Björn Steinbrink wrote:

> There's a note right above the function that explains it:
>  * NOTE! This depends on the fact that even if we _do_
>  * get an old value of "parent", we can happily dereference
>  * the pointer (it was and remains a dereferencable kernel pointer
>  * no matter what): we just can't necessarily trust the result
>  * until we know that the parent pointer is valid.

Even without getting into just how ugly this is, is it really worth
it?

Cheers,
Muli

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Alan Cox]

Ar Maw, 2006-08-08 am 17:57 +0300, ysgrifennodd Muli Ben-Yehuda:
> On Tue, Aug 08, 2006 at 04:51:38PM +0200, Björn Steinbrink wrote:
> 
> > There's a note right above the function that explains it:
> >  * NOTE! This depends on the fact that even if we _do_
> >  * get an old value of "parent", we can happily dereference
> >  * the pointer (it was and remains a dereferencable kernel pointer
> >  * no matter what): we just can't necessarily trust the result
> >  * until we know that the parent pointer is valid.
> 
> Even without getting into just how ugly this is, is it really worth
> it?

It never was in my opinion but I lost that battle to Linus in 1.3.40 or
so timescales. Given how critical getppid _isnt_ I don't see the point
in being clever.

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Kirill Korotaev]

>>Even without getting into just how ugly this is, is it really worth
>>it?
it is impossible to run debug kernels w/o this patch :/
or are you asking whether this optimization worth it?

What makes me worry is that this is a sign that vendors
don't even bother to run debug kernels :((((

> It never was in my opinion but I lost that battle to Linus in 1.3.40 or
> so timescales. Given how critical getppid _isnt_ I don't see the point
> in being clever.
Alan, if you sign off the patch I will prepare another one, which removes
the optimization away and make it always safe.

Kirill

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Dave Jones]

On Tue, Aug 08, 2006 at 07:39:52PM +0400, Kirill Korotaev wrote:
 > >>Even without getting into just how ugly this is, is it really worth
 > >>it?
 > it is impossible to run debug kernels w/o this patch :/
 > or are you asking whether this optimization worth it?
 > 
 > What makes me worry is that this is a sign that vendors
 > don't even bother to run debug kernels :((((

Fedora rawhide is nearly always shipping with DEBUG_SLAB enabled,
and we didn't hit this once.  Are you sure this is a problem
with DEBUG_SLAB, and not DEBUG_PAGEALLOC ?

		Dave

-- 
http://www.codemonkey.org.uk

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Kirill Korotaev]

>  > >>Even without getting into just how ugly this is, is it really worth
>  > >>it?
>  > it is impossible to run debug kernels w/o this patch :/
>  > or are you asking whether this optimization worth it?
>  > 
>  > What makes me worry is that this is a sign that vendors
>  > don't even bother to run debug kernels :((((
> 
> Fedora rawhide is nearly always shipping with DEBUG_SLAB enabled,
> and we didn't hit this once.  Are you sure this is a problem
> with DEBUG_SLAB, and not DEBUG_PAGEALLOC ?
Sorry, it's my fault. Surely, CONFIG_DEBUG_PAGEALLOC.

Kirill

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Dave Jones]

On Wed, Aug 09, 2006 at 12:12:49PM +0400, Kirill Korotaev wrote:
 > >  > >>Even without getting into just how ugly this is, is it really worth
 > >  > >>it?
 > >  > it is impossible to run debug kernels w/o this patch :/
 > >  > or are you asking whether this optimization worth it?
 > >  > 
 > >  > What makes me worry is that this is a sign that vendors
 > >  > don't even bother to run debug kernels :((((
 > > 
 > > Fedora rawhide is nearly always shipping with DEBUG_SLAB enabled,
 > > and we didn't hit this once.  Are you sure this is a problem
 > > with DEBUG_SLAB, and not DEBUG_PAGEALLOC ?
 > Sorry, it's my fault. Surely, CONFIG_DEBUG_PAGEALLOC.

Then you're correct, vendors rarely turn this on :)
I do sometimes if I'm trying to chase down something particularly
difficult, and it usually gets me a bunch of mail from users
asking why 'everything got all slow', so it's a last-resort option
rather than a 'on all the time' option.

		Dave

-- 
http://www.codemonkey.org.uk

Re: + sys_getppid-oopses-on-debug-kernel.patch added to -mm tree

[Kirill Korotaev]

> Then you're correct, vendors rarely turn this on :)
> I do sometimes if I'm trying to chase down something particularly
> difficult, and it usually gets me a bunch of mail from users
> asking why 'everything got all slow', so it's a last-resort option
> rather than a 'on all the time' option.
Yeah, maybe it is a last resort option for users, but at least for internal
testing it helps. We regularly run such debug kernel through testing cycle
and it catches some races from time to time.

Kirill