Skip to content

Commit

Permalink
Add ability for xattr handler to "strip" NFSv4 ACL (openzfs#54)
Browse files Browse the repository at this point in the history 
On Linux POSIX ACLs can be removed via rmxattr() for the
relevant system xattrs. On FreeBSD a non-trivial ACL
can be converted to one that is described by the mode with
no loss of info via combination of acl_get_file(), acl_strip_np(),
and acl_set_file(). Since there's no libc equivalent of these
ops in Linux for NFSv4 ACLs, this commit makes this less error
prone by handling entirely in ZFS. When user performs
rmxattr() vfs_setxattr() is called with value of NULL and length
of 0. Add special handling for this situation in the xattr
handler for the NFSv4 ACL so that we generate a new ACL and
zfs_acl_chmod() with the existing mode of file, then set the ACL.

Signed-off-by: Andrew Walker <[email protected]>
  • Loading branch information
@anodos325 @ixhamza
anodos325 authored and ixhamza committed Feb 29, 2024
1 parent 29d6360 commit d1df3f6
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 42 deletions.
2 changes: 2 additions & 0 deletions include/sys/zfs_acl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ void zfs_acl_ids_free(zfs_acl_ids_t *);
boolean_t zfs_acl_ids_overquota(struct zfsvfs *, zfs_acl_ids_t *, uint64_t);
int zfs_getacl(struct znode *, vsecattr_t *, boolean_t, cred_t *);
int zfs_setacl(struct znode *, vsecattr_t *, boolean_t, cred_t *);
int zfs_stripacl(struct znode *, cred_t *);

void zfs_acl_rele(void *);
void zfs_oldace_byteswap(ace_t *, int);
void zfs_ace_byteswap(void *, size_t, boolean_t);
Expand Down
6 changes: 6 additions & 0 deletions module/os/freebsd/zfs/zfs_acl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,12 @@ zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
return (error);
}

int
zfs_stripacl(znode_t *zp, cred_t *cr)
{
return (SET_ERROR(EOPNOTSUPP));
}

/*
* Check accesses of interest (AoI) against attributes of the dataset
* such as read-only. Returns zero if no AoI conflict with dataset
Expand Down
157 changes: 121 additions & 36 deletions module/os/linux/zfs/zfs_acl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include <sys/zfs_quota.h>
#include <sys/zfs_vfsops.h>
#include <sys/dmu.h>
#include <sys/dmu_objset.h>
#include <sys/dnode.h>
#include <sys/zap.h>
#include <sys/sa.h>
Expand Down Expand Up @@ -1963,8 +1964,8 @@ zfs_acl_ids_overquota(zfsvfs_t *zv, zfs_acl_ids_t *acl_ids, uint64_t projid)
/*
* Retrieve a file's ACL
*/
int
zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
static int
zfs_getacl_impl(znode_t *zp, vsecattr_t *vsecp, boolean_t stripped, cred_t *cr)
{
zfs_acl_t *aclp;
ulong_t mask;
Expand All @@ -1975,21 +1976,21 @@ zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
mask = vsecp->vsa_mask & (VSA_ACE | VSA_ACECNT |
VSA_ACE_ACLFLAGS | VSA_ACE_ALLTYPES);

if (mask == 0)
return (SET_ERROR(ENOSYS));

if ((error = zfs_zaccess(zp, ACE_READ_ACL, 0, skipaclchk, cr,
zfs_init_idmap)))
return (error);

mutex_enter(&zp->z_acl_lock);

error = zfs_acl_node_read(zp, B_FALSE, &aclp, B_FALSE);
if (error != 0) {
mutex_exit(&zp->z_acl_lock);
return (error);
if (stripped) {
mode_t mode = ZTOI(zp)->i_mode;
aclp = zfs_acl_alloc(zfs_acl_version_zp(zp));
(aclp)->z_hints = zp->z_pflags & V4_ACL_WIDE_FLAGS;
zfs_acl_chmod(S_ISDIR(mode), mode, B_TRUE,
(ZTOZSB(zp)->z_acl_mode == ZFS_ACL_GROUPMASK), aclp);
} else {
error = zfs_acl_node_read(zp, B_FALSE, &aclp, B_FALSE);
if (error != 0)
return (error);
}

mask = vsecp->vsa_mask & (VSA_ACE | VSA_ACECNT |
VSA_ACE_ACLFLAGS | VSA_ACE_ALLTYPES);

/*
* Scan ACL to determine number of ACEs
*/
Expand Down Expand Up @@ -2038,7 +2039,7 @@ zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)

for (aclnode = list_head(&aclp->z_acl); aclnode;
aclnode = list_next(&aclp->z_acl, aclnode)) {
memcpy(start, aclnode->z_acldata,
bcopy(aclnode->z_acldata, start,
aclnode->z_size);
start = (caddr_t)start + aclnode->z_size;
}
Expand All @@ -2060,9 +2061,29 @@ zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
vsecp->vsa_aclflags |= ACL_IS_DIR;
}

return (0);
}

int
zfs_getacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
{
int error;
ulong_t mask;

mask = vsecp->vsa_mask & (VSA_ACE | VSA_ACECNT |
VSA_ACE_ACLFLAGS | VSA_ACE_ALLTYPES);

if (mask == 0)
return (SET_ERROR(ENOSYS));

if ((error = zfs_zaccess(zp, ACE_READ_ACL, 0, skipaclchk, cr)))
return (error);

mutex_enter(&zp->z_acl_lock);
error = zfs_getacl_impl(zp, vsecp, B_FALSE, cr);
mutex_exit(&zp->z_acl_lock);

return (0);
return (error);
}

