nfsd: fix posix_acl leak on SETACL decode failure
commit 0853ac544c590880d797b04daa33fcb72b6be0e1 upstream.
nfsaclsvc_decode_setaclargs() and nfs3svc_decode_setaclargs() each
call nfs_stream_decode_acl() twice, first for NFS_ACL and then for
NFS_DFACL. Each successful call transfers ownership of a freshly
allocated posix_acl into argp->acl_access or argp->acl_default. If
the first call succeeds but the second fails, the decoder returns
false and argp->acl_access is left dangling.
ACLPROC2_SETACL.pc_release was wired to nfssvc_release_attrstat and
ACLPROC3_SETACL.pc_release was wired to nfs3svc_release_fhandle.
Both only call fh_put() and have no knowledge of the ACL fields on
argp. The posix_acl_release() pairs sat at the out: labels inside
nfsacld_proc_setacl() and nfsd3_proc_setacl(), but svc_process()
skips pc_func when pc_decode returns false, so that cleanup is
unreachable on decode failure:
svc_process_common()
pc_decode() /* decode_setaclargs: false */
/* pc_func skipped */
pc_release() /* fh_put only -- ACLs leaked */
The orphaned posix_acl is leaked for the lifetime of the server.
Fix by adding nfsaclsvc_release_setacl() and nfs3svc_release_setacl(),
which release both argp->acl_access and argp->acl_default in addition
to fh_put(), and wiring them as pc_release for their respective SETACL
procedures. pc_release runs on every path svc_process() takes after
decode, including decode failure, so the posix_acl_release() pairs are
removed from the proc functions' out: labels to keep ownership in one
place. This matches the existing release_getacl() pattern used by
the sibling GETACL procedures.
Fixes: a257cdd0e2 ("[PATCH] NFSD: Add server support for NFSv3 ACLs.")
Cc: stable@vger.kernel.org
Assisted-by: kres:claude-opus-4-7
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[uli: backport to 4.19]
Signed-off-by: Ulrich Hecht <uli@kernel.org>
Reviewed-by: Pavel Machek <pavel@nabladev.com>
This commit is contained in:
1 parent
22e81af523
commit
56ee568e6c
2 files changed
+25
-10
No files matched your search
+13
-5
@@ -127,11 +127,9 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
|
||||
nfserr = fh_getattr(fh, &resp->stat);
|
||||
|
||||
out:
|
||||
/* argp->acl_{access,default} may have been allocated in
|
||||
nfssvc_decode_setaclargs. */
|
||||
posix_acl_release(argp->acl_access);
|
||||
posix_acl_release(argp->acl_default);
|
||||
/* argp->acl_{access,default} are released in nfsaclsvc_release_setacl. */
|
||||
return nfserr;
|
||||
|
||||
out_drop_lock:
|
||||
fh_unlock(fh);
|
||||
fh_drop_write(fh);
|
||||
@@ -354,6 +352,16 @@ static void nfsaclsvc_release_access(struct svc_rqst *rqstp)
|
||||
#define nfsd3_voidres nfsd3_voidargs
|
||||
struct nfsd3_voidargs { int dummy; };
|
||||
|
||||
static void nfsaclsvc_release_setacl(struct svc_rqst *rqstp)
|
||||
{
|
||||
struct nfsd3_setaclargs *argp = rqstp->rq_argp;
|
||||
struct nfsd_attrstat *resp = rqstp->rq_resp;
|
||||
|
||||
fh_put(&resp->fh);
|
||||
posix_acl_release(argp->acl_access);
|
||||
posix_acl_release(argp->acl_default);
|
||||
}
|
||||
|
||||
#define PROC(name, argt, rest, relt, cache, respsize) \
|
||||
{ \
|
||||
.pc_func = nfsacld_proc_##name, \
|
||||
@@ -374,7 +382,7 @@ struct nfsd3_voidargs { int dummy; };
|
||||
static const struct svc_procedure nfsd_acl_procedures2[] = {
|
||||
PROC(null, void, void, void, RC_NOCACHE, ST),
|
||||
PROC(getacl, getacl, getacl, getacl, RC_NOCACHE, ST+1+2*(1+ACL)),
|
||||
PROC(setacl, setacl, attrstat, attrstat, RC_NOCACHE, ST+AT),
|
||||
PROC(setacl, setacl, attrstat, setacl, RC_NOCACHE, ST+AT),
|
||||
PROC(getattr, fhandle, attrstat, attrstat, RC_NOCACHE, ST+AT),
|
||||
PROC(access, access, access, access, RC_NOCACHE, ST+AT+1),
|
||||
};
|
||||
|
||||
+12
-5
@@ -115,10 +115,7 @@ out_drop_lock:
|
||||
out_errno:
|
||||
nfserr = nfserrno(error);
|
||||
out:
|
||||
/* argp->acl_{access,default} may have been allocated in
|
||||
nfs3svc_decode_setaclargs. */
|
||||
posix_acl_release(argp->acl_access);
|
||||
posix_acl_release(argp->acl_default);
|
||||
/* argp->acl_{access,default} are released in nfs3svc_release_setacl. */
|
||||
RETURN_STATUS(nfserr);
|
||||
}
|
||||
|
||||
@@ -241,6 +238,16 @@ static void nfs3svc_release_getacl(struct svc_rqst *rqstp)
|
||||
#define nfsd3_voidres nfsd3_voidargs
|
||||
struct nfsd3_voidargs { int dummy; };
|
||||
|
||||
static void nfs3svc_release_setacl(struct svc_rqst *rqstp)
|
||||
{
|
||||
struct nfsd3_setaclargs *argp = rqstp->rq_argp;
|
||||
struct nfsd3_attrstat *resp = rqstp->rq_resp;
|
||||
|
||||
fh_put(&resp->fh);
|
||||
posix_acl_release(argp->acl_access);
|
||||
posix_acl_release(argp->acl_default);
|
||||
}
|
||||
|
||||
#define PROC(name, argt, rest, relt, cache, respsize) \
|
||||
{ \
|
||||
.pc_func = nfsd3_proc_##name, \
|
||||
@@ -261,7 +268,7 @@ struct nfsd3_voidargs { int dummy; };
|
||||
static const struct svc_procedure nfsd_acl_procedures3[] = {
|
||||
PROC(null, void, void, void, RC_NOCACHE, ST),
|
||||
PROC(getacl, getacl, getacl, getacl, RC_NOCACHE, ST+1+2*(1+ACL)),
|
||||
PROC(setacl, setacl, setacl, fhandle, RC_NOCACHE, ST+pAT),
|
||||
PROC(setacl, setacl, setacl, setacl, RC_NOCACHE, ST+pAT),
|
||||
};
|
||||
|
||||
static unsigned int nfsd_acl_count3[ARRAY_SIZE(nfsd_acl_procedures3)];
|
||||
|
||||
Reference in new issue
Block a user