From: Jeff Layton <jlayton@redhat.com> Date: Wed, 5 Aug 2009 11:15:27 -0400 Subject: [fs] cifs: new opts to disable overriding of ownership Message-id: 1249485327-20613-1-git-send-email-jlayton@redhat.com O-Subject: [RHEL5.5 PATCH] BZ#515252: cifs: add new options to disable overriding of ownership Bugzilla: 515252 RH-Acked-by: Peter Staubach <staubach@redhat.com> CIFS has 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 SPNEGO blob for krb5 auth. 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 behavior also makes MultiuserMount "feature" less usable. Once you pass in the uid= option for a mount (which is needed to allow CIFS to match the uid of a process to a SMB session), then you can't use unix ownership info and allow someone to share the mount. My initial attempt to fix this upstream disabled the automatic override of ownership when the server provides it, and added new options to enable it for people that wanted it. This was considered a regression however and the default had to be reverted. The option names remain the same however. This patch backports these options for RHEL-5. In addition to adding the new options, this patch also contains a cleanup of cifs_show_options that removes some unneeded NULL pointer checks. Tested myself by mounting a share and using the various option combinations. An earlier version of the patch was also tested by the bug reporter who stated that it fixed the problem they were having with the MultiuserMount feature. diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 4b2bc75..68ae5d8 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -378,80 +378,75 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m) struct TCP_Server_Info *server; cifs_sb = CIFS_SB(m->mnt_sb); + tcon = cifs_sb->tcon; - if (cifs_sb) { - tcon = cifs_sb->tcon; - if (tcon) { - seq_printf(s, ",unc=%s", cifs_sb->tcon->treeName); - if (tcon->ses) { - if (tcon->ses->userName) - seq_printf(s, ",username=%s", - tcon->ses->userName); - if (tcon->ses->domainName) - seq_printf(s, ",domain=%s", - tcon->ses->domainName); - server = tcon->ses->server; - if (server) { - seq_printf(s, ",addr="); - switch (server->addr.sockAddr6. - sin6_family) { - case AF_INET6: - seq_printf(s, NIP6_FMT, - NIP6(server->addr.sockAddr6.sin6_addr)); - break; - case AF_INET: - seq_printf(s, NIPQUAD_FMT, - NIPQUAD(server->addr.sockAddr.sin_addr.s_addr)); - break; - } - } - } - if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID) || - !(tcon->unix_ext)) - seq_printf(s, ",uid=%d", cifs_sb->mnt_uid); - if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_GID) || - !(tcon->unix_ext)) - seq_printf(s, ",gid=%d", cifs_sb->mnt_gid); - if (!tcon->unix_ext) { - seq_printf(s, ",file_mode=0%o,dir_mode=0%o", + seq_printf(s, ",unc=%s", cifs_sb->tcon->treeName); + if (tcon->ses->userName) + seq_printf(s, ",username=%s", tcon->ses->userName); + if (tcon->ses->domainName) + seq_printf(s, ",domain=%s", tcon->ses->domainName); + + seq_printf(s, ",uid=%d", cifs_sb->mnt_uid); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID) + seq_printf(s, ",forceuid"); + else + seq_printf(s, ",noforceuid"); + + seq_printf(s, ",gid=%d", cifs_sb->mnt_gid); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_GID) + seq_printf(s, ",forcegid"); + else + seq_printf(s, ",noforcegid"); + + server = tcon->ses->server; + seq_printf(s, ",addr="); + switch (server->addr.sockAddr6.sin6_family) { + case AF_INET6: + seq_printf(s, NIP6_FMT, NIP6(server->addr.sockAddr6.sin6_addr)); + break; + case AF_INET: + seq_printf(s, NIPQUAD_FMT, NIPQUAD(server->addr.sockAddr.sin_addr.s_addr)); + break; + } + + if (!tcon->unix_ext) + seq_printf(s, ",file_mode=0%o,dir_mode=0%o", cifs_sb->mnt_file_mode, cifs_sb->mnt_dir_mode); - } - if (tcon->seal) - seq_printf(s, ",seal"); - if (tcon->nocase) - seq_printf(s, ",nocase"); - if (tcon->retry) - seq_printf(s, ",hard"); - } - if (cifs_sb->prepath) - seq_printf(s, ",prepath=%s", cifs_sb->prepath); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) - seq_printf(s, ",posixpaths"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) - seq_printf(s, ",setuids"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) - seq_printf(s, ",serverino"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO) - seq_printf(s, ",directio"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR) - seq_printf(s, ",nouser_xattr"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR) - seq_printf(s, ",mapchars"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) - seq_printf(s, ",sfu"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL) - seq_printf(s, ",nobrl"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) - seq_printf(s, ",cifsacl"); - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM) - seq_printf(s, ",dynperm"); - if (m->mnt_sb->s_flags & MS_POSIXACL) - seq_printf(s, ",acl"); - - seq_printf(s, ",rsize=%d", cifs_sb->rsize); - seq_printf(s, ",wsize=%d", cifs_sb->wsize); - } + if (tcon->seal) + seq_printf(s, ",seal"); + if (tcon->nocase) + seq_printf(s, ",nocase"); + if (tcon->retry) + seq_printf(s, ",hard"); + if (cifs_sb->prepath) + seq_printf(s, ",prepath=%s", cifs_sb->prepath); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) + seq_printf(s, ",posixpaths"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) + seq_printf(s, ",setuids"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) + seq_printf(s, ",serverino"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO) + seq_printf(s, ",directio"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR) + seq_printf(s, ",nouser_xattr"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR) + seq_printf(s, ",mapchars"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) + seq_printf(s, ",sfu"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL) + seq_printf(s, ",nobrl"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) + seq_printf(s, ",cifsacl"); + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM) + seq_printf(s, ",dynperm"); + if (m->mnt_sb->s_flags & MS_POSIXACL) + seq_printf(s, ",acl"); + + seq_printf(s, ",rsize=%d", cifs_sb->rsize); + seq_printf(s, ",wsize=%d", cifs_sb->wsize); + return 0; } diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index ff91010..b7d5897 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -857,6 +857,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; separator[0] = ','; separator[1] = 0; @@ -1141,18 +1145,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); - vol->override_uid = 1; - } - } else if (strnicmp(data, "gid", 3) == 0) { - if (value && *value) { - vol->linux_gid = - simple_strtoul(value, &value, 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 = @@ -1409,6 +1415,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; }