Re: [linux-cifs-client] [PATCH] cifs: reinstate original behavior when uid=/gid= options are specified

[Shirish Pargaonkar <shirishpargaonkar@gmail.com>]

On Fri, Jul 31, 2009 at 5:55 AM, Jeff Layton<jlayton@redhat.com> wrote:
> This patch fixes the regression reported here:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=13861
>
> commit 4ae1507f6d266d0cc3dd36e474d83aad70fec9e4 changed the default
> behavior when the uid= or gid= option was specified for a mount. The
> existing behavior was to always clobber the ownership information
> provided by the server when these options were specified. The above
> commit changed this behavior so that these options simply provided
> defaults when the server did not provide this information.
>
> This patch reverts this change so that the default behavior is restored.
> It also adds "noforceuid" and "noforcegid" options to make it so that
> ownership information from the server is preserved, even when the mount
> has uid= or gid= options specified.
>
> It also adds a couple of printk notices that pop up when forceuid or
> forcegid options are specified without a uid= or gid= option.
>
> Reported-by: Tom Chiverton <bugzilla.kernel.org@falkensweb.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c |   42 ++++++++++++++++++++++++++++++------------
>  1 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index f248688..1f3345d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -803,6 +803,10 @@ cifs_parse_mount_options(char *options, const char *devname,
>        char *data;
>        unsigned int  temp_len, i, j;
>        char separator[2];
> +       short int override_uid = -1;
> +       short int override_gid = -1;
> +       bool uid_specified = false;
> +       bool gid_specified = false;

Should override_uid  and override_gid be initialized to 1 so that if
uid and/or gid
are specified and none of the options, either forceuid/gid and or
noforceuid/gid are
specified, the vol-> override defaults to 1 (i.e. forceuid/gid)?

>
>        separator[0] = ',';
>        separator[1] = 0;
> @@ -1093,18 +1097,20 @@ cifs_parse_mount_options(char *options, const char *devname,
>                                                    "too long.\n");
>                                return 1;
>                        }
> -               } else if (strnicmp(data, "uid", 3) == 0) {
> -                       if (value && *value)
> -                               vol->linux_uid =
> -                                       simple_strtoul(value, &value, 0);
> -               } else if (strnicmp(data, "forceuid", 8) == 0) {
> -                               vol->override_uid = 1;
> -               } else if (strnicmp(data, "gid", 3) == 0) {
> -                       if (value && *value)
> -                               vol->linux_gid =
> -                                       simple_strtoul(value, &value, 0);
> -               } else if (strnicmp(data, "forcegid", 8) == 0) {
> -                               vol->override_gid = 1;
> +               } else if (!strnicmp(data, "uid", 3) && value && *value) {
> +                       vol->linux_uid = simple_strtoul(value, &value, 0);
> +                       uid_specified = true;
> +               } else if (!strnicmp(data, "forceuid", 8)) {
> +                       override_uid = 1;
> +               } else if (!strnicmp(data, "noforceuid", 10)) {
> +                       override_uid = 0;
> +               } else if (!strnicmp(data, "gid", 3) && value && *value) {
> +                       vol->linux_gid = simple_strtoul(value, &value, 0);
> +                       gid_specified = true;
> +               } else if (!strnicmp(data, "forcegid", 8)) {
> +                       override_gid = 1;
> +               } else if (!strnicmp(data, "noforcegid", 10)) {
> +                       override_gid = 0;
>                } else if (strnicmp(data, "file_mode", 4) == 0) {
>                        if (value && *value) {
>                                vol->file_mode =
> @@ -1355,6 +1361,18 @@ cifs_parse_mount_options(char *options, const char *devname,
>        if (vol->UNCip == NULL)
>                vol->UNCip = &vol->UNC[2];
>
> +       if (uid_specified)
> +               vol->override_uid = override_uid;
> +       else if (override_uid == 1)
> +               printk(KERN_NOTICE "CIFS: ignoring forceuid mount option "
> +                                  "specified with no uid= option.\n");
> +
> +       if (gid_specified)
> +               vol->override_gid = override_gid;
> +       else if (override_gid == 1)
> +               printk(KERN_NOTICE "CIFS: ignoring forcegid mount option "
> +                                  "specified with no gid= option.\n");
> +
>        return 0;
>  }
>
> --
> 1.6.0.6
>
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html