Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them

[ronnie sahlberg <ronniesahlberg@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