Re: [PATCH] ksmbd: clear RENAME_NOREPLACE before calling vfs_rename

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 15, 2024 at 12:51 PM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote:
>
> 2024년 4월 15일 (월) 오후 6:01, Marios Makassikis <mmakassikis@xxxxxxxxxx>님이 작성:
> >
> > On Wed, Mar 13, 2024 at 2:07 PM Marios Makassikis
> > <mmakassikis@xxxxxxxxxx> wrote:
> > >
> > > File overwrite case is explicitly handled, so it is not necessary to
> > > pass RENAME_NOREPLACE to vfs_rename.
> > >
> > > Clearing the flag fixes rename operations when the share is a ntfs-3g
> > > mount. The latter uses an older version of fuse with no support for
> > > flags in the ->rename op.
> > >
> > > Signed-off-by: Marios Makassikis <mmakassikis@xxxxxxxxxx>
> > > ---
> > >  fs/smb/server/vfs.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> >
> > Bumping this as I haven't received any feedback.
> > Are there any issues with the patch ?
> Sorry for missing this patch. Please cc me when submitting the patch
> to the list next time.
> I didn't understand why it is a problem with ntfs-3g yet.
> Is it just clean-up patch ? or this flags cause some issue with ntfs-3g ?
> Could you please elaborate more ?
>
> Thanks!

Until commit 74d7970febf ("ksmbd: fix racy issue from using ->d_parent
and ->d_name"),
the logic to overwrite a file or fail depending on the ReplaceIfExists
flag was open-coded.
This is the same as calling vfs_rename() with the RENAME_NOREPLACE flag, so it
makes sense to use that instead.

When using FUSE, the behaviour depends on the userland application implementing
the fs. On the kernel side, this is the function that ends up being called:

fs/fuse/dir.c:
static int fuse_rename2(struct mnt_idmap *idmap, struct inode *olddir,
                        struct dentry *oldent, struct inode *newdir,
                        struct dentry *newent, unsigned int flags)
{
        struct fuse_conn *fc = get_fuse_conn(olddir);
        int err;

        if (fuse_is_bad(olddir))
                return -EIO;

        if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
                return -EINVAL;

        if (flags) {
                if (fc->no_rename2 || fc->minor < 23)
                        return -EINVAL;

                err = fuse_rename_common(olddir, oldent, newdir, newent, flags,
                                         FUSE_RENAME2,
                                         sizeof(struct fuse_rename2_in));
                if (err == -ENOSYS) {
                        fc->no_rename2 = 1;
                        err = -EINVAL;
                }
        } else {
                err = fuse_rename_common(olddir, oldent, newdir, newent, 0,
                                         FUSE_RENAME,
                                         sizeof(struct fuse_rename_in));
        }

        return err;
}

Because ntfs-3g uses an older version of the FUSE API and flags are
passed by ksmbd,
rename attempts fail because of this bit:

        if (flags) {
                if (fc->no_rename2 || fc->minor < 23)
                        return -EINVAL;

ksmbd already handles the overwrite case before even calling
vfs_rename(). So passing
the flag doesn't add much.

--
Marios





[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux