[PATCH] cifs: reinstate the forcegid option

[PATCH] cifs: reinstate the forcegid option

[Jeff Layton]

Apparently this was lost when we converted to the standard option
parser.

Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/connect.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 2fdbe08..5c670b9 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -67,6 +67,7 @@ enum {
 	/* Mount options that take no arguments */
 	Opt_user_xattr, Opt_nouser_xattr,
 	Opt_forceuid, Opt_noforceuid,
+	Opt_forcegid, Opt_noforcegid,
 	Opt_noblocksend, Opt_noautotune,
 	Opt_hard, Opt_soft, Opt_perm, Opt_noperm,
 	Opt_mapchars, Opt_nomapchars, Opt_sfu,
@@ -117,6 +118,8 @@ static const match_table_t cifs_mount_option_tokens = {
 	{ Opt_nouser_xattr, "nouser_xattr" },
 	{ Opt_forceuid, "forceuid" },
 	{ Opt_noforceuid, "noforceuid" },
+	{ Opt_forcegid, "forcegid" },
+	{ Opt_noforcegid, "noforcegid" },
 	{ Opt_noblocksend, "noblocksend" },
 	{ Opt_noautotune, "noautotune" },
 	{ Opt_hard, "hard" },
@@ -1195,6 +1198,12 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 		case Opt_noforceuid:
 			override_uid = 0;
 			break;
+		case Opt_forcegid:
+			override_gid = 1;
+			break;
+		case Opt_noforcegid:
+			override_gid = 0;
+			break;
 		case Opt_noblocksend:
 			vol->noblocksnd = 1;
 			break;
-- 
1.7.11.4

Re: [PATCH] cifs: reinstate the forcegid option

[Sachin Prabhu]

On Wed, 2012-10-03 at 16:02 -0400, Jeff Layton wrote:
> Apparently this was lost when we converted to the standard option
> parser.
> 
> Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

> ---
>  fs/cifs/connect.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 2fdbe08..5c670b9 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -67,6 +67,7 @@ enum {
>  	/* Mount options that take no arguments */
>  	Opt_user_xattr, Opt_nouser_xattr,
>  	Opt_forceuid, Opt_noforceuid,
> +	Opt_forcegid, Opt_noforcegid,
>  	Opt_noblocksend, Opt_noautotune,
>  	Opt_hard, Opt_soft, Opt_perm, Opt_noperm,
>  	Opt_mapchars, Opt_nomapchars, Opt_sfu,
> @@ -117,6 +118,8 @@ static const match_table_t cifs_mount_option_tokens = {
>  	{ Opt_nouser_xattr, "nouser_xattr" },
>  	{ Opt_forceuid, "forceuid" },
>  	{ Opt_noforceuid, "noforceuid" },
> +	{ Opt_forcegid, "forcegid" },
> +	{ Opt_noforcegid, "noforcegid" },
>  	{ Opt_noblocksend, "noblocksend" },
>  	{ Opt_noautotune, "noautotune" },
>  	{ Opt_hard, "hard" },
> @@ -1195,6 +1198,12 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  		case Opt_noforceuid:
>  			override_uid = 0;
>  			break;
> +		case Opt_forcegid:
> +			override_gid = 1;
> +			break;
> +		case Opt_noforcegid:
> +			override_gid = 0;
> +			break;
>  		case Opt_noblocksend:
>  			vol->noblocksnd = 1;
>  			break;

Re: [PATCH] cifs: reinstate the forcegid option

[Suresh Jayaraman]

On 10/04/2012 01:32 AM, Jeff Layton wrote:
> Apparently this was lost when we converted to the standard option
> parser.
> 
> Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---

Oops, atleast a couple of options have been reported missing and fixed
so far. Time to audit the new parsing code so that we fix them
proactively before we get reports from users..?


Thanks
Suresh

Re: [PATCH] cifs: reinstate the forcegid option

[Scott Lovenberg]

On Thu, Oct 4, 2012 at 9:16 AM, Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
> On 10/04/2012 01:32 AM, Jeff Layton wrote:
>> Apparently this was lost when we converted to the standard option
>> parser.
>>
>> Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>
> Oops, atleast a couple of options have been reported missing and fixed
> so far. Time to audit the new parsing code so that we fix them
> proactively before we get reports from users..?
>
>
> Thanks
> Suresh
>

Seems like every time we play with the parsing code an option gets
dropped.  It's difficult though because there are over 80 options at
this point.  :/
I'll take a look over the code too.

-- 
Peace and Blessings,
-Scott.

Re: [PATCH] cifs: reinstate the forcegid option

[Sachin Prabhu]

On Thu, 2012-10-04 at 18:46 +0530, Suresh Jayaraman wrote:
> On 10/04/2012 01:32 AM, Jeff Layton wrote:
> > Apparently this was lost when we converted to the standard option
> > parser.
> > 
> > Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> 
> Oops, atleast a couple of options have been reported missing and fixed
> so far. Time to audit the new parsing code so that we fix them
> proactively before we get reports from users..?
> 

I am taking another look at the code to compare the options supported
against the old parser with the new parser. Some gotchas which tripped
me last time were checks for shortened options which also matched the
longer versions of the mount options.

We should also start thinking about paring down the number of mount
options which in my opinion are on the high side.

Sachin Prabhu

Re: [PATCH] cifs: reinstate the forcegid option

[Sachin Prabhu]

On Thu, 2012-10-04 at 15:50 +0100, Sachin Prabhu wrote:
> On Thu, 2012-10-04 at 18:46 +0530, Suresh Jayaraman wrote:
> > On 10/04/2012 01:32 AM, Jeff Layton wrote:
> > > Apparently this was lost when we converted to the standard option
> > > parser.
> > > 
> > > Cc: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Reported-by: Gregory Lee Bartholomew <gregory.lee.bartholomew-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > ---
> > 
> > Oops, atleast a couple of options have been reported missing and fixed
> > so far. Time to audit the new parsing code so that we fix them
> > proactively before we get reports from users..?
> > 
> 
> I am taking another look at the code to compare the options supported
> against the old parser with the new parser. Some gotchas which tripped
> me last time were checks for shortened options which also matched the
> longer versions of the mount options.
> 
> We should also start thinking about paring down the number of mount
> options which in my opinion are on the high side.

I've done a quick check by parsing the older cifs_parse_mount_options
to print out the list of mount options supported and compared this
output to the list of mount options supported by the current parser and
it appears that following the forcegid/noforcegid option, we have
accounted for all the other mount options.

There have been a number of mount options which have changed since.
example: cache, vers. These have been accounted for.

Sachin Prabhu