linux-kernel - Re: [PATCH 8/19]: SCST SYSFS interface implementation

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: <AANLkTikA7GRDR2Y3nUfQYxLHCWVLfzVPJmZED_whuCWD@mail.gmail.com>
Date:	Fri, 12 Nov 2010 13:09:48 +0100
From:	Bart Van Assche <bvanassche@....org>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Greg KH <greg@...ah.com>
Cc:	Vladislav Bolkhovitin <vst@...b.net>,
	Boaz Harrosh <bharrosh@...asas.com>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	scst-devel <scst-devel@...ts.sourceforge.net>
Subject: Re: [PATCH 8/19]: SCST SYSFS interface implementation

On Fri, Nov 12, 2010 at 2:23 AM, Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
> On Thu, Nov 11, 2010 at 11:50:01PM +0300, Vladislav Bolkhovitin wrote:
> > [ ... ]
> >
> > This is the last internal put. All other references are from outsiders.
> > So, we are waiting for all them to put before we go on.
>
> The question is why do you need to wait here? I presume it is module
> unloading path, but then it is quite bad - you can easily wedge your
> subsystem if you make something to take a reference to your kobject
> while module is trying to be unloaded. Back when sysfs attributes tied
> kobjects the easiest thing was to do:
>
>        rmmod <module> < / sys/devices/..../attribute
>
> If you are done with the kobject - just proceed with what you were doing
> and let it die its own peaceful death some time later. You just need to
> make sure release code sticks around to free it and your subsystem core
> can be tasked with this. Use module counter to prevent unloading of the
> subsystem core until all kobjects belonging to the subsystem are
> destroyed.

Do you mean keeping a kref object in the kernel module, invoking
kref_get() every time a kobject has been created and invoking
kref_put() from the kobject/ktype release method ? That would help to
reduce the race window but would not eliminate all races: as soon as
the last kref_put() has been invoked from the release method, the
module can get unloaded. And module unloading involves freeing all
module code sections, including the section that contains the
implementation of the release method. Which is a race condition.

I'm not sure that it is even possible with the current kobject
implementation to solve this race. I haven't found any information
about this race in Documentation/kobject.txt. And it seems to me that
the code in samples/kobject/kobject-example.c is vulnerable to this
race: methods like foo_show() and foo_store() can access statically
allocated memory ("static int foo") after the module has been
unloaded. Although the race window is small, this makes me wonder
whether module unloading been overlooked at the time the kobject
subsystem has been designed and implemented ?

Bart.
--
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/