[PATCH v1 0/2] nfs_common: Encode an ACL with fewer than three entries as empty

[PATCH v1 0/2] nfs_common: Encode an ACL with fewer than three entries as empty

[Chuck Lever]

Both ACL encoders in fs/nfs_common report at least four wire entries
for any ACL that has entries, then walk a_entries[] up to that count.
An ACL with one or two entries sends the walk past the end of the
posix_acl allocation.

nfsacl_encode() builds only the NFS client's SETACL arguments, and
nfs_stream_encode_acl() builds only NFSD's GETACL replies. The fixes
are separate patches because a combined one does not apply to the
stable trees that predate nfs_stream_encode_acl().

The NFS client still caches a GETACL result without validating it.
This series does not change that.

Neither over-read has been observed at runtime. Both paths were
traced from the code, and the patches are build-tested only.

Patch 1 changes what the NFS client sends in SETACL, so it needs an
Acked-by from the NFS client maintainers before it goes through the
nfsd tree.

Chuck Lever (2):
  nfs_common: Do not encode an ACL with fewer than three entries
  nfs_common: Do not stream-encode an ACL with fewer than three entries

 fs/nfs_common/nfsacl.c | 6 ++++--
 include/linux/nfsacl.h | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH v1 1/2] nfs_common: Do not encode an ACL with fewer than three entries

[Chuck Lever]

nfsacl_encode() reports at least four wire entries for any ACL
that has entries, because a three-entry ACL is sent as four with a
synthesized ACL_MASK. The entry encoder then walks a_entries[] up
to that count.

A one- or two-entry ACL cannot arrive from setfacl, because
set_posix_acl() validates it first. It can arrive from an NFS
server. The NFS client caches a GETACL result without validating
it, and posix_acl_create() hands the cached default ACL to
nfs3_proc_setacls() when the client creates a directory. The walk
then reads past the end of the posix_acl allocation and copies the
bytes into the SETACL arguments.

Report zero wire entries for an ACL with fewer than three, the
same as for an ACL with none. Count them the same way in
nfsacl_size(), which otherwise sizes such an ACL at four entries
and leaves the reserved bytes unwritten in the SETACL arguments.

Fixes: a257cdd0e217 ("[PATCH] NFSD: Add server support for NFSv3 ACLs.")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfs_common/nfsacl.c | 3 ++-
 include/linux/nfsacl.h | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/nfs_common/nfsacl.c b/fs/nfs_common/nfsacl.c
