SGkgQnJ1Y2UsDQoNCkknbSBwdXp6bGVkIGFib3V0IHRoZSByZXN1bHQgb2Yg
bmZzNC4wL3NlcnZlcnRlc3RzL3N0X2NvbXBvdW5kL3Rlc3RMb25nQ29tcG91
bmQgaW4gcHlORlMuDQoNClRoZSB0ZXN0IGluY3JlYXNlcyB0aGUgbnVtYmVy
IG9mIHJlcXVlc3RzIGluIHRoZSBjb21wb3VuZCwgYW5kIGV4cGVjdHMgdGhl
IHNlcnZlciB0byByZXR1cm4gTkZTIDQgRVJSIFJFU09VUkNFIHdoZW4gdGhl
IG51bWJlciBvZiByZXF1ZXN0cyByZWFjaGVzIHRoZSB1cHBlciBsaW1pdCBv
biB0aGUgc2VydmVyIHNpZGUuDQoNCkluIHRoZSBSSEVMIDcuNEdBIHNvdXJj
ZSBjb2RlLCBhZnRlciBORlM0RVJSX0JBRFhEUiBpcyByZXR1cm5lZCBhZnRl
ciB0aGUgbnVtYmVyIG9mIHJlcXVlc3RzIHJlYWNoZXMgdGhlIHNlcnZlciBs
aW1pdCwgUlBDX0dBUkJBR0VfQVJHUyBpcyByZXR1cm5lZCBpbiB0aGUgZGlz
cGF0Y2ggZnVuY3Rpb24uDQoNCkFjY29yZGluZyB0byBSRkMgNzUzMCwgd2hl
biB0aGUgc2VydmVyIHBhcnNlcyBDT01QT1VORCwgaXQgcmV0dXJucyBORlMg
NCBFUlIgUkVTT1VSQ0UgaWYgdGhlIHJlc291cmNlIGlzIGluc3VmZmljaWVu
dC4gSWYgYW4gZXJyb3Igb2NjdXJzIHdoZW4gcGFyc2luZyBYRFIsIE5GUzRF
UlJfQkFEWERSIGlzIHJldHVybmVkLg0KUmVxdWVzdHMgaW4gQ29tcG91bmQg
YXJlIHBhcnNlZCBpbiBYRFIuIFRoZXJlZm9yZSwgSSBkZWNpZGUgaGVyZSB0
aGF0IE5GUzRFUlJfQkFEWERSIHNob3VsZCBiZSByZXR1cm5lZC4gVGhlIHRl
c3RjYXNlIHNob3VsZCBiZSBwYXNzZWQgaGVyZS4gSG93ZXZlcix0aGUgcmVz
dWx0IGlzIGZhaWx1cmUuDQoNClRoYW5rcywNCkx1IFhpbnl1Cgo=
On Thu, Nov 30, 2017 at 08:44:33AM +0000, Lu, Xinyu wrote:
> I'm puzzled about the result of nfs4.0/servertests/st_compound/testLongCompound in pyNFS.
>
> The test increases the number of requests in the compound, and expects the server to return NFS 4 ERR RESOURCE when the number of requests reaches the upper limit on the server side.
>
> In the RHEL 7.4GA source code, after NFS4ERR_BADXDR is returned after the number of requests reaches the server limit, RPC_GARBAGE_ARGS is returned in the dispatch function.
>
> According to RFC 7530, when the server parses COMPOUND, it returns NFS 4 ERR RESOURCE if the resource is insufficient. If an error occurs when parsing XDR, NFS4ERR_BADXDR is returned.
> Requests in Compound are parsed in XDR. Therefore, I decide here that NFS4ERR_BADXDR should be returned. The testcase should be passed here. However,the result is failure.
I agree it's a bug, though not likely to cause problems for clients in
practice.
I wrote the following patch, but I think it's not quite correct--I need
to take another look.
(By the way this was written in response to another report from you, I
think--the name was the same but the email address was different. Which
name and address would you like me to use to credit you?)
--b.
commit fd9a5e1c4952
Author: J. Bruce Fields <[email protected]>
Date: Wed Nov 15 12:30:27 2017 -0500
nfsd: return RESOURCE not GARBAGE_ARGS on too many ops
A client that sends more than a hundred ops in a single compound
currently gets an rpc-level GARBAGE_ARGS error.
It would be more helpful to return NFS4ERR_RESOURCE, since that gives
the client a better idea how to recover (for example by splitting up the
compound into smaller compounds).
This is all a bit academic since we've never actually seen a reason for
clients to send such long compounds, but we may as well fix it.
While we're there, just use NFSD4_MAX_OPS_PER_COMPOUND == 16, the
constant we already use in the 4.1 case, instead of hard-coding 100.
Chances anyone actually uses even 16 ops per compound are small enough
that I think there's a neglible risk or any regression.
This fixes pynfs test COMP6.
Reported-by: sunlianwen <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 008ea0b627d0..c9669d75c157 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1703,6 +1703,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
status = nfserr_minor_vers_mismatch;
if (nfsd_minorversion(args->minorversion, NFSD_TEST) <= 0)
goto out;
+ status = nfserr_resource;
+ if (resp->opcnt > NFSD_MAX_OPS_PER_COMPOUND)
+ goto out;
status = nfs41_check_op_ordering(args);
if (status) {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2c61c6b8ae09..5dcd7cb45b2d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1918,8 +1918,13 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
if (argp->taglen > NFSD4_MAX_TAGLEN)
goto xdr_error;
- if (argp->opcnt > 100)
- goto xdr_error;
+ /*
+ * NFS4ERR_RESOURCE is a more helpful error than GARBAGE_ARGS
+ * here, so we return success at the xdr level so that
+ * nfsd4_proc can handle this is an NFS-level error.
+ */
+ if (argp->opcnt > NFSD_MAX_OPS_PER_COMPOUND)
+ return 0;
if (argp->opcnt > ARRAY_SIZE(argp->iops)) {
argp->ops = kzalloc(argp->opcnt * sizeof(*argp->ops), GFP_KERNEL);
VGhpcyBpcyBteSBjb3JyZWN0IGFkZHJlc3MuIFRoZSBmb3JtZXIgbWFpbCB3
YXMgc2VudCBieSBteSBjb2xsZWFndWUgYmVjYXVzZSBJIGRpZCBub3QgaGF2
ZSBhY2Nlc3MgdG8gc2VuZGluZyBlbWFpbHMgdG8gdGhlIGV4dGVybmFsIG5l
dHdvcmsgYXQgdGhhdCB0aW1lLiBMT0wuDQoNCk9uIDEyLzAxLzIwMTcgMTI6
MzMgQU0sIEouIEJydWNlIEZpZWxkcyB3cm90ZToNCj4gT24gVGh1LCBOb3Yg
MzAsIDIwMTcgYXQgMDg6NDQ6MzNBTSArMDAwMCwgTHUsIFhpbnl1IHdyb3Rl
Og0KPj4gSSdtIHB1enpsZWQgYWJvdXQgdGhlIHJlc3VsdCBvZiBuZnM0LjAv
c2VydmVydGVzdHMvc3RfY29tcG91bmQvdGVzdExvbmdDb21wb3VuZCBpbiBw
eU5GUy4NCj4+DQo+PiBUaGUgdGVzdCBpbmNyZWFzZXMgdGhlIG51bWJlciBv
ZiByZXF1ZXN0cyBpbiB0aGUgY29tcG91bmQsIGFuZCBleHBlY3RzIHRoZSBz
ZXJ2ZXIgdG8gcmV0dXJuIE5GUyA0IEVSUiBSRVNPVVJDRSB3aGVuIHRoZSBu
dW1iZXIgb2YgcmVxdWVzdHMgcmVhY2hlcyB0aGUgdXBwZXIgbGltaXQgb24g
dGhlIHNlcnZlciBzaWRlLg0KPj4NCj4+IEluIHRoZSBSSEVMIDcuNEdBIHNv
dXJjZSBjb2RlLCBhZnRlciBORlM0RVJSX0JBRFhEUiBpcyByZXR1cm5lZCBh
ZnRlciB0aGUgbnVtYmVyIG9mIHJlcXVlc3RzIHJlYWNoZXMgdGhlIHNlcnZl
ciBsaW1pdCwgUlBDX0dBUkJBR0VfQVJHUyBpcyByZXR1cm5lZCBpbiB0aGUg
ZGlzcGF0Y2ggZnVuY3Rpb24uDQo+Pg0KPj4gQWNjb3JkaW5nIHRvIFJGQyA3
NTMwLCB3aGVuIHRoZSBzZXJ2ZXIgcGFyc2VzIENPTVBPVU5ELCBpdCByZXR1
cm5zIE5GUyA0IEVSUiBSRVNPVVJDRSBpZiB0aGUgcmVzb3VyY2UgaXMgaW5z
dWZmaWNpZW50LiBJZiBhbiBlcnJvciBvY2N1cnMgd2hlbiBwYXJzaW5nIFhE
UiwgTkZTNEVSUl9CQURYRFIgaXMgcmV0dXJuZWQuDQo+PiBSZXF1ZXN0cyBp
biBDb21wb3VuZCBhcmUgcGFyc2VkIGluIFhEUi4gVGhlcmVmb3JlLCBJIGRl
Y2lkZSBoZXJlIHRoYXQgTkZTNEVSUl9CQURYRFIgc2hvdWxkIGJlIHJldHVy
bmVkLiBUaGUgdGVzdGNhc2Ugc2hvdWxkIGJlIHBhc3NlZCBoZXJlLiBIb3dl
dmVyLHRoZSByZXN1bHQgaXMgZmFpbHVyZS4NCj4gDQo+IEkgYWdyZWUgaXQn
cyBhIGJ1ZywgdGhvdWdoIG5vdCBsaWtlbHkgdG8gY2F1c2UgcHJvYmxlbXMg
Zm9yIGNsaWVudHMgaW4NCj4gcHJhY3RpY2UuDQo+IA0KPiBJIHdyb3RlIHRo
ZSBmb2xsb3dpbmcgcGF0Y2gsIGJ1dCBJIHRoaW5rIGl0J3Mgbm90IHF1aXRl
IGNvcnJlY3QtLUkgbmVlZA0KPiB0byB0YWtlIGFub3RoZXIgbG9vay4NCj4g
DQo+IChCeSB0aGUgd2F5IHRoaXMgd2FzIHdyaXR0ZW4gaW4gcmVzcG9uc2Ug
dG8gYW5vdGhlciByZXBvcnQgZnJvbSB5b3UsIEkNCj4gdGhpbmstLXRoZSBu
YW1lIHdhcyB0aGUgc2FtZSBidXQgdGhlIGVtYWlsIGFkZHJlc3Mgd2FzIGRp
ZmZlcmVudC4gIFdoaWNoDQo+IG5hbWUgYW5kIGFkZHJlc3Mgd291bGQgeW91
IGxpa2UgbWUgdG8gdXNlIHRvIGNyZWRpdCB5b3U/KQ0KPiANCj4gLS1iLg0K
PiANCj4gY29tbWl0IGZkOWE1ZTFjNDk1Mg0KPiBBdXRob3I6IEouIEJydWNl
IEZpZWxkcyA8YmZpZWxkc0ByZWRoYXQuY29tPg0KPiBEYXRlOiAgIFdlZCBO
b3YgMTUgMTI6MzA6MjcgMjAxNyAtMDUwMA0KPiANCj4gICAgIG5mc2Q6IHJl
dHVybiBSRVNPVVJDRSBub3QgR0FSQkFHRV9BUkdTIG9uIHRvbyBtYW55IG9w
cw0KPiAgICAgDQo+ICAgICBBIGNsaWVudCB0aGF0IHNlbmRzIG1vcmUgdGhh
biBhIGh1bmRyZWQgb3BzIGluIGEgc2luZ2xlIGNvbXBvdW5kDQo+ICAgICBj
dXJyZW50bHkgZ2V0cyBhbiBycGMtbGV2ZWwgR0FSQkFHRV9BUkdTIGVycm9y
Lg0KPiAgICAgDQo+ICAgICBJdCB3b3VsZCBiZSBtb3JlIGhlbHBmdWwgdG8g
cmV0dXJuIE5GUzRFUlJfUkVTT1VSQ0UsIHNpbmNlIHRoYXQgZ2l2ZXMNCj4g
ICAgIHRoZSBjbGllbnQgYSBiZXR0ZXIgaWRlYSBob3cgdG8gcmVjb3ZlciAo
Zm9yIGV4YW1wbGUgYnkgc3BsaXR0aW5nIHVwIHRoZQ0KPiAgICAgY29tcG91
bmQgaW50byBzbWFsbGVyIGNvbXBvdW5kcykuDQo+ICAgICANCj4gICAgIFRo
aXMgaXMgYWxsIGEgYml0IGFjYWRlbWljIHNpbmNlIHdlJ3ZlIG5ldmVyIGFj
dHVhbGx5IHNlZW4gYSByZWFzb24gZm9yDQo+ICAgICBjbGllbnRzIHRvIHNl
bmQgc3VjaCBsb25nIGNvbXBvdW5kcywgYnV0IHdlIG1heSBhcyB3ZWxsIGZp
eCBpdC4NCj4gICAgIA0KPiAgICAgV2hpbGUgd2UncmUgdGhlcmUsIGp1c3Qg
dXNlIE5GU0Q0X01BWF9PUFNfUEVSX0NPTVBPVU5EID09IDE2LCB0aGUNCj4g
ICAgIGNvbnN0YW50IHdlIGFscmVhZHkgdXNlIGluIHRoZSA0LjEgY2FzZSwg
aW5zdGVhZCBvZiBoYXJkLWNvZGluZyAxMDAuDQo+ICAgICBDaGFuY2VzIGFu
eW9uZSBhY3R1YWxseSB1c2VzIGV2ZW4gMTYgb3BzIHBlciBjb21wb3VuZCBh
cmUgc21hbGwgZW5vdWdoDQo+ICAgICB0aGF0IEkgdGhpbmsgdGhlcmUncyBh
IG5lZ2xpYmxlIHJpc2sgb3IgYW55IHJlZ3Jlc3Npb24uDQo+ICAgICANCj4g
ICAgIFRoaXMgZml4ZXMgcHluZnMgdGVzdCBDT01QNi4NCj4gICAgIA0KPiAg
ICAgUmVwb3J0ZWQtYnk6IHN1bmxpYW53ZW4gPHN1bmx3LmZuc3RAY24uZnVq
aXRzdS5jb20+DQo+ICAgICBTaWduZWQtb2ZmLWJ5OiBKLiBCcnVjZSBGaWVs
ZHMgPGJmaWVsZHNAcmVkaGF0LmNvbT4NCj4gDQo+IGRpZmYgLS1naXQgYS9m
cy9uZnNkL25mczRwcm9jLmMgYi9mcy9uZnNkL25mczRwcm9jLmMNCj4gaW5k
ZXggMDA4ZWEwYjYyN2QwLi5jOTY2OWQ3NWMxNTcgMTAwNjQ0DQo+IC0tLSBh
L2ZzL25mc2QvbmZzNHByb2MuYw0KPiArKysgYi9mcy9uZnNkL25mczRwcm9j
LmMNCj4gQEAgLTE3MDMsNiArMTcwMyw5IEBAIG5mc2Q0X3Byb2NfY29tcG91
bmQoc3RydWN0IHN2Y19ycXN0ICpycXN0cCkNCj4gIAlzdGF0dXMgPSBuZnNl
cnJfbWlub3JfdmVyc19taXNtYXRjaDsNCj4gIAlpZiAobmZzZF9taW5vcnZl
cnNpb24oYXJncy0+bWlub3J2ZXJzaW9uLCBORlNEX1RFU1QpIDw9IDApDQo+
ICAJCWdvdG8gb3V0Ow0KPiArCXN0YXR1cyA9IG5mc2Vycl9yZXNvdXJjZTsN
Cj4gKwlpZiAocmVzcC0+b3BjbnQgPiBORlNEX01BWF9PUFNfUEVSX0NPTVBP
VU5EKQ0KPiArCQlnb3RvIG91dDsNCj4gIA0KPiAgCXN0YXR1cyA9IG5mczQx
X2NoZWNrX29wX29yZGVyaW5nKGFyZ3MpOw0KPiAgCWlmIChzdGF0dXMpIHsN
Cj4gZGlmZiAtLWdpdCBhL2ZzL25mc2QvbmZzNHhkci5jIGIvZnMvbmZzZC9u
ZnM0eGRyLmMNCj4gaW5kZXggMmM2MWM2YjhhZTA5Li41ZGNkN2NiNDViMmQg
MTAwNjQ0DQo+IC0tLSBhL2ZzL25mc2QvbmZzNHhkci5jDQo+ICsrKyBiL2Zz
L25mc2QvbmZzNHhkci5jDQo+IEBAIC0xOTE4LDggKzE5MTgsMTMgQEAgbmZz
ZDRfZGVjb2RlX2NvbXBvdW5kKHN0cnVjdCBuZnNkNF9jb21wb3VuZGFyZ3Mg
KmFyZ3ApDQo+ICANCj4gIAlpZiAoYXJncC0+dGFnbGVuID4gTkZTRDRfTUFY
X1RBR0xFTikNCj4gIAkJZ290byB4ZHJfZXJyb3I7DQo+IC0JaWYgKGFyZ3At
Pm9wY250ID4gMTAwKQ0KPiAtCQlnb3RvIHhkcl9lcnJvcjsNCj4gKwkvKg0K
PiArCSAqIE5GUzRFUlJfUkVTT1VSQ0UgaXMgYSBtb3JlIGhlbHBmdWwgZXJy
b3IgdGhhbiBHQVJCQUdFX0FSR1MNCj4gKwkgKiBoZXJlLCBzbyB3ZSByZXR1
cm4gc3VjY2VzcyBhdCB0aGUgeGRyIGxldmVsIHNvIHRoYXQNCj4gKwkgKiBu
ZnNkNF9wcm9jIGNhbiBoYW5kbGUgdGhpcyBpcyBhbiBORlMtbGV2ZWwgZXJy
b3IuDQo+ICsJICovDQo+ICsJaWYgKGFyZ3AtPm9wY250ID4gTkZTRF9NQVhf
T1BTX1BFUl9DT01QT1VORCkNCj4gKwkJcmV0dXJuIDA7DQo+ICANCj4gIAlp
ZiAoYXJncC0+b3BjbnQgPiBBUlJBWV9TSVpFKGFyZ3AtPmlvcHMpKSB7DQo+
ICAJCWFyZ3AtPm9wcyA9IGt6YWxsb2MoYXJncC0+b3BjbnQgKiBzaXplb2Yo
KmFyZ3AtPm9wcyksIEdGUF9LRVJORUwpOw0KPiANCj4gDQo+IC4NCj4gDQoN
Ci0tIA0KTHUgWGlueXUNCi0tDQpCZXN0IFJlZ2FyZHMhDQoKCg==