int
Expand Down Expand Up @@ -2123,29 +2144,18 @@ zfs_vsec_2_aclp(zfsvfs_t *zfsvfs, umode_t obj_mode,
/*
* Set a file's ACL
*/
int
zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
static int
zfs_setacl_impl(znode_t *zp, vsecattr_t *vsecp, cred_t *cr)
{
zfsvfs_t *zfsvfs = ZTOZSB(zp);
zilog_t *zilog = zfsvfs->z_log;
ulong_t mask = vsecp->vsa_mask & (VSA_ACE | VSA_ACECNT);
dmu_tx_t *tx;
int error;
zfs_acl_t *aclp;
zfs_fuid_info_t *fuidp = NULL;
boolean_t fuid_dirtied;
uint64_t acl_obj;

if (mask == 0)
return (SET_ERROR(ENOSYS));

if (zp->z_pflags & ZFS_IMMUTABLE)
return (SET_ERROR(EPERM));

if ((error = zfs_zaccess(zp, ACE_WRITE_ACL, 0, skipaclchk, cr,
zfs_init_idmap)))
return (error);

error = zfs_vsec_2_aclp(zfsvfs, ZTOI(zp)->i_mode, vsecp, cr, &fuidp,
&aclp);
if (error)
Expand All @@ -2160,9 +2170,6 @@ zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
(zp->z_pflags & V4_ACL_WIDE_FLAGS);
}
top:
mutex_enter(&zp->z_acl_lock);
mutex_enter(&zp->z_lock);

tx = dmu_tx_create(zfsvfs->z_os);

dmu_tx_hold_sa(tx, zp->z_sa_hdl, B_TRUE);
Expand Down Expand Up @@ -2193,12 +2200,15 @@ zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
zfs_sa_upgrade_txholds(tx, zp);
error = dmu_tx_assign(tx, TXG_NOWAIT);
if (error) {
mutex_exit(&zp->z_acl_lock);
mutex_exit(&zp->z_lock);

if (error == ERESTART) {
mutex_exit(&zp->z_acl_lock);
mutex_exit(&zp->z_lock);

dmu_tx_wait(tx);
dmu_tx_abort(tx);

mutex_enter(&zp->z_acl_lock);
mutex_enter(&zp->z_lock);
goto top;
}
dmu_tx_abort(tx);
Expand All @@ -2220,9 +2230,84 @@ zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
zfs_fuid_info_free(fuidp);
dmu_tx_commit(tx);

return (error);
}

int
zfs_setacl(znode_t *zp, vsecattr_t *vsecp, boolean_t skipaclchk, cred_t *cr)
{
ulong_t mask = vsecp->vsa_mask & (VSA_ACE | VSA_ACECNT);
int error;

if (mask == 0)
return (SET_ERROR(ENOSYS));

if (zp->z_pflags & ZFS_IMMUTABLE)
return (SET_ERROR(EPERM));

if ((error = zfs_zaccess(zp, ACE_WRITE_ACL, 0, skipaclchk, cr)))
return (error);

mutex_enter(&zp->z_acl_lock);
mutex_enter(&zp->z_lock);

error = zfs_setacl_impl(zp, vsecp, cr);

mutex_exit(&zp->z_lock);
mutex_exit(&zp->z_acl_lock);
return (error);
}


int
zfs_stripacl(znode_t *zp, cred_t *cr)
{
int error;
zfsvfs_t *zfsvfs = ZTOZSB(zp);

vsecattr_t vsec = {
.vsa_mask = VSA_ACE_ALLTYPES | VSA_ACECNT | VSA_ACE |
VSA_ACE_ACLFLAGS
};

ZFS_ENTER(zfsvfs);
ZFS_VERIFY_ZP(zp);

if (zp->z_pflags & ZFS_IMMUTABLE) {
error = SET_ERROR(EPERM);
goto done;
}

if ((error = zfs_zaccess(zp, ACE_WRITE_ACL, 0, B_FALSE, cr)))
goto done;

if (zp->z_pflags & ZFS_ACL_TRIVIAL) {
// ACL is already stripped. Nothing to do.
error = 0;
goto done;
}

mutex_enter(&zp->z_acl_lock);
error = zfs_getacl_impl(zp, &vsec, B_TRUE, cr);
if (error) {
mutex_exit(&zp->z_acl_lock);
goto done;
}

mutex_enter(&zp->z_lock);

error = zfs_setacl_impl(zp, &vsec, cr);

mutex_exit(&zp->z_lock);
mutex_exit(&zp->z_acl_lock);

kmem_free(vsec.vsa_aclentp, vsec.vsa_aclentsz);

if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zfsvfs->z_log, 0);

done:
ZFS_EXIT(zfsvfs);
return (error);
}

Expand Down
12 changes: 6 additions & 6 deletions module/os/linux/zfs/zpl_xattr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1867,12 +1867,12 @@ __zpl_xattr_nfs41acl_set(struct inode *ip, const char *name,
if (ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4)
return (-EOPNOTSUPP);

/*
* TODO: we may receive NULL value and size 0
* when rmxattr() on our special xattr is called.
* A function to "strip" the ACL needs to be added
* to avoid POLA violation.
*/
if (value == NULL && size == 0) {
crhold(cr);
error = zfs_stripacl(ITOZ(ip), cr);
crfree(cr);
return (error);
}

/* xdr data is 4-byte aligned */
if (((ulong_t)value % 4) != 0) {
Expand Down

0 comments on commit d1df3f6

Please sign in to comment.