Re: [PATCH 5/6] nfsd: release OPEN-decoded posix ACLs via op_release

Jeff Layton <[email protected]>
Newsgroups gmane.linux.nfs,gmane.linux.kernel
Message-ID <[email protected]>
On Sun, 2026-05-31 at 11:54 -0400, Chuck Lever wrote:
> On Sun, May 31, 2026, at 8:07 AM, Jeff Layton wrote:
> > From: Chris Mason <[email protected]>
> 
> > Remove the matching posix_acl_release() pair from nfsd4_open()'s
> > out_err: label: the compound dispatcher calls op_release
> > unconditionally after every op, [...]
> 
> The double-free fix is right, but op_release is not called
> unconditionally after every op, and that gap leaks the ACLs on the
> v4.0 replay path.
> 
> op_release runs only at the release: label inside
> nfsd4_encode_operation().  The compound loop skips that encoder on
> a replay:
> 
>         if (op->status == nfserr_replay_me) {
>                 nfsd4_encode_replay(resp->xdr, op);     /* no op_release */
>                 ...
>         } else {
>                 nfsd4_encode_operation(resp, op);       /* op_release here */
>         }
> 
> So every ACL-bearing v4.0 OPEN retransmit leaks two posix_acl refs.
> 
> Please release op->u on the replay branch too:
> 
>         if (op->status == nfserr_replay_me) {
>                 op->replay = &cstate->replay_owner->so_replay;
>                 nfsd4_encode_replay(resp->xdr, op);
>                 status = op->status = op->replay->rp_status;
>                 if (op->opdesc->op_release)
>                         op->opdesc->op_release(&op->u);
>         }
> 
> Let's fix the "unconditionally after every op" wording too.
> 
> I've applied the other 5 in this series, so you can just resend
> this one.
> 

Good catch, will fix.

I initially had hoped that we could rely on the fact that a v4.0 client
couldn't issue an OPEN with a POSIX ACL attribute in it, but it turns
out that knfsd doesn't gate attributes by minorversion.

Maybe it should? That's a separate changeset though if so.
-- 
Jeff Layton <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.