[PATCH v3] nfsd: deal with revoked delegations appropriately

[PATCH v3] nfsd: deal with revoked delegations appropriately

[Andrew Elble]

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.

Signed-off-by: Andrew Elble <aweits@rit.edu>
---
 v2: deconflicting with Trond's OPEN/CLOSE locking work
 v3: don't return NFS4_OK on DELEGRETURN with revoked delegation
 
 fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0c04f81aa63b..d386d569edbc 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 @@ static bool nfsd4_is_deleg_cur(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 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 		     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 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 	*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;
 }
 
-- 
1.8.3.1

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[Trond Myklebust]

VGhhbmtzIGZvciB0aGUgcXVpY2sgdHVybmFyb3VuZCENCg0KT24gRnJpLCAyMDE3LTExLTAzIGF0
IDE0OjA2IC0wNDAwLCBBbmRyZXcgRWxibGUgd3JvdGU6DQo+IElmIGEgZGVsZWdhdGlvbiBoYXMg
YmVlbiByZXZva2VkIGJ5IHRoZSBzZXJ2ZXIsIG9wZXJhdGlvbnMgdXNpbmcgdGhhdA0KPiBkZWxl
Z2F0aW9uIHNob3VsZCBlcnJvciBvdXQgd2l0aCBORlM0RVJSX0RFTEVHX1JFVk9LRUQgaW4gdGhl
ID40LjENCj4gY2FzZSwgYW5kIE5GUzRFUlJfQkFEX1NUQVRFSUQgb3RoZXJ3aXNlLg0KPiANCj4g
U2lnbmVkLW9mZi1ieTogQW5kcmV3IEVsYmxlIDxhd2VpdHNAcml0LmVkdT4NCg0KUmV2aWV3ZWQt
Ynk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbT4NCg0K
PiAtLS0NCj4gIHYyOiBkZWNvbmZsaWN0aW5nIHdpdGggVHJvbmQncyBPUEVOL0NMT1NFIGxvY2tp
bmcgd29yaw0KPiAgdjM6IGRvbid0IHJldHVybiBORlM0X09LIG9uIERFTEVHUkVUVVJOIHdpdGgg
cmV2b2tlZCBkZWxlZ2F0aW9uDQo+ICANCj4gIGZzL25mc2QvbmZzNHN0YXRlLmMgfCAyNSArKysr
KysrKysrKysrKysrKysrKysrKystDQo+ICAxIGZpbGUgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygr
KSwgMSBkZWxldGlvbigtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mc2QvbmZzNHN0YXRlLmMg
Yi9mcy9uZnNkL25mczRzdGF0ZS5jDQo+IGluZGV4IDBjMDRmODFhYTYzYi4uZDM4NmQ1NjllZGJj
IDEwMDY0NA0KPiAtLS0gYS9mcy9uZnNkL25mczRzdGF0ZS5jDQo+ICsrKyBiL2ZzL25mc2QvbmZz
NHN0YXRlLmMNCj4gQEAgLTM5NjYsNyArMzk2Niw4IEBAIHN0YXRpYyBzdHJ1Y3QgbmZzNF9kZWxl
Z2F0aW9uDQo+ICpmaW5kX2RlbGVnX3N0YXRlaWQoc3RydWN0IG5mczRfY2xpZW50ICpjbCwgc3Rh
dGVpDQo+ICB7DQo+ICAJc3RydWN0IG5mczRfc3RpZCAqcmV0Ow0KPiAgDQo+IC0JcmV0ID0gZmlu
ZF9zdGF0ZWlkX2J5X3R5cGUoY2wsIHMsIE5GUzRfREVMRUdfU1RJRCk7DQo+ICsJcmV0ID0gZmlu
ZF9zdGF0ZWlkX2J5X3R5cGUoY2wsIHMsDQo+ICsJCQkJTkZTNF9ERUxFR19TVElEfE5GUzRfUkVW
T0tFRF9ERUxFR19TDQo+IFRJRCk7DQo+ICAJaWYgKCFyZXQpDQo+ICAJCXJldHVybiBOVUxMOw0K
PiAgCXJldHVybiBkZWxlZ3N0YXRlaWQocmV0KTsNCj4gQEAgLTM5ODksNiArMzk5MCwxMiBAQCBz
dGF0aWMgYm9vbCBuZnNkNF9pc19kZWxlZ19jdXIoc3RydWN0DQo+IG5mc2Q0X29wZW4gKm9wZW4p
DQo+ICAJZGVsZWcgPSBmaW5kX2RlbGVnX3N0YXRlaWQoY2wsICZvcGVuLT5vcF9kZWxlZ2F0ZV9z
dGF0ZWlkKTsNCj4gIAlpZiAoZGVsZWcgPT0gTlVMTCkNCj4gIAkJZ290byBvdXQ7DQo+ICsJaWYg
KGRlbGVnLT5kbF9zdGlkLnNjX3R5cGUgPT0gTkZTNF9SRVZPS0VEX0RFTEVHX1NUSUQpIHsNCj4g
KwkJbmZzNF9wdXRfc3RpZCgmZGVsZWctPmRsX3N0aWQpOw0KPiArCQlpZiAoY2wtPmNsX21pbm9y
dmVyc2lvbikNCj4gKwkJCXN0YXR1cyA9IG5mc2Vycl9kZWxlZ19yZXZva2VkOw0KPiArCQlnb3Rv
IG91dDsNCj4gKwl9DQo+ICAJZmxhZ3MgPSBzaGFyZV9hY2Nlc3NfdG9fZmxhZ3Mob3Blbi0+b3Bf
c2hhcmVfYWNjZXNzKTsNCj4gIAlzdGF0dXMgPSBuZnM0X2NoZWNrX2RlbGVnbW9kZShkZWxlZywg
ZmxhZ3MpOw0KPiAgCWlmIChzdGF0dXMpIHsNCj4gQEAgLTQ4NTgsNiArNDg2NSwxNiBAQCBzdGF0
aWMgX19iZTMyIG5mc2Q0X3ZhbGlkYXRlX3N0YXRlaWQoc3RydWN0DQo+IG5mczRfY2xpZW50ICpj
bCwgc3RhdGVpZF90ICpzdGF0ZWlkKQ0KPiAgCQkgICAgIHN0cnVjdCBuZnM0X3N0aWQgKipzLCBz
dHJ1Y3QgbmZzZF9uZXQgKm5uKQ0KPiAgew0KPiAgCV9fYmUzMiBzdGF0dXM7DQo+ICsJYm9vbCBy
ZXR1cm5fcmV2b2tlZCA9IGZhbHNlOw0KPiArDQo+ICsJLyoNCj4gKwkgKiAgb25seSByZXR1cm4g
cmV2b2tlZCBkZWxlZ2F0aW9ucyBpZiBleHBsaWNpdGx5IGFza2VkLg0KPiArCSAqICBvdGhlcndp
c2Ugd2UgcmVwb3J0IHJldm9rZWQgb3IgYmFkX3N0YXRlaWQgc3RhdHVzLg0KPiArCSAqLw0KPiAr
CWlmICh0eXBlbWFzayAmIE5GUzRfUkVWT0tFRF9ERUxFR19TVElEKQ0KPiArCQlyZXR1cm5fcmV2
b2tlZCA9IHRydWU7DQo+ICsJZWxzZSBpZiAodHlwZW1hc2sgJiBORlM0X0RFTEVHX1NUSUQpDQo+
ICsJCXR5cGVtYXNrIHw9IE5GUzRfUkVWT0tFRF9ERUxFR19TVElEOw0KPiAgDQo+ICAJaWYgKFpF
Uk9fU1RBVEVJRChzdGF0ZWlkKSB8fCBPTkVfU1RBVEVJRChzdGF0ZWlkKSkNCj4gIAkJcmV0dXJu
IG5mc2Vycl9iYWRfc3RhdGVpZDsNCj4gQEAgLTQ4NzIsNiArNDg4OSwxMiBAQCBzdGF0aWMgX19i
ZTMyIG5mc2Q0X3ZhbGlkYXRlX3N0YXRlaWQoc3RydWN0DQo+IG5mczRfY2xpZW50ICpjbCwgc3Rh
dGVpZF90ICpzdGF0ZWlkKQ0KPiAgCSpzID0gZmluZF9zdGF0ZWlkX2J5X3R5cGUoY3N0YXRlLT5j
bHAsIHN0YXRlaWQsIHR5cGVtYXNrKTsNCj4gIAlpZiAoISpzKQ0KPiAgCQlyZXR1cm4gbmZzZXJy
X2JhZF9zdGF0ZWlkOw0KPiArCWlmICgoKCpzKS0+c2NfdHlwZSA9PSBORlM0X1JFVk9LRURfREVM
RUdfU1RJRCkgJiYNCj4gIXJldHVybl9yZXZva2VkKSB7DQo+ICsJCW5mczRfcHV0X3N0aWQoKnMp
Ow0KPiArCQlpZiAoY3N0YXRlLT5taW5vcnZlcnNpb24pDQo+ICsJCQlyZXR1cm4gbmZzZXJyX2Rl
bGVnX3Jldm9rZWQ7DQo+ICsJCXJldHVybiBuZnNlcnJfYmFkX3N0YXRlaWQ7DQo+ICsJfQ0KPiAg
CXJldHVybiBuZnNfb2s7DQo+ICB9DQo+ICANCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBO
RlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFy
eWRhdGEuY29tDQo=

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[bfields]

