I had a vague concern that some users with cifs.upcall idmapping
configured might choose to map SID->uid differently for these "well
known" SIDs, but the bigger question is whether mapping (in kernel
cifs.ko) "well known" SIDs to UIDs should be default behavior or not.
I am leaning toward making it the default, at least for SMB2.1 and
later, but would prefer that we try it for a release before we turn it
on by default. We can then add the "no" version of the mount option
to turn it off for that small subset of users that wants to map these
4 SIDs differently or for the subset of users that really wants all
owners to show up as the mount uid/gid.
On Fri, Oct 14, 2016 at 1:36 PM, Steve French <smfrench@xxxxxxxxx> wrote:
> I will do a man page update for this. We may want to change some of
> the SMB3 defaults as well as we get closer to finishing the POSIX
> extensions for SMB3, but so far they are reasonably close.
>
> On Fri, Oct 14, 2016 at 1:34 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> On Fri, 2016-10-14 at 10:10 -0500, Steve French wrote:
>>> Add "idsfromsid" mount option to indicate to cifs.ko that it should
>>> try to retrieve the uid and gid owner fields from special sids in the
>>> ACL if present. This first patch just adds the parsing for the mount
>>> option.
>>>
>>> Signed-off-by: Steve French <steve.french@xxxxxxxxxxxxxxx>
>>> Reviewed-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>> ---
>>> fs/cifs/cifs_fs_sb.h | 1 +
>>> fs/cifs/cifsfs.c | 2 ++
>>> fs/cifs/cifsglob.h | 1 +
>>> fs/cifs/connect.c | 8 +++++++-
>>> 4 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>>> index 1418daa..07ed81c 100644
>>> --- a/fs/cifs/cifs_fs_sb.h
>>> +++ b/fs/cifs/cifs_fs_sb.h
>>> @@ -49,6 +49,7 @@
>>> #define CIFS_MOUNT_USE_PREFIX_PATH 0x1000000 /* make subpath with unaccessible
>>> * root mountable
>>> */
>>> +#define CIFS_MOUNT_UID_FROM_ACL 0x2000000 /* try to get UID via special SID */
>>>
>>> struct cifs_sb_info {
>>> struct rb_root tlink_tree;
>>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>>> index f963c88..15261ba 100644
>>> --- a/fs/cifs/cifsfs.c
>>> +++ b/fs/cifs/cifsfs.c
>>> @@ -469,6 +469,8 @@ static void cifs_i_callback(struct rcu_head *head)
>>> seq_puts(s, ",posixpaths");
>>> if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID)
>>> seq_puts(s, ",setuids");
>>> + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UID_FROM_ACL)
>>> + seq_puts(s, ",idsfromsid");
>>> if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
>>> seq_puts(s, ",serverino");
>>> if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 0c828d3..1f17f6b 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -478,6 +478,7 @@ struct smb_vol {
>>> bool retry:1;
>>> bool intr:1;
>>> bool setuids:1;
>>> + bool setuidfromacl:1;
>>> bool override_uid:1;
>>> bool override_gid:1;
>>> bool dynperm:1;
>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>> index 657369d..aab5227 100644
>>> --- a/fs/cifs/connect.c
>>> +++ b/fs/cifs/connect.c
>>> @@ -75,7 +75,7 @@ enum {
>>> Opt_noposixpaths, Opt_nounix,
>>> Opt_nocase,
>>> Opt_brl, Opt_nobrl,
>>> - Opt_forcemandatorylock, Opt_setuids,
>>> + Opt_forcemandatorylock, Opt_setuidfromacl, Opt_setuids,
>>> Opt_nosetuids, Opt_dynperm, Opt_nodynperm,
>>> Opt_nohard, Opt_nosoft,
>>> Opt_nointr, Opt_intr,
>>> @@ -147,6 +147,7 @@ enum {
>>> { Opt_forcemandatorylock, "forcemand" },
>>> { Opt_setuids, "setuids" },
>>> { Opt_nosetuids, "nosetuids" },
>>> + { Opt_setuidfromacl, "idsfromsid" },
>>> { Opt_dynperm, "dynperm" },
>>> { Opt_nodynperm, "nodynperm" },
>>> { Opt_nohard, "nohard" },
>>> @@ -1376,6 +1377,9 @@ static int cifs_parse_security_flavors(char *value,
>>> case Opt_nosetuids:
>>> vol->setuids = 0;
>>> break;
>>> + case Opt_setuidfromacl:
>>> + vol->setuidfromacl = 1;
>>> + break;
>>> case Opt_dynperm:
>>> vol->dynperm = true;
>>> break;
>>> @@ -3279,6 +3283,8 @@ int cifs_setup_cifs_sb(struct smb_vol *pvolume_info,
>>> cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NO_PERM;
>>> if (pvolume_info->setuids)
>>> cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_SET_UID;
>>> + if (pvolume_info->setuidfromacl)
>>> + cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_UID_FROM_ACL;
>>> if (pvolume_info->server_ino)
>>> cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_SERVER_INUM;
>>> if (pvolume_info->remap)
>>
>> Is there any reason not to do this universally so we can avoid the new
>> mount option? If not, then can you also roll a patch to update the
>> mount.cifs manpage to document this?
>>
>> Thanks,
>> --
>> Jeff Layton <jlayton@xxxxxxxxx>
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
