NFSD: Refactor nfsd_setattr()'s ACL error reporting

Support for FATTR4_POSIX_ACCESS_ACL and FATTR4_POSIX_DEFAULT_ACL
attributes in subsequent patches allows clients to set both ACL
types simultaneously during SETATTR and file creation. Each ACL
type can succeed or fail independently, requiring the server to
clear individual attribute bits in the reply bitmap when one
fails while the other succeeds.

The existing na_aclerr field cannot distinguish which ACL type
encountered an error. Separate error fields (na_paclerr for
access ACLs, na_dpaclerr for default ACLs) enable the server to
report per-ACL-type failures accurately.

This refactoring also adds validation previously absent: default
ACL processing rejects non-directory targets with EINVAL and
passes NULL to set_posix_acl() when a_count is zero to delete
the ACL. Access ACL processing rejects zero a_count with EINVAL
for ACL_SCOPE_FILE_SYSTEM semantics (the only scope currently
supported).

The changes preserve compatibility with existing NFSv4 ACL code.
NFSv4 ACL conversion (nfs4_acl_nfsv4_to_posix()) never produces
POSIX ACLs with a_count == 0, so the new validation logic only
affects future POSIX ACL attribute handling.

Signed-off-by: Rick Macklem <rmacklem@uoguelph.ca>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a77ec0685eeec9ab84cd8a7bf0d07aa454dc957f..2da092f9ac40bebb0f98ea757fa9cd6d0bc0efb3 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -377,7 +377,7 @@ set_attr:
 
        if (attrs.na_labelerr)
                open->op_bmval[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
-       if (attrs.na_aclerr)
+       if (attrs.na_paclerr || attrs.na_dpaclerr)
                open->op_bmval[0] &= ~FATTR4_WORD0_ACL;
 out:
        end_creating(child);
@@ -858,7 +858,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
        if (attrs.na_labelerr)
                create->cr_bmval[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
-       if (attrs.na_aclerr)
+       if (attrs.na_paclerr || attrs.na_dpaclerr)
                create->cr_bmval[0] &= ~FATTR4_WORD0_ACL;
        set_change_info(&create->cr_cinfo, &cstate->current_fh);
        fh_dup2(&cstate->current_fh, &resfh);
@@ -1232,7 +1232,9 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        if (!status)
                status = nfserrno(attrs.na_labelerr);
        if (!status)
-               status = nfserrno(attrs.na_aclerr);
+               status = nfserrno(attrs.na_dpaclerr);
+       if (!status)
+               status = nfserrno(attrs.na_paclerr);
 out:
        nfsd_attrs_free(&attrs);
        fh_drop_write(&cstate->current_fh);

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 168d3ccc8155711294ef55681d51933b23926cb6..c884c3f34afb044ee5cacc962a04a97de2f7fd88 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -596,15 +596,35 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (attr->na_seclabel && attr->na_seclabel->len)
                attr->na_labelerr = security_inode_setsecctx(dentry,
                        attr->na_seclabel->data, attr->na_seclabel->len);
-       if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && attr->na_pacl)
-               attr->na_aclerr = set_posix_acl(&nop_mnt_idmap,
-                                               dentry, ACL_TYPE_ACCESS,
-                                               attr->na_pacl);
-       if (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
-           !attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))
-               attr->na_aclerr = set_posix_acl(&nop_mnt_idmap,
+       if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && attr->na_dpacl) {
+               if (!S_ISDIR(inode->i_mode))
+                       attr->na_dpaclerr = -EINVAL;
+               else if (attr->na_dpacl->a_count > 0)
+                       /* a_count == 0 means delete the ACL. */
+                       attr->na_dpaclerr = set_posix_acl(&nop_mnt_idmap,
                                                dentry, ACL_TYPE_DEFAULT,
                                                attr->na_dpacl);
+               else
+                       attr->na_dpaclerr = set_posix_acl(&nop_mnt_idmap,
+                                               dentry, ACL_TYPE_DEFAULT,
+                                               NULL);
+       }
+       if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && attr->na_pacl) {
+               /*
+                * For any file system that is not ACL_SCOPE_FILE_OBJECT,
+                * a_count == 0 MUST reply nfserr_inval.
+                * For a file system that is ACL_SCOPE_FILE_OBJECT,
+                * a_count == 0 deletes the ACL.
+                * XXX File systems that are ACL_SCOPE_FILE_OBJECT
+                * are not yet supported.
+                */
+               if (attr->na_pacl->a_count > 0)
+                       attr->na_paclerr = set_posix_acl(&nop_mnt_idmap,
+                                                       dentry, ACL_TYPE_ACCESS,
+                                                       attr->na_pacl);
+               else
+                       attr->na_paclerr = -EINVAL;
+       }
 out_fill_attrs:
        /*
         * RFC 1813 Section 3.3.2 does not mandate that an NFS server

diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index e192dca4a67919a6068ffc8bef67984d2550257c..702a844f2106b11d8a843b7b16fa35ef2a669bb4 100644 (file)
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -53,7 +53,8 @@ struct nfsd_attrs {
        struct posix_acl        *na_dpacl;      /* input */
 
        int                     na_labelerr;    /* output */
-       int                     na_aclerr;      /* output */
+       int                     na_dpaclerr;    /* output */
+       int                     na_paclerr;     /* output */
 };
 
 static inline void nfsd_attrs_free(struct nfsd_attrs *attrs)