lkml.org 
[lkml]   [2010]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/6] sysfs: Implement sysfs tagged directory support.
Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
>
> >> > This is a huge patch, and for the most part I haven't found any problems,
> >> > except potentially this one. It looks like sysfs_rename_link() checks
> >> > old_ns and new_ns before calling sysfs_rename(). But sysfs_mutex isn't
> >> > taken until sysfs_rename(). sysfs_rename() will then proceed to do
> >> > the rename, and unconditionally set sd->ns = new_ns.
> >> >
> >> > In the meantime, it seems as though new_ns might have exited, and
> >> > sysfs_exit_ns() unset new_ns on the new parent dir. This means that
> >> > we'll end up with the namespace code having thought that it cleared
> >> > all new_ns's, but this file will have snuck by. Meaning an action on
> >> > the renamed file might dereference a freed namespace.
> >> >
> >> > Or am I way off base?
> >>
> >> There are a couple of reasons why this is not a concern.
> >>
> >> The only new_ns we clear is on the super block.
> >
> > Oops, yeah - I failed to note that.
> >
> >> sysfs itself never dereferences namespace arguments and only uses them
> >> for comparison purposes. They are just cookies that cause comparisons
> >> to differ from a sysfs perspective.
> >>
> >> The upper levels are responsible for taking care of them selves
> >> sysfs_mutex does not protect them. If you compile out sysfs the sysfs
> >> mutex is not even present.
> >>
> >> In the worst case if the upper levels mess up we will have a stale
> >> token that we never dereference on a sysfs dirent, which in a pathological
> >> case will happen to be the same as a new namespace and we will have
> >> a spurious directory entry that we have leaked.
> >>
> >> In practice we move all network devices (and thus sysfs files) out of
> >> a network namespace before allowing it to exit.
> >
> > Ok, that makes sense too - so any tagged sysfs file created for some object
> > in a ns must be deleted at netns exit. I could imagine someone expecting
> > that if the ns exits, the tasks in the ns will exit, causing the sysfs
> > mount to be umounted and auto-deleting the files? (which of course would
> > get buggered if task in other ns was examining the mount which it got
> > through mounts propagation) We'll have to make sure noone does that. Should
> > it be documented somewhere, or is that obvious enough?
>
> In general it is simply true. An object in a namespace either keeps
> the namespace alive, or it is destroyed when the namespace exits
> because the object is unreachable.

I guess you'd hope so :)

> So the only possible problem I can think of is of ordering the object
> destruction and calling sysfs_exit_ns. So for the moment I am going
> to vote that this is simply obvious enough not to worry about in detail.
>
> It is also pretty obvious if you trace the code and ask how does sysfs
> dirent X get destroyed.
>
> Today there is just a wee bit of automatic file destruction at the sysfs
> level. The device layer does not take advantage of it, and in hierarchical
> situation it leads to bugs. So even I think if we document anything it
> should be that sysfs can not safely automatically delete anything, for
> you.
>
> Eric

Ok. I'm convinced.

thanks,
-serge


\
 
 \ /
  Last update: 2010-03-31 06:55    [from the cache]
©2003-2011 Jasper Spaans