linux-kernel - Re: [PATCH 1/2] kobject: don't delay to cleanup module kobject

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]

Message-ID: <YaEMaWMT+PmMvSwg@kroah.com>
Date:   Fri, 26 Nov 2021 17:33:45 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Ming Lei <ming.lei@...hat.com>
Cc:     Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org,
        Luis Chamberlain <mcgrof@...nel.org>,
        Joe Lawrence <joe.lawrence@...hat.com>
Subject: Re: [PATCH 1/2] kobject: don't delay to cleanup module kobject

On Sat, Nov 27, 2021 at 12:28:48AM +0800, Ming Lei wrote:
> On Fri, Nov 26, 2021 at 05:08:16PM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Nov 05, 2021 at 02:37:09PM +0800, Ming Lei wrote:
> > > CONFIG_DEBUG_KOBJECT_RELEASE is used for debugging kobject release/cleanup
> > > issue. The module kobject is released after module_exit() returns. If
> > > this kobject is delayed too much, and may cause other kobject's
> > > cleaned up a bit earlier before freeing module, then real issue is
> > > hidden.
> > > 
> > > So don't delay module kobject's cleanup, meantime module kobject is
> > > always cleaned up synchronously, and we needn't module kobject's
> > > cleanup.
> > > 
> > > Signed-off-by: Ming Lei <ming.lei@...hat.com>
> > > ---
> > >  lib/kobject.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/lib/kobject.c b/lib/kobject.c
> > > index ea53b30cf483..4c0dbe11be3d 100644
> > > --- a/lib/kobject.c
> > > +++ b/lib/kobject.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/stat.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/random.h>
> > > +#include <linux/module.h>
> > >  
> > >  /**
> > >   * kobject_namespace() - Return @kobj's namespace tag.
> > > @@ -727,6 +728,10 @@ static void kobject_release(struct kref *kref)
> > >  	struct kobject *kobj = container_of(kref, struct kobject, kref);
> > >  #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> > >  	unsigned long delay = HZ + HZ * (get_random_int() & 0x3);
> > > +
> > > +	if (kobj->ktype == &module_ktype)
> > > +		delay = 0;
> > 
> > No, there should not be anything "special" about module kobjects to get
> > this kind of treatment.  They should work like any other kobject and
> > clean up properly when needed.
> 
> Here setting 0 delay for module kobject is just for making DEBUG_KOBJECT_RELEASE
> reliable to detect/report issues. Otherwise if the random delay for module
> kobject is bigger than other kobjects, potential use-after-after won't
> be exposed.

So you now can not debug the module kobject code?

This needs to be documented really really really well why this kobject
type is somehow "special" in the code.  We should not special-case these
things unless you have a great reason, and I am not yet convinced.

thanks,

greg k-h