diff mbox series

cifs: reinstate original behavior again for forceuid/forcegid

Message ID 6cf163fe-a974-68ab-0edc-11ebc54314ef@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: reinstate original behavior again for forceuid/forcegid | expand

Commit Message

Takayuki Nagata April 6, 2023, 12:02 p.m. UTC
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. This patch reinstates original behavior to overriding
uid/gid with specified uid/gid.

Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
Signed-off-by: Takayuki Nagata <tnagata@redhat.com>
---
 fs/cifs/fs_context.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Tom Talpey April 6, 2023, 2:31 p.m. UTC | #1
On 4/6/2023 8:02 AM, Takayuki Nagata 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. This patch reinstates original behavior to overriding
> uid/gid with specified uid/gid.
> 
> Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")

This looks correct, but I'd love to hear Ronnie's call. Looking at
that commit, was it simply an oversight to set override_{uid,gid}?
It kind of looks that way, but...

Tom.

> Signed-off-by: Takayuki Nagata <tnagata@redhat.com>
> ---
>   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;
ronnie sahlberg April 6, 2023, 6:38 p.m. UTC | #2
Yeah, an oversight.
Acked-by me

On Fri, 7 Apr 2023 at 00:32, Tom Talpey via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> On 4/6/2023 8:02 AM, Takayuki Nagata 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. This patch reinstates original behavior to overriding
> > uid/gid with specified uid/gid.
> >
> > Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
>
> This looks correct, but I'd love to hear Ronnie's call. Looking at
> that commit, was it simply an oversight to set override_{uid,gid}?
> It kind of looks that way, but...
>
> Tom.
>
> > Signed-off-by: Takayuki Nagata <tnagata@redhat.com>
> > ---
> >   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;
>
Steve French April 7, 2023, 4:13 a.m. UTC | #3
Tentatively merged into cifs-2.6.git for-next

Any thoughts on priority sending it upstream soon?

On Thu, Apr 6, 2023 at 7:06 AM 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. This patch reinstates original behavior to overriding
> uid/gid with specified uid/gid.
>
> Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
> Signed-off-by: Takayuki Nagata <tnagata@redhat.com>
> ---
>  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
>
Takayuki Nagata April 7, 2023, 4:42 a.m. UTC | #4
Thank you for reviewing my patch.
I have no thoughts on the priority. So If it does not bother you to
send the v2 patch, I will resend the v2 patch soon with a revised
commit message to clarify "what breaks".

Takayuki Nagata

2023年4月7日(金) 13:13 Steve French <smfrench@gmail.com>:
>
> Tentatively merged into cifs-2.6.git for-next
>
> Any thoughts on priority sending it upstream soon?
>
> On Thu, Apr 6, 2023 at 7:06 AM 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. This patch reinstates original behavior to overriding
> > uid/gid with specified uid/gid.
> >
> > Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
> > Signed-off-by: Takayuki Nagata <tnagata@redhat.com>
> > ---
> >  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
> >
>
>
> --
> Thanks,
>
> Steve
>
diff mbox series

Patch

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;