2017-10-31 19:29:45

by Andrew W Elble

[permalink] [raw]
Subject: [PATCH] nfsd: deal with revoked delegations appropriately

If a delegation has been revoked by the server, operations using that
delegation should error out with NFS4ERR_DELEG_REVOKED in the >4.1
case, and NFS4ERR_BAD_STATEID otherwise. DELEGRETURN has also been
optimized to return NFS4_OK when called with a revoked delegation.

Thread on DELEGRETURN optimization is here:
https://www.spinics.net/lists/linux-nfs/msg55216.html

Signed-off-by: Andrew Elble <[email protected]>
---
fs/nfsd/nfs4state.c | 53 +++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0c04f81aa63b..2545ca69a6b5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3966,7 +3966,8 @@ static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl, statei
{
struct nfs4_stid *ret;

- ret = find_stateid_by_type(cl, s, NFS4_DELEG_STID);
+ ret = find_stateid_by_type(cl, s,
+ NFS4_DELEG_STID|NFS4_REVOKED_DELEG_STID);
if (!ret)
return NULL;
return delegstateid(ret);
@@ -3989,6 +3990,12 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
deleg = find_deleg_stateid(cl, &open->op_delegate_stateid);
if (deleg == NULL)
goto out;
+ if (deleg->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
+ nfs4_put_stid(&deleg->dl_stid);
+ if (cl->cl_minorversion)
+ status = nfserr_deleg_revoked;
+ goto out;
+ }
flags = share_access_to_flags(open->op_share_access);
status = nfs4_check_delegmode(deleg, flags);
if (status) {
@@ -4858,6 +4865,16 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
struct nfs4_stid **s, struct nfsd_net *nn)
{
__be32 status;
+ bool return_revoked = false;
+
+ /*
+ * only return revoked delegations if explicitly asked.
+ * otherwise we report revoked or bad_stateid status.
+ */
+ if (typemask & NFS4_REVOKED_DELEG_STID)
+ return_revoked = true;
+ else if (typemask & NFS4_DELEG_STID)
+ typemask |= NFS4_REVOKED_DELEG_STID;

if (ZERO_STATEID(stateid) || ONE_STATEID(stateid))
return nfserr_bad_stateid;
@@ -4872,6 +4889,12 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
*s = find_stateid_by_type(cstate->clp, stateid, typemask);
if (!*s)
return nfserr_bad_stateid;
+ if (((*s)->sc_type == NFS4_REVOKED_DELEG_STID) && !return_revoked) {
+ nfs4_put_stid(*s);
+ if (cstate->minorversion)
+ return nfserr_deleg_revoked;
+ return nfserr_bad_stateid;
+ }
return nfs_ok;
}

@@ -5106,13 +5129,11 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
status = nfsd4_check_seqid(cstate, sop, seqid);
if (status)
return status;
- if (stp->st_stid.sc_type == NFS4_CLOSED_STID
- || stp->st_stid.sc_type == NFS4_REVOKED_DELEG_STID)
- /*
- * "Closed" stateid's exist *only* to return
- * nfserr_replay_me from the previous step, and
- * revoked delegations are kept only for free_stateid.
- */
+ /*
+ * "Closed" stateid's exist *only* to return
+ * nfserr_replay_me from the previous step
+ */
+ if (stp->st_stid.sc_type == NFS4_CLOSED_STID)
return nfserr_bad_stateid;
mutex_lock(&stp->st_mutex);
status = check_stateid_generation(stateid, &stp->st_stid.sc_stateid, nfsd4_has_session(cstate));
@@ -5359,7 +5380,9 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
return status;

- status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID, &s, nn);
+ status = nfsd4_lookup_stateid(cstate, stateid,
+ NFS4_DELEG_STID|NFS4_REVOKED_DELEG_STID,
+ &s, nn);
if (status)
goto out;
dp = delegstateid(s);
@@ -5367,7 +5390,17 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (status)
goto put_stateid;