On Fri, Nov 03, 2017 at 06:36:55PM +0000, Trond Myklebust wrote:
> Thanks for the quick turnaround!
> 
> On Fri, 2017-11-03 at 14:06 -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.
> > 
> > Signed-off-by: Andrew Elble <aweits@rit.edu>
> 
> Reviewed-by: Trond Myklebust <trond.myklebust@primarydata.com>

I wonder if there's a way to simplify the resulting logic in
nfsd4_lookup_stateid--I guess I don't see anything.

Could we get some context here in the changelog, though?  What actual
problem was this causing?

--b.

> 
> > ---
> >  v2: deconflicting with Trond's OPEN/CLOSE locking work
> >  v3: don't return NFS4_OK on DELEGRETURN with revoked delegation
> >  
> >  fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 0c04f81aa63b..d386d569edbc 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_S
> > TID);
> >  	if (!ret)
> >  		return NULL;
> >  	return delegstateid(ret);
> > @@ -3989,6 +3990,12 @@ static bool nfsd4_is_deleg_cur(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 @@ static __be32 nfsd4_validate_stateid(struct
> > nfs4_client *cl, stateid_t *stateid)
> >  		     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 @@ static __be32 nfsd4_validate_stateid(struct
> > nfs4_client *cl, stateid_t *stateid)
> >  	*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;
> >  }
> >  
> -- 
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> trond.myklebust@primarydata.com

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[Andrew W Elble]

