linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ronnie sahlberg <ronniesahlberg@gmail.com>
To: Pavel Shilovsky <piastryyy@gmail.com>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>,
	linux-cifs <linux-cifs@vger.kernel.org>,
	Steve French <smfrench@gmail.com>
Subject: Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
Date: Fri, 9 Jul 2021 09:25:16 +1000	[thread overview]
Message-ID: <CAN05THRzn29As=NO6vNyGyJocuJx4ddiuZeXQ05g2Am6Z+2GjA@mail.gmail.com> (raw)
In-Reply-To: <CAKywueSiuJU4YipMtsyf+CYDywSX3ySgMrmsvt9swJT4GkjTZg@mail.gmail.com>

On Fri, Jul 9, 2021 at 8:12 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> ср, 7 июл. 2021 г. в 16:25, Ronnie Sahlberg <lsahlber@redhat.com>:
> >
> > Use the nice helpers to initialize and the uid/gid/cred_uid when passed as mount arguments.
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/fs_context.c | 24 +++++++++++++++++++-----
> >  fs/cifs/fs_context.h |  1 +
> >  2 files changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > index 92d4ab029c91..553adfbcc22a 100644
> > --- a/fs/cifs/fs_context.c
> > +++ b/fs/cifs/fs_context.c
> > @@ -322,7 +322,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
> >         new_ctx->UNC = NULL;
> >         new_ctx->source = NULL;
> >         new_ctx->iocharset = NULL;
> > -
> >         /*
> >          * Make sure to stay in sync with smb3_cleanup_fs_context_contents()
> >          */
> > @@ -792,6 +791,8 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> >         int i, opt;
> >         bool is_smb3 = !strcmp(fc->fs_type->name, "smb3");
> >         bool skip_parsing = false;
> > +       kuid_t uid;
> > +       kgid_t gid;
> >
> >         cifs_dbg(FYI, "CIFS: parsing cifs mount option '%s'\n", param->key);
> >
> > @@ -904,18 +905,31 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> >                 }
> >                 break;
> >         case Opt_uid:
> > -               ctx->linux_uid.val = result.uint_32;
> > +               uid = make_kuid(current_user_ns(), result.uint_32);
> > +               if (!uid_valid(uid))
> > +                       goto cifs_parse_mount_err;
> > +               ctx->linux_uid = uid;
> >                 ctx->uid_specified = true;
> >                 break;
> >         case Opt_cruid:
> > -               ctx->cred_uid.val = result.uint_32;
> > +               uid = make_kuid(current_user_ns(), result.uint_32);
> > +               if (!uid_valid(uid))
> > +                       goto cifs_parse_mount_err;
> > +               ctx->cred_uid = uid;
> > +               ctx->cruid_specified = true;
> >                 break;
> >         case Opt_backupgid:
> > -               ctx->backupgid.val = result.uint_32;
> > +               gid = make_kgid(current_user_ns(), result.uint_32);
> > +               if (!gid_valid(gid))
> > +                       goto cifs_parse_mount_err;
> > +               ctx->backupgid = gid;
> >                 ctx->backupgid_specified = true;
> >                 break;
> >         case Opt_gid:
> > -               ctx->linux_gid.val = result.uint_32;
> > +               gid = make_kgid(current_user_ns(), result.uint_32);
> > +               if (!gid_valid(gid))
> > +                       goto cifs_parse_mount_err;
> > +               ctx->linux_gid = gid;
> >                 ctx->gid_specified = true;
> >                 break;
> >         case Opt_port:
> > diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
> > index 2a71c8e411ac..b6243972edf3 100644
> > --- a/fs/cifs/fs_context.h
> > +++ b/fs/cifs/fs_context.h
> > @@ -155,6 +155,7 @@ enum cifs_param {
> >
> >  struct smb3_fs_context {
> >         bool uid_specified;
> > +       bool cruid_specified;
>
> Is it going to be used somewhere?

I use it in other patches,  and may add a similar variable to the
session structure as well so we can change the code where we print the
mount argument and make it conditional on whether cruid was set on the
original command line or not.
We currently always print cruid as a mount argument in the mount
output and print it as cruid=0 if it was not specified, which is fine
but may be confusing/redundant.

I am fine with either leaving it in or removing it, and I can add it
back later once it actually starts being used.

>
> >         bool gid_specified;
> >         bool sloppy;
> >         bool got_ip;
> > --
> > 2.30.2
> >
>
> Acked-by: Pavel Shilovsky <pshilovsky@samba.org>
>
> --
> Best regards,
> Pavel Shilovsky

  reply	other threads:[~2021-07-08 23:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-07 23:24 [PATCH 0/1] cifs: use helpers when parsing uid/gid mount options and Ronnie Sahlberg
2021-07-07 23:24 ` [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them Ronnie Sahlberg
2021-07-08  0:57   ` Steve French
2021-07-08 22:11   ` Pavel Shilovsky
2021-07-08 23:25     ` ronnie sahlberg [this message]
2021-07-09  0:20       ` Steve French

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='CAN05THRzn29As=NO6vNyGyJocuJx4ddiuZeXQ05g2Am6Z+2GjA@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 \
    --subject='Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them' \
    /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

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