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

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

[Florian Margaine]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

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(-)

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;
-- 
2.26.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.

[Al Viro]

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.

Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.

[Florian Margaine]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

Al Viro <viro@zeniv.linux.org.uk> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.

[Matthew Wilcox]

On Sat, Aug 29, 2020 at 11:23:34PM +0200, Florian Margaine wrote:
> Al Viro <viro@zeniv.linux.org.uk> 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.

Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.

[Florian Margaine]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Matthew Wilcox <willy@infradead.org> writes:

> On Sat, Aug 29, 2020 at 11:23:34PM +0200, Florian Margaine wrote:
>> Al Viro <viro@zeniv.linux.org.uk> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]