"bfields@fieldses.org" <bfields@fieldses.org> writes:

> On Fri, Nov 03, 2017 at 06:36:55PM +0000, Trond Myklebust wrote:
>> Thanks for the quick turnaround!
>> 
>> On Fri, 2017-11-03 at 14:06 -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.
>> > 
>> > Signed-off-by: Andrew Elble <aweits@rit.edu>
>> 
>> Reviewed-by: Trond Myklebust <trond.myklebust@primarydata.com>
>
> I wonder if there's a way to simplify the resulting logic in
> nfsd4_lookup_stateid--I guess I don't see anything.
>
> Could we get some context here in the changelog, though?  What actual
> problem was this causing?

Prior thread (roughly) here: http://www.spinics.net/lists/linux-nfs/msg55260.html

This is the one patch I'm still carrying from the lost delegation work a
while back. Testing showed that there is a path still open to lost
delegations via delegreturn.

running with:
echo "error != 0" | sudo tee /sys/kernel/debug/tracing/events/nfs4/nfs4_delegreturn_exit/filter

gave us this at one point with an interim version of this patch:

kworker/0:0H-3990  [000] .... 5899655.609266: nfs4_delegreturn_exit: 
               error=-10087 (DELEG_REVOKED) dev=00:30 fhandle=0xe43d9d3a
kworker/0:2H-12665 [000] .... 5900011.719468: nfs4_delegreturn_exit: 
               error=-10087 (DELEG_REVOKED) dev=00:30 fhandle=0x16e37c0a

The Linux client prior to 26d36301bd653df6481fd38f3e1435a1f15e56d1 would
just drop delegations that suffered from a nfserr_bad_stateid during
delegreturn. Now it will do test/free if the return error is
nfserr_deleg_revoked.

If the client drops a delegation while the server has it on the revoked
list, we stay stuck in endless state manager looping for that client.

It might be a good idea for a stable backport of aforementioned commit,
or some kind of other workaround? Possibly interpreting
nfserr_bad_stateid an analogue of nfserr_deleg_revoked clientside
when dealing with a >4.0 mount? (also seems like an error on the putfh
might need to be caught as well?)

Thanks,

Andy

-- 
Andrew W. Elble
aweits@discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[bfields]

On Mon, Nov 06, 2017 at 12:30:23PM -0500, Andrew W Elble wrote:
> Prior thread (roughly) here: http://www.spinics.net/lists/linux-nfs/msg55260.html
> 
> This is the one patch I'm still carrying from the lost delegation work a
> while back. Testing showed that there is a path still open to lost
> delegations via delegreturn.
> 
> running with:
> echo "error != 0" | sudo tee /sys/kernel/debug/tracing/events/nfs4/nfs4_delegreturn_exit/filter
> 
> gave us this at one point with an interim version of this patch:
> 
> kworker/0:0H-3990  [000] .... 5899655.609266: nfs4_delegreturn_exit: 
>                error=-10087 (DELEG_REVOKED) dev=00:30 fhandle=0xe43d9d3a
> kworker/0:2H-12665 [000] .... 5900011.719468: nfs4_delegreturn_exit: 
>                error=-10087 (DELEG_REVOKED) dev=00:30 fhandle=0x16e37c0a
> 
> The Linux client prior to 26d36301bd653df6481fd38f3e1435a1f15e56d1 would
> just drop delegations that suffered from a nfserr_bad_stateid during
> delegreturn. Now it will do test/free if the return error is
> nfserr_deleg_revoked.
> 
> If the client drops a delegation while the server has it on the revoked
> list, we stay stuck in endless state manager looping for that client.
> 
> It might be a good idea for a stable backport of aforementioned commit,
> or some kind of other workaround? Possibly interpreting
> nfserr_bad_stateid an analogue of nfserr_deleg_revoked clientside
> when dealing with a >4.0 mount? (also seems like an error on the putfh
> might need to be caught as well?)

