linux-kernel - Re: [PATCH 04/20] sysfs: Handle the general case of removing of directories with subdirectories

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:	Fri, 22 May 2009 23:33:59 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Tejun Heo <tj@...nel.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	linux-kernel@...r.kernel.org,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	linux-fsdevel@...r.kernel.org,
	"Eric W. Biederman" <ebiederm@...stanetworks.com>
Subject: Re: [PATCH 04/20] sysfs: Handle the general case of removing of directories with subdirectories

Tejun Heo <tj@...nel.org> writes:

> Hello, Eric.
>
> Eric W. Biederman wrote:
>> Tejun Heo <tj@...nel.org> writes:
>> 
>>> Well, it can be trivially fixed by checking the removed flag.  The
>>> add/rm thing is designed to help additions and removals of multiple
>>> nodes at one go and I'd really like to see it working that way.  Any
>>> chance you can change code toward that direction?
>> 
>> Yes.  We definitely need to check the removed flag in sysfs_add_one.
>> Regardless of anything else.
>> 
>> I need to sleep on this but I am inclined to get rid of the rest of
>> the complications simply by failing the removal of non-empty
>> directories.  Going through the upper layers and making them properly
>> responsible for their actions.
>> 
>> I am afraid friendlier in this circumstance might equate to easier
>> to misuse and let code bugs pile up.
>
> I'm going through the latter part of the patchset and the code around
> this area gets much simpler there.  Would it be possible to make it
> atomic after the simplification?  Requiring recursive deletion from
> all the callers is silly and error prone.

I have slept and looked at this in some detail.

There may be some virtue in better support from sysfs for deleting
objects.  At this point my observation is that support comes from the
kobject and device layers.  Where you can define all of the attributes
of a device up front.

My goal is to make the current sysfs as simple and as correct as I can
before changes are made to either it's interface or otherwise make it
better suited to work.

The case I have been worried about is someone removing a subsystem
before unregistering it's devices, or otherwise removing a real parent
before removing it's children.

After a little more investigation that is exactly what is happening
today with /sys/dev/char and /sys/dev/block.

Those ordering issues we must handle because quite frequently there
are real hardware complications and that is exactly the case that the
kernel device tree was built to address.  So I expect any amount of
getting it wrong in sysfs is actually getting it wrong in the device
tree.

There is another problem with relying on recursive delete.  When we
come to delete one of our objects that someone else recursively
deleted we will hit the BUG_ON in sysfs_remove_one, and then attempt
to run operations that we have already run.  Not maintaining that the
prerequisite things exist for the lifetime of something in sysfs
sounds really icky.

My plan going forward is to fix the ordering problems with deleting
/sys/dev/char and /sys/dev/block.  Add a WARN_ON if we delete a
non-empty directory.  Ensure we don't add something to an already
deleted directory.

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