linux-kernel - Re: 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]

Date:	Tue, 7 Jan 2014 13:16:05 -0500 (EST)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
cc:	Mike Snitzer <snitzer@...hat.com>,
	Bart Van Assche <bvanassche@....org>,
	Jeff Mahoney <jeffm@...e.com>, linux-kernel@...r.kernel.org,
	device-mapper development <dm-devel@...hat.com>,
	tglx@...utronix.de, paulmck@...ux.vnet.ibm.com,
	torvalds@...ux-foundation.org, mingo@...nel.org
Subject: Re: kobject: provide kobject_put_wait to fix module unload race



On Tue, 7 Jan 2014, Greg Kroah-Hartman wrote:

> > Index: linux-3.13-rc7/drivers/md/dm-sysfs.c
> > ===================================================================
> > --- linux-3.13-rc7.orig/drivers/md/dm-sysfs.c	2014-01-07 02:06:08.000000000 +0100
> > +++ linux-3.13-rc7/drivers/md/dm-sysfs.c	2014-01-07 02:07:09.000000000 +0100
> > @@ -79,6 +79,11 @@ static const struct sysfs_ops dm_sysfs_o
> >  	.show	= dm_attr_show,
> >  };
> >  
> > +static void dm_kobject_release(struct kobject *kobj)
> > +{
> > +	complete(dm_get_completion_from_kobject(kobj));
> > +}
> 
> Please read the kobject documentation in the kernel tree for why this
> isn't ok.  The fact that you didn't have a release function at all means
> this code has always been broken, why have you been ignoring the kernel
> complaining about this for so long before?

I read that file and I say that the usage pattern presented in that file 
is broken w.r.t. module unload.

I don't want to fix one race condition and introduce another one (albeit 
smaller).

> You need to free the memory in the release function, not just sit around
> and wait for potentially forever.

How could that code wait forever? Even if userspace keeps the appropriate 
files in sysfs open, the dm device can be unloaded, it doesn't wait 
forever.

> thanks,
> 
> greg k-h

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/