[PATCH] cifs: update new ACE pointer after populate_new_aces.

[PATCH] cifs: update new ACE pointer after populate_new_aces.

[Shyam Prasad N]

[-- Attachment #1: Type: text/plain, Size: 198 bytes --]

Hi Steve,

Please consider this fix for the failing generic/317 test for cifsacl
and idsfromsid,modefromsid.
This is exposed on certain combinations of default ACEs on the file.

-- 
Regards,
Shyam

[-- Attachment #2: 0001-cifs-update-new-ACE-pointer-after-populate_new_aces.patch --]
[-- Type: application/octet-stream, Size: 2013 bytes --]

From 8e206cd0dcf27e5c1885c56d318b205cb62a18cf Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Wed, 10 Mar 2021 10:22:27 +0000
Subject: [PATCH] cifs: update new ACE pointer after populate_new_aces.

After the fix for retaining externally set ACEs with cifsacl and
modefromsid,idsfromsid, there was an issue in populating the
inherited ACEs after setting the ACEs introduced by these two modes.
Fixed this by updating the ACE pointer again after the call to
populate_new_aces.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/cifs/cifsacl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 9d29eb9660c2..2be22a5c690f 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -1118,7 +1118,6 @@ static int set_chmod_dacl(struct cifs_acl *pdacl, struct cifs_acl *pndacl,
 	/* Retain old ACEs which we can retain */
 	for (i = 0; i < src_num_aces; ++i) {
 		pntace = (struct cifs_ace *) (acl_base + size);
-		pnntace = (struct cifs_ace *) (nacl_base + nsize);
 
 		if (!new_aces_set && (pntace->flags & INHERITED_ACE)) {
 			/* Place the new ACEs in between existing explicit and inherited */
@@ -1131,14 +1130,18 @@ static int set_chmod_dacl(struct cifs_acl *pdacl, struct cifs_acl *pndacl,
 		}
 
 		/* If it's any one of the ACE we're replacing, skip! */
-		if ((compare_sids(&pntace->sid, &sid_unix_NFS_mode) == 0) ||
+		if (!mode_from_sid &&
+				((compare_sids(&pntace->sid, &sid_unix_NFS_mode) == 0) ||
 				(compare_sids(&pntace->sid, pownersid) == 0) ||
 				(compare_sids(&pntace->sid, pgrpsid) == 0) ||
 				(compare_sids(&pntace->sid, &sid_everyone) == 0) ||
-				(compare_sids(&pntace->sid, &sid_authusers) == 0)) {
+				(compare_sids(&pntace->sid, &sid_authusers) == 0))) {
 			goto next_ace;
 		}
 
+		/* update the pointer to the next ACE to populate*/
+		pnntace = (struct cifs_ace *) (nacl_base + nsize);
+
 		nsize += cifs_copy_ace(pnntace, pntace, NULL);
 		num_aces++;
 
-- 
2.25.1

Re: [PATCH] cifs: update new ACE pointer after populate_new_aces.

[Rohith Surabattula]

Looks good to me.
Reviewed-by: Rohith Surabattula <rohiths@microsoft.com>

On Wed, Mar 10, 2021 at 11:14 PM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> Hi Steve,
>
> Please consider this fix for the failing generic/317 test for cifsacl
> and idsfromsid,modefromsid.
> This is exposed on certain combinations of default ACEs on the file.
>
> --
> Regards,
> Shyam

Re: [PATCH] cifs: update new ACE pointer after populate_new_aces.

[Steve French]

Test 317 will require additional changes for test 317 when run with
cifsacl (to map ownership) or will need to be run with winbind or sssd
- but I verified that it does fix test 317 for idsfromsid,modefromsid

On Wed, Mar 10, 2021 at 11:58 AM Rohith Surabattula
<rohiths.msft@gmail.com> wrote:
>
> Looks good to me.
> Reviewed-by: Rohith Surabattula <rohiths@microsoft.com>
>
> On Wed, Mar 10, 2021 at 11:14 PM Shyam Prasad N <nspmangalore@gmail.com> wrote:
> >
> > Hi Steve,
> >
> > Please consider this fix for the failing generic/317 test for cifsacl
> > and idsfromsid,modefromsid.
> > This is exposed on certain combinations of default ACEs on the file.
> >
> > --
> > Regards,
> > Shyam



-- 
Thanks,

Steve