Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx): > "Serge E. Hallyn" <serue@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
