fs: allow do_renameat2() over bind mounts of the same filesystem.

Commit Message

Florian Margaine Aug. 28, 2020, 8:40 p.m. UTC

There's currently this seemingly unnecessary limitation that rename()
cannot work over bind mounts of the same filesystem, because the
current check is against the vfsmount, not over the superblock. Given
that the path in do_renameat2() is using dentries, the rename is
properly supported across different mount points, because it is
supported as it is the same superblock.
---
 fs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Al Viro Aug. 28, 2020, 9:34 p.m. UTC | #1

On Fri, Aug 28, 2020 at 10:40:35PM +0200, Florian Margaine wrote:
> There's currently this seemingly unnecessary limitation that rename()
> cannot work over bind mounts of the same filesystem,

... is absolutely deliberate - that's how you set a boundary in the
tree, preventing both links and renames across it.

Incidentally, doing that would have fun effects for anyone with current
directory inside the subtree you'd moved - try and see.

Florian Margaine Aug. 29, 2020, 9:23 p.m. UTC | #2

Al Viro <[e-mail]> writes:

> On Fri, Aug 28, 2020 at 10:40:35PM +0200, Florian Margaine wrote:
>> There's currently this seemingly unnecessary limitation that rename()
>> cannot work over bind mounts of the same filesystem,
>
> ... is absolutely deliberate - that's how you set a boundary in the
> tree, preventing both links and renames across it.

Sorry, I'm not not sure I understand what you're saying.

As I understand it, the tree is the superblock there, not the mount. As
in, the dentries are relative to the superblock, and the mountpoint is
no more than a pointer to a superblock's dentry.

In addition, I noticed this snippet in fs/read_write.c:

    /*
     * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
     * the same mount. Practically, they only need to be on the same file
     * system.
     */
    if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
        return -EXDEV;

Which seems to confirm my understanding.

What am I getting wrong there?

>
> Incidentally, doing that would have fun effects for anyone with current
> directory inside the subtree you'd moved - try and see.

Matthew Wilcox Aug. 29, 2020, 10:12 p.m. UTC | #3

On Sat, Aug 29, 2020 at 11:23:34PM +0200, Florian Margaine wrote:
> Al Viro <[e-mail]> writes:
> 
> > On Fri, Aug 28, 2020 at 10:40:35PM +0200, Florian Margaine wrote:
> >> There's currently this seemingly unnecessary limitation that rename()
> >> cannot work over bind mounts of the same filesystem,
> >
> > ... is absolutely deliberate - that's how you set a boundary in the
> > tree, preventing both links and renames across it.
> 
> Sorry, I'm not not sure I understand what you're saying.

Al's saying this is the way an administrator can intentionally prevent
renames.

>     /*
>      * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
>      * the same mount. Practically, they only need to be on the same file
>      * system.
>      */
>     if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
>         return -EXDEV;

clone doesn't change the contents of a file, merely how they're laid out
on storage.  There's no particular reason for an administrator to
prohibit clone across mount points.

Florian Margaine Aug. 29, 2020, 10:42 p.m. UTC | #4

Matthew Wilcox <[e-mail]> writes:

> On Sat, Aug 29, 2020 at 11:23:34PM +0200, Florian Margaine wrote:
>> Al Viro <[e-mail]> writes:
>> 
>> > On Fri, Aug 28, 2020 at 10:40:35PM +0200, Florian Margaine wrote:
>> >> There's currently this seemingly unnecessary limitation that rename()
>> >> cannot work over bind mounts of the same filesystem,
>> >
>> > ... is absolutely deliberate - that's how you set a boundary in the
>> > tree, preventing both links and renames across it.
>> 
>> Sorry, I'm not not sure I understand what you're saying.
>
> Al's saying this is the way an administrator can intentionally prevent
> renames.

Ah, ok. Thanks!

>
>>     /*
>>      * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
>>      * the same mount. Practically, they only need to be on the same file
>>      * system.
>>      */
>>     if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
>>         return -EXDEV;
>
> clone doesn't change the contents of a file, merely how they're laid out
> on storage.  There's no particular reason for an administrator to
> prohibit clone across mount points.

Patch

diff --git a/fs/namei.c b/fs/namei.c
index e99e2a9da0f7..863e5be88278 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4386,7 +4386,7 @@  static int do_renameat2(int olddfd, const char __user *oldname, int newdfd,
 	}
 
 	error = -EXDEV;
-	if (old_path.mnt != new_path.mnt)
+	if (old_path.mnt->mnt_sb != new_path.mnt->mnt_sb)
 		goto exit2;
 
 	error = -EBUSY;