| Message ID | 172488638886.4433.12153470259536099118@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [nfsd-fixes] nfsd: fix nfsd4_deleg_getattr_conflict in presence of third party lease | expand |
On Thu, 29 Aug 2024 09:06:28 +1000, NeilBrown wrote: > It is not safe to dereference fl->c.flc_owner without first confirming > fl->fl_lmops is the expected manager. nfsd4_deleg_getattr_conflict() > tests fl_lmops but largely ignores the result and assumes that flc_owner > is an nfs4_delegation anyway. This is wrong. > > With this patch we restore the "!= &nfsd_lease_mng_ops" case to behave > as it did before the changed mentioned below. This the same as the > current code, but without any reference to a possible delegation. > > [...] Applied to nfsd-fixes for v6.11-rc, thanks! [1/1] nfsd: fix nfsd4_deleg_getattr_conflict in presence of third party lease commit: 1271e30308ff96cc702bc7ef764fa6546c0541fb
On Thu, 2024-08-29 at 09:06 +1000, NeilBrown wrote: > It is not safe to dereference fl->c.flc_owner without first confirming > fl->fl_lmops is the expected manager. nfsd4_deleg_getattr_conflict() > tests fl_lmops but largely ignores the result and assumes that flc_owner > is an nfs4_delegation anyway. This is wrong. > > With this patch we restore the "!= &nfsd_lease_mng_ops" case to behave > as it did before the changed mentioned below. This the same as the > current code, but without any reference to a possible delegation. > > Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation") > Signed-off-by: NeilBrown <neilb@suse.de> > --- > fs/nfsd/nfs4state.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 07f2496850c4..f6a67ef27435 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -8859,7 +8859,15 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, > */ > if (type == F_RDLCK) > break; > - goto break_lease; > + > + nfsd_stats_wdeleg_getattr_inc(nn); > + spin_unlock(&ctx->flc_lock); > + > + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); > + if (status != nfserr_jukebox || > + !nfsd_wait_for_delegreturn(rqstp, inode)) > + return status; > + return 0; > } > if (type == F_WRLCK) { > struct nfs4_delegation *dp = fl->c.flc_owner; > @@ -8868,7 +8876,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, > spin_unlock(&ctx->flc_lock); > return 0; > } > -break_lease: > nfsd_stats_wdeleg_getattr_inc(nn); > dp = fl->c.flc_owner; > refcount_inc(&dp->dl_stid.sc_count); I meant to send this the other day and got sidetracked! Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 07f2496850c4..f6a67ef27435 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -8859,7 +8859,15 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, */ if (type == F_RDLCK) break; - goto break_lease; + + nfsd_stats_wdeleg_getattr_inc(nn); + spin_unlock(&ctx->flc_lock); + + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); + if (status != nfserr_jukebox || + !nfsd_wait_for_delegreturn(rqstp, inode)) + return status; + return 0; } if (type == F_WRLCK) { struct nfs4_delegation *dp = fl->c.flc_owner; @@ -8868,7 +8876,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, spin_unlock(&ctx->flc_lock); return 0; } -break_lease: nfsd_stats_wdeleg_getattr_inc(nn); dp = fl->c.flc_owner; refcount_inc(&dp->dl_stid.sc_count);
It is not safe to dereference fl->c.flc_owner without first confirming fl->fl_lmops is the expected manager. nfsd4_deleg_getattr_conflict() tests fl_lmops but largely ignores the result and assumes that flc_owner is an nfs4_delegation anyway. This is wrong. With this patch we restore the "!= &nfsd_lease_mng_ops" case to behave as it did before the changed mentioned below. This the same as the current code, but without any reference to a possible delegation. Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation") Signed-off-by: NeilBrown <neilb@suse.de> --- fs/nfsd/nfs4state.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)