From: ronnie sahlberg <ronniesahlberg@gmail.com>
To: Pavel Shilovsky <piastryyy@gmail.com>
Cc: Steve French <smfrench@gmail.com>,
Ronnie Sahlberg <lsahlber@redhat.com>,
linux-cifs <linux-cifs@vger.kernel.org>
Subject: Re: [PATCH 14/21] cifs: we do not allow changing username/password/unc/... during remount
Date: Wed, 9 Dec 2020 07:38:20 +1000 [thread overview]
Message-ID: <CAN05THSM2QdRjZ48NHEDeqfGP8kMB19dOdgj91qF-_U_sq8i2Q@mail.gmail.com> (raw)
In-Reply-To: <CAKywueRVB96NKx89Te0OD_O1GcZjbZO+utDN9Lx2A=tGSKeGAA@mail.gmail.com>
On Wed, Dec 9, 2020 at 7:08 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> пн, 7 дек. 2020 г. в 21:07, Steve French <smfrench@gmail.com>:
> >
> > 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@redhat.com> wrote:
> > >
> > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > ---
> > > 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?
No, that is a bug. I will fix that.
Thanks.
>
> --
> Best regards,
> Pavel Shilovsky
next prev parent reply other threads:[~2020-12-08 21:39 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20201207233646.29823-1-lsahlber@redhat.com>
2020-12-07 23:36 ` [PATCH 02/21] cifs: rename dup_vol to smb3_fs_context_dup and move it into fs_context.c Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 03/21] cifs: move the enum for cifs parameters into fs_context.h Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 04/21] cifs: move cifs_parse_devname to fs_context.c Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 05/21] cifs: switch to new mount api Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 06/21] cifs: remove the devname argument to cifs_compose_mount_options Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 07/21] cifs: add an smb3_fs_context to cifs_sb Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 08/21] cifs: get rid of cifs_sb->mountdata Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 09/21] cifs: remove [gu]id/backup[gu]id/file_mode/dir_mode from cifs_sb Ronnie Sahlberg
2020-12-12 19:42 ` Steve French
2020-12-07 23:36 ` [PATCH 10/21] cifs: remove actimeo " Ronnie Sahlberg
2020-12-12 19:44 ` Steve French
2020-12-07 23:36 ` [PATCH 11/21] cifs: move cifs_cleanup_volume_info[_content] to fs_context.c Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 12/21] cifs: move [brw]size from cifs_sb to cifs_sb->ctx Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 13/21] cifs: add initial reconfigure support Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 14/21] cifs: we do not allow changing username/password/unc/... during remount Ronnie Sahlberg
2020-12-08 5:06 ` Steve French
2020-12-08 18:42 ` Pavel Shilovsky
2020-12-08 21:38 ` ronnie sahlberg [this message]
2020-12-07 23:36 ` [PATCH 15/21] cifs: simplify handling of cifs_sb/ctx->local_nls Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 16/21] cifs: don't create a temp nls in cifs_setup_ipc Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 17/21] cifs: uncomplicate printing the iocharset parameter Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 18/21] cifs: do not allow changing posix_paths during remount Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 19/21] cifs: remove ctx argument from cifs_setup_cifs_sb Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 20/21] cifs: move update of flags into a separate function Ronnie Sahlberg
2020-12-07 23:36 ` [PATCH 21/21] cifs: update mnt_cifs_flags during reconfigure Ronnie Sahlberg
2020-12-09 3:15 [PATCH 14/21] cifs: we do not allow changing username/password/unc/... during remount Ronnie Sahlberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAN05THSM2QdRjZ48NHEDeqfGP8kMB19dOdgj91qF-_U_sq8i2Q@mail.gmail.com \
--to=ronniesahlberg@gmail.com \
--cc=linux-cifs@vger.kernel.org \
--cc=lsahlber@redhat.com \
--cc=piastryyy@gmail.com \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).