index e2eaac14fd8e..38bb2c294d7c 100644
--- a/fs/nfs_common/nfsacl.c
+++ b/fs/nfs_common/nfsacl.c
@@ -93,7 +93,8 @@ xdr_nfsace_encode(struct xdr_array2_desc *desc, void *elem)
 int nfsacl_encode(struct xdr_buf *buf, unsigned int base, struct inode *inode,
 		  struct posix_acl *acl, int encode_entries, int typeflag)
 {
-	int entries = (acl && acl->a_count) ? max_t(int, acl->a_count, 4) : 0;
+	int entries = (acl && acl->a_count >= 3) ?
+		      max_t(int, acl->a_count, 4) : 0;
 	struct nfsacl_encode_desc nfsacl_desc = {
 		.desc = {
 			.elem_size = 12,
diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
index 8e76a79cdc6a..e4155e69c8dd 100644
--- a/include/linux/nfsacl.h
+++ b/include/linux/nfsacl.h
@@ -26,8 +26,9 @@ static inline unsigned int
 nfsacl_size(struct posix_acl *acl_access, struct posix_acl *acl_default)
 {
 	unsigned int w = 16;
-	w += max(acl_access ? (int)acl_access->a_count : 3, 4) * 12;
-	if (acl_default)
+	if (acl_access && acl_access->a_count >= 3)
+		w += max((int)acl_access->a_count, 4) * 12;
+	if (acl_default && acl_default->a_count >= 3)
 		w += max((int)acl_default->a_count, 4) * 12;
 	return w;
 }
-- 
2.55.0

Re: [PATCH v1 1/2] nfs_common: Do not encode an ACL with fewer than three entries

[Anna Schumaker]

On Thu, Sep 24, 2026, at 5:22 PM, Chuck Lever wrote:
> nfsacl_encode() reports at least four wire entries for any ACL
> that has entries, because a three-entry ACL is sent as four with a
> synthesized ACL_MASK. The entry encoder then walks a_entries[] up
> to that count.
>
> A one- or two-entry ACL cannot arrive from setfacl, because
> set_posix_acl() validates it first. It can arrive from an NFS
> server. The NFS client caches a GETACL result without validating
> it, and posix_acl_create() hands the cached default ACL to
> nfs3_proc_setacls() when the client creates a directory. The walk
> then reads past the end of the posix_acl allocation and copies the
> bytes into the SETACL arguments.
>
> Report zero wire entries for an ACL with fewer than three, the
> same as for an ACL with none. Count them the same way in
> nfsacl_size(), which otherwise sizes such an ACL at four entries
> and leaves the reserved bytes unwritten in the SETACL arguments.
>
> Fixes: a257cdd0e217 ("[PATCH] NFSD: Add server support for NFSv3 ACLs.")
> Signed-off-by: Chuck Lever <cel@kernel.org>

Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>


> ---
>  fs/nfs_common/nfsacl.c | 3 ++-
>  include/linux/nfsacl.h | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs_common/nfsacl.c b/fs/nfs_common/nfsacl.c
> index e2eaac14fd8e..38bb2c294d7c 100644
> --- a/fs/nfs_common/nfsacl.c
> +++ b/fs/nfs_common/nfsacl.c
> @@ -93,7 +93,8 @@ xdr_nfsace_encode(struct xdr_array2_desc *desc, void *elem)
>  int nfsacl_encode(struct xdr_buf *buf, unsigned int base, struct inode *inode,
>  		  struct posix_acl *acl, int encode_entries, int typeflag)
>  {
> -	int entries = (acl && acl->a_count) ? max_t(int, acl->a_count, 4) : 0;
> +	int entries = (acl && acl->a_count >= 3) ?
> +		      max_t(int, acl->a_count, 4) : 0;
>  	struct nfsacl_encode_desc nfsacl_desc = {
>  		.desc = {
>  			.elem_size = 12,
> diff --git a/include/linux/nfsacl.h b/include/linux/nfsacl.h
> index 8e76a79cdc6a..e4155e69c8dd 100644
> --- a/include/linux/nfsacl.h
> +++ b/include/linux/nfsacl.h
> @@ -26,8 +26,9 @@ static inline unsigned int
>  nfsacl_size(struct posix_acl *acl_access, struct posix_acl *acl_default)
>  {
>  	unsigned int w = 16;
> -	w += max(acl_access ? (int)acl_access->a_count : 3, 4) * 12;
> -	if (acl_default)
> +	if (acl_access && acl_access->a_count >= 3)
> +		w += max((int)acl_access->a_count, 4) * 12;
> +	if (acl_default && acl_default->a_count >= 3)
>  		w += max((int)acl_default->a_count, 4) * 12;
>  	return w;
>  }
> -- 
> 2.55.0

[PATCH v1 2/2] nfs_common: Do not stream-encode an ACL with fewer than three entries

[Chuck Lever]

nfs_stream_encode_acl() reports at least four wire entries for any
ACL that has entries, because a three-entry ACL is sent as four
with a synthesized ACL_MASK. The entry encoder then walks
a_entries[] up to that count. For an ACL with one or two entries,
which posix_acl_from_xattr() and the filesystem on-disk parsers do
not reject, the walk reads past the end of the posix_acl
allocation and copies the bytes into the GETACL reply.

Report zero wire entries for an ACL with fewer than three, the
same as for an ACL with none.

Fixes: 8edc0648880a ("NFSD: Add an xdr_stream-based encoder for NFSv2/3 ACLs")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfs_common/nfsacl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs_common/nfsacl.c b/fs/nfs_common/nfsacl.c
index 38bb2c294d7c..24ee7f622b18 100644
--- a/fs/nfs_common/nfsacl.c
+++ b/fs/nfs_common/nfsacl.c
@@ -157,7 +157,8 @@ bool nfs_stream_encode_acl(struct xdr_stream *xdr, struct inode *inode,
 			   int typeflag)
 {
 	const size_t elem_size = XDR_UNIT * 3;
-	u32 entries = (acl && acl->a_count) ? max_t(int, acl->a_count, 4) : 0;
+	u32 entries = (acl && acl->a_count >= 3) ?
+		      max_t(int, acl->a_count, 4) : 0;
 	struct nfsacl_encode_desc nfsacl_desc = {
 		.desc = {
 			.elem_size = elem_size,
-- 
2.55.0