* [PATCH 0/1] cifs: use helpers when parsing uid/gid mount options and
@ 2021-07-07 23:24 Ronnie Sahlberg
2021-07-07 23:24 ` [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them Ronnie Sahlberg
0 siblings, 1 reply; 6+ messages in thread
From: Ronnie Sahlberg @ 2021-07-07 23:24 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
Steve,
This is a small patch to use the existing helpers to validate the uid/gid
values that can be passed as mount arguments.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
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 ` Ronnie Sahlberg
2021-07-08 0:57 ` Steve French
2021-07-08 22:11 ` Pavel Shilovsky
0 siblings, 2 replies; 6+ messages in thread
From: Ronnie Sahlberg @ 2021-07-07 23:24 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
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;
bool gid_specified;
bool sloppy;
bool got_ip;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
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
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2021-07-08 0:57 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
tentatively merged into cifs-2.6.git pending testing
On Wed, Jul 7, 2021 at 6:24 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> 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;
> bool gid_specified;
> bool sloppy;
> bool got_ip;
> --
> 2.30.2
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
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
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2021-07-08 22:11 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs, Steve French
ср, 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?
> bool gid_specified;
> bool sloppy;
> bool got_ip;
> --
> 2.30.2
>
Acked-by: Pavel Shilovsky <pshilovsky@samba.org>
--
Best regards,
Pavel Shilovsky
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
2021-07-08 22:11 ` Pavel Shilovsky
@ 2021-07-08 23:25 ` ronnie sahlberg
2021-07-09 0:20 ` Steve French
0 siblings, 1 reply; 6+ messages in thread
From: ronnie sahlberg @ 2021-07-08 23:25 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: Ronnie Sahlberg, linux-cifs, Steve French
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: use helpers when parsing uid/gid mount options and validate them
2021-07-08 23:25 ` ronnie sahlberg
@ 2021-07-09 0:20 ` Steve French
0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2021-07-09 0:20 UTC (permalink / raw)
To: ronnie sahlberg; +Cc: Pavel Shilovsky, Ronnie Sahlberg, linux-cifs
mildy easier to leave it in at the moment - if you want to remove it
later with a followon that is fine too
On Thu, Jul 8, 2021 at 6:25 PM ronnie sahlberg <ronniesahlberg@gmail.com> wrote:
>
> 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
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-09 0:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-07-09 0:20 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.