I'm just looking for a concise explanation of why your patch is
important.  And I probably haven't dug enough, but I'm still not quite
following.

If I understand right, the only visible change from your patch will be
returning DELEG_REVOKED instead of BAD_STATEID to 4.1 clients in some
cases.  I'm not clear what the result was (for old or new clients)--a
client left believing it held a delegation when it didn't, or a client
entering an infinite state manager loop?

--b.

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[Andrew W Elble]

"bfields@fieldses.org" <bfields@fieldses.org> writes:

> I'm just looking for a concise explanation of why your patch is
> important.  And I probably haven't dug enough, but I'm still not quite
> following.
>
> If I understand right, the only visible change from your patch will be
> returning DELEG_REVOKED instead of BAD_STATEID to 4.1 clients in some
> cases.  I'm not clear what the result was (for old or new clients)--a
> client left believing it held a delegation when it didn't, or a client
> entering an infinite state manager loop?

The current Linux client will "lose" a delegation on DELEGRETURN if it
does not see NFS4ERR_DELEG_REVOKED. This is unrecoverable and will
result in the client state manager looping unable to satisfy the
server's inevitable assertion of SEQ4_STATUS_RECALLABLE_STATE_REVOKED.

RFC5661 10.2.1: "A client normally finds out about revocation of a delegation
when it uses a stateid associated with a delegation and receives one of
the errors NFS4ERR_EXPIRED, NFS4ERR_ADMIN_REVOKED, or NFS4ERR_DELEG_REVOKED."

Thanks,

Andy

-- 
Andrew W. Elble
aweits@discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

Re: [PATCH v3] nfsd: deal with revoked delegations appropriately

[bfields]

On Mon, Nov 06, 2017 at 04:25:19PM -0500, Andrew W Elble wrote:
> 
> "bfields@fieldses.org" <bfields@fieldses.org> writes:
> 
> > I'm just looking for a concise explanation of why your patch is
> > important.  And I probably haven't dug enough, but I'm still not quite
> > following.
> >
> > If I understand right, the only visible change from your patch will be
> > returning DELEG_REVOKED instead of BAD_STATEID to 4.1 clients in some
> > cases.  I'm not clear what the result was (for old or new clients)--a
> > client left believing it held a delegation when it didn't, or a client
> > entering an infinite state manager loop?
> 
> The current Linux client will "lose" a delegation on DELEGRETURN if it
> does not see NFS4ERR_DELEG_REVOKED. This is unrecoverable and will
> result in the client state manager looping unable to satisfy the
> server's inevitable assertion of SEQ4_STATUS_RECALLABLE_STATE_REVOKED.
> 
> RFC5661 10.2.1: "A client normally finds out about revocation of a delegation
> when it uses a stateid associated with a delegation and receives one of
> the errors NFS4ERR_EXPIRED, NFS4ERR_ADMIN_REVOKED, or NFS4ERR_DELEG_REVOKED."

Got it, thanks for your persistence!  Committing as follows.

--b.

commit 4e9fd30e4432
Author: Andrew Elble <aweits@rit.edu>
Date:   Fri Nov 3 14:06:31 2017 -0400

    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.
    
    The server needs NFSv4.1 clients to explicitly free revoked delegations.
    If the server returns NFS4ERR_DELEG_REVOKED, the client will do that;
    otherwise it may just forget about the delegation and be unable to
    recover when it later sees SEQ4_STATUS_RECALLABLE_STATE_REVOKED set on a
    SEQUENCE reply.  That can cause the Linux 4.1 client to loop in its
    stage manager.
    
    Signed-off-by: Andrew Elble <aweits@rit.edu>
    Reviewed-by: Trond Myklebust <trond.myklebust@primarydata.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ecbc7b0dfa4d..b82817767b9d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4016,7 +4016,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);
@@ -4039,6 +4040,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) {
@@ -4908,6 +4915,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;
@@ -4922,6 +4939,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;
 }