On 11/13/24 10:49 PM, Jeremy Allison wrote:
On Thu, Nov 14, 2024 at 06:52:14AM +1000, ronnie sahlberg wrote:
On Thu, 14 Nov 2024 at 05:07, Ralph Boehme <slow@xxxxxxxxx> wrote:
On 11/13/24 4:39 PM, Ralph Boehme wrote:
> Am I missing anything? Thoughts?
did some more research on what the option modefromsid actually does and
I guess the problem is that the behaviour is likely correct for
modefromsid, it just doesn't work for smb3 posix, so populate_new_aces()
needs to be tweaked to not include sid_authusers.
Remember, it also need to work for use-cases with normal Windows and
Azure servers where you do NOT
have a multiuser mount (i.e. all client access is using the
credentials from the mount)
and you basically have an inherited ACE to "allow all access to the
mount user" for the whole share.
This is different from the SMB3-POSIX chmod operation.
I think Ralph is suggesting the modefromsid is left alone,
and a new (separate) code path used for SMB3-POSIX that
sends the one-ACE entry defined in that spec.
exactly.
I've implemented a quick fix and am currently testing it.
Thanks!
-slow
From 7b0aa527e693d1339122897c4cdec650d6826147 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@xxxxxxxxx>
Date: Thu, 14 Nov 2024 11:05:13 +0100
Subject: [PATCH] fs/smb/client: implement chmod() for SMB3 POSIX Extensions
---
fs/smb/client/cifsacl.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 1d294d53f662..e5237c62a440 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -933,7 +933,8 @@ static void populate_new_aces(char *nacl_base,
struct smb_sid *pownersid,
struct smb_sid *pgrpsid,
__u64 *pnmode, u32 *pnum_aces, u16 *pnsize,
- bool modefromsid)
+ bool modefromsid,
+ bool posix)
{
__u64 nmode;
u32 num_aces = 0;
@@ -950,13 +951,15 @@ static void populate_new_aces(char *nacl_base,
num_aces = *pnum_aces;
nsize = *pnsize;
- if (modefromsid) {
+ if (modefromsid || posix) {
pnntace = (struct smb_ace *) (nacl_base + nsize);
nsize += setup_special_mode_ACE(pnntace, nmode);
num_aces++;
- pnntace = (struct smb_ace *) (nacl_base + nsize);
- nsize += setup_authusers_ACE(pnntace);
- num_aces++;
+ if (modefromsid) {
+ pnntace = (struct smb_ace *) (nacl_base + nsize);
+ nsize += setup_authusers_ACE(pnntace);
+ num_aces++;
+ }
goto set_size;
}
@@ -1076,7 +1079,7 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *p
static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
struct smb_sid *pownersid, struct smb_sid *pgrpsid,
- __u64 *pnmode, bool mode_from_sid)
+ __u64 *pnmode, bool mode_from_sid, bool posix)
{
int i;
u16 size = 0;
@@ -1098,7 +1101,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
populate_new_aces(nacl_base,
pownersid, pgrpsid,
pnmode, &num_aces, &nsize,
- mode_from_sid);
+ mode_from_sid, posix);
goto finalize_dacl;
}
@@ -1115,7 +1118,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
populate_new_aces(nacl_base,
pownersid, pgrpsid,
pnmode, &num_aces, &nsize,
- mode_from_sid);
+ mode_from_sid, posix);
new_aces_set = true;
}
@@ -1144,7 +1147,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
populate_new_aces(nacl_base,
pownersid, pgrpsid,
pnmode, &num_aces, &nsize,
- mode_from_sid);
+ mode_from_sid, posix);
new_aces_set = true;
}
@@ -1251,7 +1254,7 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
/* Convert permission bits from mode to equivalent CIFS ACL */
static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
__u32 secdesclen, __u32 *pnsecdesclen, __u64 *pnmode, kuid_t uid, kgid_t gid,
- bool mode_from_sid, bool id_from_sid, int *aclflag)
+ bool mode_from_sid, bool id_from_sid, bool posix, int *aclflag)
{
int rc = 0;
__u32 dacloffset;
@@ -1288,7 +1291,7 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
ndacl_ptr->num_aces = cpu_to_le32(0);
rc = set_chmod_dacl(dacl_ptr, ndacl_ptr, owner_sid_ptr, group_sid_ptr,
- pnmode, mode_from_sid);
+ pnmode, mode_from_sid, posix);
sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
/* copy the non-dacl portion of secdesc */
@@ -1587,6 +1590,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
struct smb_version_operations *ops;
bool mode_from_sid, id_from_sid;
+ bool posix = tlink_tcon(tlink)->posix_extensions;
const u32 info = 0;
if (IS_ERR(tlink))
@@ -1622,12 +1626,13 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
id_from_sid = false;
/* Potentially, five new ACEs can be added to the ACL for U,G,O mapping */
- nsecdesclen = secdesclen;
if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */
- if (mode_from_sid)
- nsecdesclen += 2 * sizeof(struct smb_ace);
+ if (posix)
+ nsecdesclen = 1 * sizeof(struct smb_ace);
+ else if (mode_from_sid)
+ nsecdesclen = secdesclen + (2 * sizeof(struct smb_ace));
else /* cifsacl */
- nsecdesclen += 5 * sizeof(struct smb_ace);
+ nsecdesclen = secdesclen + (5 * sizeof(struct smb_ace));
} else { /* chown */
/* When ownership changes, changes new owner sid length could be different */
nsecdesclen = sizeof(struct smb_ntsd) + (sizeof(struct smb_sid) * 2);
@@ -1657,7 +1662,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
}
rc = build_sec_desc(pntsd, pnntsd, secdesclen, &nsecdesclen, pnmode, uid, gid,
- mode_from_sid, id_from_sid, &aclflag);
+ mode_from_sid, id_from_sid, posix, &aclflag);
cifs_dbg(NOISY, "build_sec_desc rc: %d\n", rc);
--
2.47.0