- destroy_delegation(dp);
+ if (dp->dl_stid.sc_type == NFS4_DELEG_STID)
+ destroy_delegation(dp);
+ if (dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
+ /*
+ * optimization: don't return error if client is
+ * returning a revoked delegation
+ */
+ list_del_init(&dp->dl_recall_lru);
+ nfs4_put_stid(s);
+ }
+
put_stateid:
nfs4_put_stid(&dp->dl_stid);
out:
--
2.10.1



2017-10-31 20:12:12

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] nfsd: deal with revoked delegations appropriately

T24gVHVlLCAyMDE3LTEwLTMxIGF0IDE1OjIyIC0wNDAwLCBBbmRyZXcgRWxibGUgd3JvdGU6DQo+
IElmIGEgZGVsZWdhdGlvbiBoYXMgYmVlbiByZXZva2VkIGJ5IHRoZSBzZXJ2ZXIsIG9wZXJhdGlv
bnMgdXNpbmcgdGhhdA0KPiBkZWxlZ2F0aW9uIHNob3VsZCBlcnJvciBvdXQgd2l0aCBORlM0RVJS
X0RFTEVHX1JFVk9LRUQgaW4gdGhlID40LjENCj4gY2FzZSwgYW5kIE5GUzRFUlJfQkFEX1NUQVRF
SUQgb3RoZXJ3aXNlLiBERUxFR1JFVFVSTiBoYXMgYWxzbyBiZWVuDQo+IG9wdGltaXplZCB0byBy
ZXR1cm4gTkZTNF9PSyB3aGVuIGNhbGxlZCB3aXRoIGEgcmV2b2tlZCBkZWxlZ2F0aW9uLg0KPiAN
Cj4gVGhyZWFkIG9uIERFTEVHUkVUVVJOIG9wdGltaXphdGlvbiBpcyBoZXJlOg0KPiBodHRwczov
L3d3dy5zcGluaWNzLm5ldC9saXN0cy9saW51eC1uZnMvbXNnNTUyMTYuaHRtbA0KPiANCj4gU2ln
bmVkLW9mZi1ieTogQW5kcmV3IEVsYmxlIDxhd2VpdHNAcml0LmVkdT4NCj4gDQpQbGVhc2Ugc2Vl
IHRoZSBwYXRjaCBzZXJpZXMgZm9yIGtuZnNkIHRoYXQgSSBwb3N0ZWQgeWVzdGVyZGF5LiBEb2Vz
DQp0aGF0IGZpeCB5b3VyIGlzc3VlPw0KDQpDaGVlcnMNCiAgVHJvbmQNCi0tIA0KVHJvbmQgTXlr
bGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5t
eWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=


2017-10-31 20:37:54

by Andrew W Elble

[permalink] [raw]
Subject: Re: [PATCH] nfsd: deal with revoked delegations appropriately


Trond Myklebust <[email protected]> writes:

> On Tue, 2017-10-31 at 15:22 -0400, Andrew Elble wrote:
>> If a delegation has been revoked by the server, operations using that
>> delegation should error out with NFS4ERR_DELEG_REVOKED in the >4.1
>> case, and NFS4ERR_BAD_STATEID otherwise. DELEGRETURN has also been
>> optimized to return NFS4_OK when called with a revoked delegation.
>>
>> Thread on DELEGRETURN optimization is here:
>> https://www.spinics.net/lists/linux-nfs/msg55216.html
>>
>> Signed-off-by: Andrew Elble <[email protected]>
>>
> Please see the patch series for knfsd that I posted yesterday. Does
> that fix your issue?

Just about to start looking at that in-depth, but I don't think so unless somehow
the typemask is changed to actually return revoked stateids? The problem
is we're not looking for revoked stateids in typemask for
nfsd4_lookup_stateid(), and they're not returned by default.

Thanks,

Andy

--
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