On Wed, 31 Jan 2024 at 05:14, Steven Rostedt <rost...@goodmis.org> wrote: > > If you also notice, tracefs only allows mkdir/rmdir to be assigned to > one directory. Once it is assigned, no other directories can have mkdir > rmdir functionality.
I think that does limit the damage, but it's not clear that it is actually safe. Because you don't hold the inode lock, somebody could come in and do a mkdir inside the other one that is being removed, ie something like - thread 1 does took the inode lock, called ->rmdir - it then drops the inode lock (both parent and the directory that is getting removed) and gets the event lock - but thread 2 can come in between that inode lock drop and event lock Notice: thread 2 comes in where there is *no* locking. Nada. Zilch. This is what worries me. But it does help that it's all only in *one* directory. At least another mkdir cannot happen *inside* the one that is going away while the locks are not held. So the good news is that it does mean that there's only one point that is being protected. But I do worry about things like this (in vfs_rmdir()): inode_lock(dentry->d_inode); error = -EBUSY; if (is_local_mountpoint(dentry) || (dentry->d_inode->i_flags & S_KERNEL_FILE)) goto out; error = security_inode_rmdir(dir, dentry); if (error) goto out; error = dir->i_op->rmdir(dir, dentry); if (error) goto out; notice how it does that "is this a mount point" test atomically wrt the rmdir before it is allowed to proceed. And I do think that the inode lock is what also protects it from concurrent mounts. So now what happens when that "thread 2" above comes in while there is *no* locking, and mounts something there? Now, I'm not saying this is a huge problem. But it's very much an example of a thing that *could* be a problem. Dropping locks in the middle is just very scary. Linus