Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.
@ 2020-08-28 20:40 Florian Margaine
  2020-08-28 21:34 ` Al Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Margaine @ 2020-08-28 20:40 UTC (permalink / raw)
  To: viro, linux-fsdevel, linux-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.
  2020-08-28 20:40 [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem Florian Margaine
@ 2020-08-28 21:34 ` Al Viro
  2020-08-29 21:23   ` Florian Margaine
  0 siblings, 1 reply; 5+ messages in thread
From: Al Viro @ 2020-08-28 21:34 UTC (permalink / raw)
  To: Florian Margaine; +Cc: linux-fsdevel, linux-kernel

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.
  2020-08-28 21:34 ` Al Viro
@ 2020-08-29 21:23   ` Florian Margaine
  2020-08-29 22:12     ` Matthew Wilcox
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Margaine @ 2020-08-29 21:23 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, linux-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.
  2020-08-29 21:23   ` Florian Margaine
@ 2020-08-29 22:12     ` Matthew Wilcox
  2020-08-29 22:42       ` Florian Margaine
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2020-08-29 22:12 UTC (permalink / raw)
  To: Florian Margaine; +Cc: Al Viro, linux-fsdevel, linux-kernel

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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem.
  2020-08-29 22:12     ` Matthew Wilcox
@ 2020-08-29 22:42       ` Florian Margaine
  0 siblings, 0 replies; 5+ messages in thread
From: Florian Margaine @ 2020-08-29 22:42 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Al Viro, linux-fsdevel, linux-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-29 22:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 20:40 [PATCH] fs: allow do_renameat2() over bind mounts of the same filesystem Florian Margaine
2020-08-28 21:34 ` Al Viro
2020-08-29 21:23   ` Florian Margaine
2020-08-29 22:12     ` Matthew Wilcox
2020-08-29 22:42       ` Florian Margaine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).