| Message ID | 1efcd842-b6a3-353a-0bf9-3ebf890eb712@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] cifs: reinstate original behavior again for forceuid/forcegid | expand |
Looks good. The question arises, are there any situations where forceuid is meaningful without uid= argument and what would it mean? On Fri, 7 Apr 2023 at 15:09, Takayuki Nagata <tnagata@redhat.com> wrote: > > forceuid/forcegid should be enabled by default when uid=/gid= options are > specified, but commit 24e0a1eff9e2 ("cifs: switch to new mount api") > changed the behavior. Due to the change, a mounted share does not show > intentional uid/gid for files and directories even though uid=/gid= > options are specified since forceuid/forcegid are not enabled. > > This patch reinstates original behavior that overrides uid/gid with > specified uid/gid by the options. > > Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api") > Signed-off-by: Takayuki Nagata <tnagata@redhat.com> > Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> > Acked-by: Tom Talpey <tom@talpey.com> > Signed-off-by: Steve French <stfrench@microsoft.com> > --- > V1 -> V2: Revised commit message to clarify "what breaks". > > fs/cifs/fs_context.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c > index ace11a1a7c8a..6f7c5ca3764f 100644 > --- a/fs/cifs/fs_context.c > +++ b/fs/cifs/fs_context.c > @@ -972,6 +972,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > goto cifs_parse_mount_err; > ctx->linux_uid = uid; > ctx->uid_specified = true; > + ctx->override_uid = 1; > break; > case Opt_cruid: > uid = make_kuid(current_user_ns(), result.uint_32); > @@ -1000,6 +1001,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > goto cifs_parse_mount_err; > ctx->linux_gid = gid; > ctx->gid_specified = true; > + ctx->override_gid = 1; > break; > case Opt_port: > ctx->port = result.uint_32; > -- > 2.40.0 >
I do not think that forceuid is meaningful without uid=, because the admin does not intend to override uid for files on the share. So specifying forceuid without uid= shows "ignoring forceuid mount option specified with no uid= option", and forceuid is disabled (=noforceuid is enabled). Then any uid provided by the server should be used for the files if the server provides it. Takayuki Nagata 2023年4月7日(金) 19:14 ronnie sahlberg <ronniesahlberg@gmail.com>: > > Looks good. > The question arises, are there any situations where forceuid is > meaningful without uid= argument and what would it mean? > > On Fri, 7 Apr 2023 at 15:09, Takayuki Nagata <tnagata@redhat.com> wrote: > > > > forceuid/forcegid should be enabled by default when uid=/gid= options are > > specified, but commit 24e0a1eff9e2 ("cifs: switch to new mount api") > > changed the behavior. Due to the change, a mounted share does not show > > intentional uid/gid for files and directories even though uid=/gid= > > options are specified since forceuid/forcegid are not enabled. > > > > This patch reinstates original behavior that overrides uid/gid with > > specified uid/gid by the options. > > > > Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api") > > Signed-off-by: Takayuki Nagata <tnagata@redhat.com> > > Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> > > Acked-by: Tom Talpey <tom@talpey.com> > > Signed-off-by: Steve French <stfrench@microsoft.com> > > --- > > V1 -> V2: Revised commit message to clarify "what breaks". > > > > fs/cifs/fs_context.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c > > index ace11a1a7c8a..6f7c5ca3764f 100644 > > --- a/fs/cifs/fs_context.c > > +++ b/fs/cifs/fs_context.c > > @@ -972,6 +972,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > > goto cifs_parse_mount_err; > > ctx->linux_uid = uid; > > ctx->uid_specified = true; > > + ctx->override_uid = 1; > > break; > > case Opt_cruid: > > uid = make_kuid(current_user_ns(), result.uint_32); > > @@ -1000,6 +1001,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > > goto cifs_parse_mount_err; > > ctx->linux_gid = gid; > > ctx->gid_specified = true; > > + ctx->override_gid = 1; > > break; > > case Opt_port: > > ctx->port = result.uint_32; > > -- > > 2.40.0 > > >
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c index ace11a1a7c8a..6f7c5ca3764f 100644 --- a/fs/cifs/fs_context.c +++ b/fs/cifs/fs_context.c @@ -972,6 +972,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, goto cifs_parse_mount_err; ctx->linux_uid = uid; ctx->uid_specified = true; + ctx->override_uid = 1; break; case Opt_cruid: uid = make_kuid(current_user_ns(), result.uint_32); @@ -1000,6 +1001,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, goto cifs_parse_mount_err; ctx->linux_gid = gid; ctx->gid_specified = true; + ctx->override_gid = 1; break; case Opt_port: ctx->port = result.uint_32;
forceuid/forcegid should be enabled by default when uid=/gid= options are specified, but commit 24e0a1eff9e2 ("cifs: switch to new mount api") changed the behavior. Due to the change, a mounted share does not show intentional uid/gid for files and directories even though uid=/gid= options are specified since forceuid/forcegid are not enabled. This patch reinstates original behavior that overrides uid/gid with specified uid/gid by the options. Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api") Signed-off-by: Takayuki Nagata <tnagata@redhat.com> Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> Acked-by: Tom Talpey <tom@talpey.com> Signed-off-by: Steve French <stfrench@microsoft.com> --- V1 -> V2: Revised commit message to clarify "what breaks". fs/cifs/fs_context.c | 2 ++ 1 file changed, 2 insertions(+)