linux-kernel - Re: [PATCH] kobject: provide kobject_put

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: <alpine.LRH.2.02.1401051706040.18406@file01.intranet.prod.int.rdu2.redhat.com>
Date:	Sun, 5 Jan 2014 17:11:23 -0500 (EST)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
cc:	Al Viro <viro@...IV.linux.org.uk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jeff Mahoney <jeffm@...e.com>, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org, dm-devel@...hat.com,
	tglx@...utronix.de, paulmck@...ux.vnet.ibm.com, mingo@...nel.org
Subject: Re: [PATCH] kobject: provide kobject_put_wait to fix module unload
 race



On Sat, 4 Jan 2014, Dmitry Torokhov wrote:

> On Sat, Jan 04, 2014 at 06:34:03PM +0000, Al Viro wrote:
> > On Sat, Jan 04, 2014 at 10:16:20AM -0800, Greg Kroah-Hartman wrote:
> > 
> > > > I came up with a simpler patch to achieve the same purpose - this patch 
> > > > makes fixing the drivers easy - the driver is fixed just by replacing 
> > > > "kobject_put" with "kobject_put_wait" in the unload routine.
> > > 
> > > No, that's not ok at all.
> > 
> > Agreed - all it takes is one cargo-culter who religoiusly does such
> > conversion and drops a ref to parent before that to child.
> > 
> > > > However, this pattern is buggy with respect to modules. The release method
> > > > is placed in the driver's module. When the driver exits, the module
> > > > reference count is zero, thus the module may be freed. However, there may
> > > > still be references to the kobject. If the module is unloaded and then the
> > > > release method is called, a crash happens.
> > > 
> > > Yes, module unloading while a kobject is still "active" is not a good
> > > thing, what modules do you have that cause this problem?  Why not just
> > > grab the module reference in your kobject if you need this type of
> > > protection?  It's not the kobject's code fault that this issue is there,
> > > or that we now have a "delayed release" function to expose this type of
> > > thing, it's the user of the kobject.
> > > 
> > > Please fix the broken users of the kobject first.
> > 
> > <snide> Are you saying that there is another kind? </snide>
> > 
> > When would you grab that reference to module?  More to the point, when
> > would you *drop* it?  Doing so from module_exit is not going to work,
> > obviously...
> 
> You normally have subsystem core module that does handle release of its
> objects and users of said objects so it is usually OK for objects to
> outlive the users, you just need to make sure the core stays around.
> 
> In input we grab module reference to input core when we allocate input
> device and drop it when input device is freed. This way we can be sure
> that input core stays around until all input devices are gone. The same
> for serio.
> 
> Thanks. 
> 
> -- 
> Dmitry

But sometimes, the driver itself needs to create nodes in the sysfs 
filesystem (for example drivers/md/dm-sysfs.c). I don't quite see how 
would you push all driver-specific sysfs nodes into the generic non-module 
code.

Mikulas
--
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/