| Message ID | 20210707232416.2694911-2-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: use helpers when parsing uid/gid mount options and | expand |
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 >
ср, 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
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
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
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;
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(-)