summaryrefslogtreecommitdiff
path: root/fs/nfsd
diff options
context:
space:
mode:
authorChuck Lever <cel@kernel.org>2026-08-24 10:04:41 -0400
committerChuck Lever <cel@kernel.org>2026-09-23 09:00:42 -0400
commitdb4c1baf4f016d56aadb2fb5fbd77c7aed77eee7 (patch)
tree0a500d1439fe81480591b349acb65d9fc1f065f5 /fs/nfsd
parentbae7f4cd79d87434656fedf7465c2e75186e40f1 (diff)
downloadlinux-next-db4c1baf4f016d56aadb2fb5fbd77c7aed77eee7.tar.gz
linux-next-db4c1baf4f016d56aadb2fb5fbd77c7aed77eee7.zip
NFSD: Fix POSIX ACL leak in unexecuted NFSv4 COMPOUND operations
nfsd4_decode_fattr4() allocates POSIX ACLs while decoding OP_OPEN, OP_CREATE, and OP_SETATTR, leaving the only reference to these ACLs in the operation's argument structure. Executing the operation hands that reference to struct nfsd_attrs, which drops it. However, if the operation is decoded but never executes, those ACLs are leaked. A client can repeat an aborting compound to force the server to leak memory. Give struct nfsd_attrs its own reference with posix_acl_dup() so nfsd_attrs_free() still balances the reference the operation took. Release the ACLs when the compound completes. Have the decoder record each ACL on the compound's temporary allocation chain, and give each chained item an optional release callback. The chain holds a reference for the life of the compound, so OP_OPEN no longer needs an op_release method. Fixes: 5fc51dfc2eb1 ("NFSD: Add support for XDR decoding POSIX draft ACLs") Cc: stable+noautosel@kernel.org # experimental, disabled by default Reported-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com> Closes: https://lore.kernel.org/linux-nfs/20260823113255.3417-1-prabhakar.pujeri@dell.com/ Reviewed-by: Jeff Layton <jlayton@kernel.org> Link: https://patch.msgid.link/20260824-nfsd-posix-acl-ownership-v1-2-090fffc608ec@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org>
Diffstat (limited to 'fs/nfsd')
-rw-r--r--fs/nfsd/nfs4proc.c30
-rw-r--r--fs/nfsd/nfs4xdr.c41
-rw-r--r--fs/nfsd/xdr4.h1
3 files changed, 34 insertions, 38 deletions
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 88385a161b4d..bb74eef43938 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -391,11 +391,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (status)
return status;
} else {
- /* The dpacl and pacl will get released by nfsd_attrs_free(). */
- attrs.na_dpacl = open->op_dpacl;
- attrs.na_pacl = open->op_pacl;
- open->op_dpacl = NULL;
- open->op_pacl = NULL;
+ attrs.na_dpacl = posix_acl_dup(open->op_dpacl);
+ attrs.na_pacl = posix_acl_dup(open->op_pacl);
}
v_mtime = 0;
@@ -795,13 +792,6 @@ static __be32 nfsd4_open_omfg(struct svc_rqst *rqstp, struct nfsd4_compound_stat
return nfsd4_open(rqstp, cstate, &op->u);
}
-static void
-nfsd4_open_release(union nfsd4_op_u *u)
-{
- posix_acl_release(u->open.op_dpacl);
- posix_acl_release(u->open.op_pacl);
-}
-
/*
* filehandle-manipulating ops.
*/
@@ -922,16 +912,13 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd_attrs attrs = {
.na_iattr = &create->cr_iattr,
.na_seclabel = &create->cr_label,
- .na_dpacl = create->cr_dpacl,
- .na_pacl = create->cr_pacl,
+ .na_dpacl = posix_acl_dup(create->cr_dpacl),
+ .na_pacl = posix_acl_dup(create->cr_pacl),
};
struct svc_fh resfh;
__be32 status;
dev_t rdev;
- create->cr_dpacl = NULL;
- create->cr_pacl = NULL;
-
fh_init(&resfh, NFS4_FHSIZE);
status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_NOP);
@@ -1341,8 +1328,8 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd_attrs attrs = {
.na_iattr = &setattr->sa_iattr,
.na_seclabel = &setattr->sa_label,
- .na_pacl = setattr->sa_pacl,
- .na_dpacl = setattr->sa_dpacl,
+ .na_pacl = posix_acl_dup(setattr->sa_pacl),
+ .na_dpacl = posix_acl_dup(setattr->sa_dpacl),
};
bool save_no_wcc, deleg_attrs;
struct nfs4_stid *st = NULL;
@@ -1350,10 +1337,6 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
__be32 status = nfs_ok;
int err;
- /* Transfer ownership to attrs for cleanup via nfsd_attrs_free() */
- setattr->sa_pacl = NULL;
- setattr->sa_dpacl = NULL;
-
deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
FATTR4_WORD2_TIME_DELEG_MODIFY);
@@ -3943,7 +3926,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
},
[OP_OPEN] = {
.op_func = nfsd4_open,
- .op_release = nfsd4_open_release,
.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
.op_name = "OP_OPEN",
.op_rsize_bop = nfsd4_open_rsize,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a154b02d82b3..5bfbeb87394e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -114,27 +114,27 @@ static int zero_clientid(clientid_t *clid)
return (clid->cl_boot == 0) && (clid->cl_id == 0);
}
-/**
- * svcxdr_tmpalloc - allocate memory to be freed after compound processing
- * @argp: NFSv4 compound argument structure
- * @len: length of buffer to allocate
- *
- * Allocates a buffer of size @len to be freed when processing the compound
- * operation described in @argp finishes.
- */
static void *
-svcxdr_tmpalloc(struct nfsd4_compoundargs *argp, size_t len)
+svcxdr_tmpalloc_release(struct nfsd4_compoundargs *argp, size_t len,
+ void (*release)(void *))
{
struct svcxdr_tmpbuf *tb;
tb = kmalloc_flex(*tb, buf, len);
if (!tb)
return NULL;
+ tb->release = release;
tb->next = argp->to_free;
argp->to_free = tb;
return tb->buf;
}
+static void *
+svcxdr_tmpalloc(struct nfsd4_compoundargs *argp, size_t len)
+{
+ return svcxdr_tmpalloc_release(argp, len, NULL);
+}
+
/*
* For xdr strings that need to be passed to other kernel api's
* as null-terminated strings.
@@ -442,10 +442,16 @@ nfsd4_decode_posixace4(struct nfsd4_compoundargs *argp,
return status;
}
+static void svcxdr_release_pacl(void *p)
+{
+ posix_acl_release(*(struct posix_acl **)p);
+}
+
static noinline __be32
nfsd4_decode_posixacl(struct nfsd4_compoundargs *argp, struct posix_acl **acl)
{
struct posix_acl_entry *ace;
+ struct posix_acl **slot;
__be32 status;
u32 count;
@@ -485,6 +491,15 @@ nfsd4_decode_posixacl(struct nfsd4_compoundargs *argp, struct posix_acl **acl)
if (count >= 3)
sort_pacl_range(*acl, 0, count - 1);
+ slot = svcxdr_tmpalloc_release(argp, sizeof(*slot),
+ svcxdr_release_pacl);
+ if (!slot) {
+ posix_acl_release(*acl);
+ *acl = NULL;
+ return nfserr_jukebox;
+ }
+ *slot = *acl;
+
return nfs_ok;
}
@@ -677,7 +692,6 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
status = nfsd4_decode_posixacl(argp, &pacl);
if (status) {
- posix_acl_release(*dpaclp);
*dpaclp = NULL;
return status;
}
@@ -687,12 +701,8 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
/* request sanity: did attrlist4 contain the expected number of words? */
if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos) {
-#ifdef CONFIG_NFSD_V4_POSIX_ACLS
- posix_acl_release(*dpaclp);
- posix_acl_release(*paclp);
*dpaclp = NULL;
*paclp = NULL;
-#endif
return nfserr_bad_xdr;
}
@@ -6846,7 +6856,10 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp)
}
while (args->to_free) {
struct svcxdr_tmpbuf *tb = args->to_free;
+
args->to_free = tb->next;
+ if (tb->release)
+ tb->release(tb->buf);
kfree(tb);
}
}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index b841bc462dac..de43a0da9668 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -800,6 +800,7 @@ bool nfsd4_cache_this_op(struct nfsd4_op *);
*/
struct svcxdr_tmpbuf {
struct svcxdr_tmpbuf *next;
+ void (*release)(void *buf);
char buf[];
};