On Thu, May 18, 2023 at 7:17 PM Joseph Koshakow <kosh...@gmail.com> wrote: > > I looked into this function and that is correct. We fail to find a > match for the revoked privilege here: > > /* > * Search the ACL for an existing entry for this grantee and grantor. If > * one exists, just modify the entry in-place (well, in the same position, > * since we actually return a copy); otherwise, insert the new entry at > * the end. > */ > > for (dst = 0; dst < num; ++dst) > { > if (aclitem_match(mod_aip, old_aip + dst)) > { > /* found a match, so modify existing item */ > new_acl = allocacl(num); > new_aip = ACL_DAT(new_acl); > memcpy(new_acl, old_acl, ACL_SIZE(old_acl)); > break; > } > } > > Seeing that there was no match, we add a new empty privilege to the end > of the existing ACL list here: > > if (dst == num) > { > /* need to append a new item */ > new_acl = allocacl(num + 1); > new_aip = ACL_DAT(new_acl); > memcpy(new_aip, old_aip, num * sizeof(AclItem)); > > /* initialize the new entry with no permissions */ > new_aip[dst].ai_grantee = mod_aip->ai_grantee; > new_aip[dst].ai_grantor = mod_aip->ai_grantor; > ACLITEM_SET_PRIVS_GOPTIONS(new_aip[dst], > ACL_NO_RIGHTS, ACL_NO_RIGHTS); > num++; /* set num to the size of new_acl */ > } > > We then try and revoke the specified privileges from the new empty > privilege, leaving it empty (modechg will equal ACL_MODECHG_DEL here): > > old_rights = ACLITEM_GET_RIGHTS(new_aip[dst]); > old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]); > > /* apply the specified permissions change */ > switch (modechg) > { > case ACL_MODECHG_ADD: > ACLITEM_SET_RIGHTS(new_aip[dst], > old_rights | ACLITEM_GET_RIGHTS(*mod_aip)); > break; > case ACL_MODECHG_DEL: > ACLITEM_SET_RIGHTS(new_aip[dst], > old_rights & ~ACLITEM_GET_RIGHTS(*mod_aip)); > break; > case ACL_MODECHG_EQL: > ACLITEM_SET_RIGHTS(new_aip[dst], > ACLITEM_GET_RIGHTS(*mod_aip)); > break; > } > > Then since the new privilege remains empty, we remove it from the ACL > list: > > new_rights = ACLITEM_GET_RIGHTS(new_aip[dst]); > new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]); > > /* > * If the adjusted entry has no permissions, delete it from the list. > */ > if (new_rights == ACL_NO_RIGHTS) > { > memmove(new_aip + dst, > new_aip + dst + 1, > (num - dst - 1) * sizeof(AclItem)); > /* Adjust array size to be 'num - 1' items */ > ARR_DIMS(new_acl)[0] = num - 1; > SET_VARSIZE(new_acl, ACL_N_SIZE(num - 1)); > }
Sorry about the unformatted code, here's the entire quoted section again with proper formatting: I looked into this function and that is correct. We fail to find a match for the revoked privilege here: /* * Search the ACL for an existing entry for this grantee and grantor. If * one exists, just modify the entry in-place (well, in the same position, * since we actually return a copy); otherwise, insert the new entry at * the end. */ for (dst = 0; dst < num; ++dst) { if (aclitem_match(mod_aip, old_aip + dst)) { /* found a match, so modify existing item */ new_acl = allocacl(num); new_aip = ACL_DAT(new_acl); memcpy(new_acl, old_acl, ACL_SIZE(old_acl)); break; } } Seeing that there was no match, we add a new empty privilege to the end of the existing ACL list here: if (dst == num) { /* need to append a new item */ new_acl = allocacl(num + 1); new_aip = ACL_DAT(new_acl); memcpy(new_aip, old_aip, num * sizeof(AclItem)); /* initialize the new entry with no permissions */ new_aip[dst].ai_grantee = mod_aip->ai_grantee; new_aip[dst].ai_grantor = mod_aip->ai_grantor; ACLITEM_SET_PRIVS_GOPTIONS(new_aip[dst], ACL_NO_RIGHTS, ACL_NO_RIGHTS); num++; /* set num to the size of new_acl */ } We then try and revoke the specified privileges from the new empty privilege, leaving it empty (modechg will equal ACL_MODECHG_DEL here): old_rights = ACLITEM_GET_RIGHTS(new_aip[dst]); old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]); /* apply the specified permissions change */ switch (modechg) { case ACL_MODECHG_ADD: ACLITEM_SET_RIGHTS(new_aip[dst], old_rights | ACLITEM_GET_RIGHTS(*mod_aip)); break; case ACL_MODECHG_DEL: ACLITEM_SET_RIGHTS(new_aip[dst], old_rights & ~ACLITEM_GET_RIGHTS(*mod_aip)); break; case ACL_MODECHG_EQL: ACLITEM_SET_RIGHTS(new_aip[dst], ACLITEM_GET_RIGHTS(*mod_aip)); break; } Then since the new privilege remains empty, we remove it from the ACL list: new_rights = ACLITEM_GET_RIGHTS(new_aip[dst]); new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]); /* * If the adjusted entry has no permissions, delete it from the list. */ if (new_rights == ACL_NO_RIGHTS) { memmove(new_aip + dst, new_aip + dst + 1, (num - dst - 1) * sizeof(AclItem)); /* Adjust array size to be 'num - 1' items */ ARR_DIMS(new_acl)[0] = num - 1; SET_VARSIZE(new_acl, ACL_N_SIZE(num - 1)); } Thanks, Joe Koshakow