nfsd: fix error handling in nfs4_set_delegation()
Subsystems (2)
PATCH 1/1 nfsd: fix error handling in nfs4_set_delegation()
I noticed a memory corruption crash in nfsd in
4.17-rc1. This patch corrects the issue.
Fix to return error if the delegation couldn't be hashed or there was a recall in progress. Use the existing error path instead of destroy_delegation() for readability.
--- fs/nfsd/nfs4state.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/nfsd/nfs4state.c +4 -1
@@ -4376,8 +4376,11 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,spin_unlock (&state_lock);if (status) -destroy_unhashed_deleg (dp); + goto out_unlock; +return dp; +out_unlock: +vfs_setlease (fp->fi_deleg_file,F_UNLCK ,NULL , (void **)&dp); out_clnt_odstate:put_clnt_odstate (dp->dl_clnt_odstate); out_stid:
Signature · 1 line
1.8.3.1
Quoted 6 lines · Andrew Elble
I noticed a memory corruption crash in nfsd in
4.17-rc1. This patch corrects the issue.Fix to return error if the delegation couldn't be hashed or there was a recall in progress. Use the existing error path instead of
destroy_delegation()for readability.
Apologies for the slow response, this looks right, I'm just still trying to figure out whether this is the only problem here--e.g. does it cause problems to have this lease temporarily applied before the delegation is hashed?
Also the RHEL7 code appears to have worse problems in the same spot and I'm trying to work out if they'd explaining a long-standing bug report....
--b.
Quoted 26 lines
Signed-off-by Andrew Elble <[email protected]>Fixes 353601e7d323c ("nfsd: create a separate lease for each delegation")--- fs/nfsd/nfs4state.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)fs/nfsd/nfs4state.c +4
-1@@ -4376,8 +4376,11 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,spin_unlock (&state_lock);if (status) -destroy_unhashed_deleg (dp); + goto out_unlock; +return dp; +out_unlock: +vfs_setlease (fp->fi_deleg_file,F_UNLCK ,NULL , (void **)&dp); out_clnt_odstate:put_clnt_odstate (dp->dl_clnt_odstate); out_stid:Signature · 1 line
1.8.3.1
Quoted 4 lines · J. Bruce Fields
Apologies for the slow response, this looks right, I'm just still trying to figure out whether this is the only problem here--e.g. does it cause problems to have this lease temporarily applied before the delegation is hashed?
So the only thing I can currently theorize is that we might eject some recalls for delegations that will never hit the wire. I think the worst possibility is that nfsd4_cb_recall_done() might wind up declaring the callback path to be down? Still looking...
Thanks,
Andy
Signature · 5 lines
Andrew W. Elble [email protected] Infrastructure Engineer, Communications Technical Lead Rochester Institute of Technology PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912