Re: [PATCH 1/3] cifs: make overriding of ownership conditional on new mount options

On Sun, 24 May 2009 18:45:15 -0400
Jeff Layton <[email protected]> wrote:

> We have a bit of a problem with the uid= option. The basic issue is that
> it means too many things and has too many side-effects.
> 
> It's possible to allow an unprivileged user to mount a filesystem if the
> user owns the mountpoint, /bin/mount is setuid root, and the mount is
> set up in /etc/fstab with the "user" option.
> 
> When doing this though, /bin/mount automatically adds the "uid=" and
> "gid=" options to the share. This is fortunate since the correct uid=
> option is needed in order to tell the upcall what user's credcache to
> use when generating the SPNEGO blob.
> 
> On a mount without unix extensions this is fine -- you generally will
> want the files to be owned by the "owner" of the mount. The problem
> comes in on a mount with unix extensions. With those enabled, the
> uid/gid options cause the ownership of files to be overriden even though
> the server is sending along the ownership info.
> 
> This means that it's not possible to have a mount by an unprivileged
> user that shows the server's file ownership info. The result is also
> inode permissions that have no reflection at all on the server. You
> simply cannot separate ownership from the mode in this fashion.
> 
> This behavior also makes MultiuserMount option less usable. Once you
> pass in the uid= option for a mount, then you can't use unix ownership
> info and allow someone to share the mount.
> 
> While I'm not thrilled with it, the only solution I can see is to stop
> making uid=/gid= force the overriding of ownership on mounts, and to add
> new mount options that turn this behavior on.
> 
> Signed-off-by: Jeff Layton <[email protected]>
> ---
>  fs/cifs/connect.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 4aa81a5..4f5a03c 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1092,17 +1092,17 @@ cifs_parse_mount_options(char *options, const char *devname,
>  				return 1;
>  			}
>  		} else if (strnicmp(data, "uid", 3) == 0) {
> -			if (value && *value) {
> +			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) {
> +			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, "file_mode", 4) == 0) {
>  			if (value && *value) {
>  				vol->file_mode =
> -- 
> 1.6.0.6
> 


Steve,
   Any word on this patch? I have a customer who is being impacted by
this bug. I need to bring it to some sort of resolution soon.

Thanks,
-- 
Jeff Layton <[email protected]>