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