Re: [PATCH 14/21] cifs: we do not allow changing username/password/unc/... during remount

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

 



пн, 7 дек. 2020 г. в 21:07, Steve French <smfrench@xxxxxxxxx>:
>
> Minor nits pointed out by checkpatch:
>
> 0015-cifs-we-do-not-allow-changing-username-password-unc-.patch
> ---------------------------------------------------------------
> WARNING: Missing commit description - Add an appropriate one
>
> WARNING: kfree(NULL) is safe and this check is probably not required
> #76: FILE: fs/cifs/fs_context.c:673:
> + if (ctx->field) { \
> + kfree(ctx->field);
>
> On Mon, Dec 7, 2020 at 5:37 PM Ronnie Sahlberg <lsahlber@xxxxxxxxxx> wrote:
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> >  fs/cifs/cifsfs.c     |  2 +-
> >  fs/cifs/fs_context.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++---
> >  fs/cifs/fs_context.h |  2 +-
> >  3 files changed, 54 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 80117e9d35f9..13d7f4a3c836 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -490,7 +490,7 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
> >
> >         if (tcon->no_lease)
> >                 seq_puts(s, ",nolease");
> > -       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)
> > +       if (cifs_sb->ctx->multiuser)
> >                 seq_puts(s, ",multiuser");
> >         else if (tcon->ses->user_name)
> >                 seq_show_option(s, "username", tcon->ses->user_name);
> > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > index edfdea129fcc..542fa75b74aa 100644
> > --- a/fs/cifs/fs_context.c
> > +++ b/fs/cifs/fs_context.c
> > @@ -629,10 +629,53 @@ static int smb3_verify_reconfigure_ctx(struct smb3_fs_context *new_ctx,
> >                 cifs_dbg(VFS, "can not change sec during remount\n");
> >                 return -EINVAL;
> >         }
> > +       if (new_ctx->multiuser != old_ctx->multiuser) {
> > +               cifs_dbg(VFS, "can not change multiuser during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->UNC &&
> > +           (!old_ctx->UNC || strcmp(new_ctx->UNC, old_ctx->UNC))) {
> > +               cifs_dbg(VFS, "can not change UNC during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->username &&
> > +           (!old_ctx->username || strcmp(new_ctx->username, old_ctx->username))) {
> > +               cifs_dbg(VFS, "can not change username during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->password &&
> > +           (!old_ctx->password || strcmp(new_ctx->password, old_ctx->password))) {
> > +               cifs_dbg(VFS, "can not change password during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->domainname &&
> > +           (!old_ctx->domainname || strcmp(new_ctx->domainname, old_ctx->domainname))) {
> > +               cifs_dbg(VFS, "can not change domainname during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->nodename &&
> > +           (!old_ctx->nodename || strcmp(new_ctx->nodename, old_ctx->nodename))) {
> > +               cifs_dbg(VFS, "can not change nodename during remount\n");
> > +               return -EINVAL;
> > +       }
> > +       if (new_ctx->iocharset &&
> > +           (!old_ctx->iocharset || strcmp(new_ctx->iocharset, old_ctx->iocharset))) {
> > +               cifs_dbg(VFS, "can not change iocharset during remount\n");
> > +               return -EINVAL;
> > +       }
> >
> >         return 0;
> >  }
> >
> > +#define STEAL_STRING(cifs_sb, ctx, field)                              \
> > +do {                                                                   \
> > +       if (ctx->field) {                                               \
> > +               kfree(ctx->field);                                      \
> > +               ctx->field = cifs_sb->ctx->field;                       \
> > +               cifs_sb->ctx->field = NULL;                             \
> > +       }                                                               \
> > +} while (0)

If ctx->field is NULL we won't assign new value from
cifs_sb->ctx->field and the procedure will become no-op. Is this an
intent?

--
Best regards,
Pavel Shilovsky




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

  Powered by Linux