2017-11-03 18:06:35

by Andrew W Elble

[permalink] [raw]
Subject: [PATCH v3] 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.

Signed-off-by: Andrew Elble <[email protected]>
---
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



2017-11-03 18:36:58

by Trond Myklebust

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

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=


2017-11-06 15:15:32

by J. Bruce Fields

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

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 <[email protected]>
>
> Reviewed-by: Trond Myklebust <[email protected]>

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
> [email protected]

2017-11-06 17:30:24

by Andrew W Elble

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


"[email protected]" <[email protected]> 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 <[email protected]>
>>
>> Reviewed-by: Trond Myklebust <[email protected]>
>
> 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
[email protected]
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

2017-11-06 19:35:28

by J. Bruce Fields

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

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.

2017-11-06 21:25:19

by Andrew W Elble

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


"[email protected]" <[email protected]> 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
[email protected]
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

2017-11-06 22:24:03

by J. Bruce Fields

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

On Mon, Nov 06, 2017 at 04:25:19PM -0500, Andrew W Elble wrote:
>
> "[email protected]" <[email protected]> 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 <[email protected]>
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 <[email protected]>
Reviewed-by: Trond Myklebust <[email protected]>
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>

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