Re: [PATCH] Btrfs: prevent deletion of mounted subvolumes

[David Sterba]

On Mon, Mar 30, 2015 at 02:02:17AM -0700, Omar Sandoval wrote:
> Before commit bafc9b754f75 ("vfs: More precise tests in d_invalidate"),
> d_invalidate() could return -EBUSY when a dentry for a directory had
> more than one reference to it. This is what prevented a mounted
> subvolume from being deleted, as struct vfsmount holds a reference to
> the subvolume dentry. However, that commit removed that case, and later
> commits in that patch series removed the return code from d_invalidate()
> completely, so we don't get that check for free anymore. So, reintroduce
> it in btrfs_ioctl_snap_destroy().

> This applies to 4.0-rc6. To be honest, I'm not sure that this is the most
> correct fix for this bug, but it's equivalent to the pre-3.18 behavior and it's
> the best that I could come up with. Thoughts?

> +	spin_lock(&dentry->d_lock);
> +	err = dentry->d_lockref.count > 1 ? -EBUSY : 0;
> +	spin_unlock(&dentry->d_lock);

The fix restores the original behaviour, but I don't think opencoding and
using internals is fine. Either there should be a vfs api for that or
there's an existing one that can be used instead.

The bug here seems defined up to the point that we're trying to delete a
subvolume that's a mountpoint. My next guess is that a check

	if (d_mountpoint(&dentry)) { ... }

could work.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/