Fix NFS option parsing bit-rot.
Subsystems (2)
PATCH 1/1 Fix NFS option parsing bit-rot.
From: Rob Landley <[email protected]>
The kernel has string parsing code for NFS mount options, but it seems to have bit-rotted over the years, so toybox mount needs the following patch to be able to mount nfs. Without it, the kernel returns "invalid argument" before sending any network traffic.
For more information, see
http://lists.landley.net/pipermail/toybox-landley.net/2016-March/004790.html
---
fs/nfs/super.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
fs/nfs/super.c +13 -5
@@ -2130,11 +2130,23 @@ static int nfs_validate_text_mount_data(void *options,int port =0 ;int max_namelen =PAGE_SIZE ;int max_pathlen =NFS_MAXPATHLEN ; +int rc;struct sockaddr *sap = (struct sockaddr *)&args->nfs_server.address;if (nfs_parse_mount_options ((char *)options, args) ==0 )return -EINVAL ; + rc =nfs_parse_devname (dev_name, + &args->nfs_server.hostname, + max_namelen, + &args->nfs_server.export_path, + max_pathlen); + + args->nfs_server.addrlen =rpc_pton (args->net, + args->nfs_server.hostname, +strlen (args->nfs_server.hostname), + sap,sizeof (args->nfs_server.address)); +if (!nfs_verify_server_address (sap)) goto out_no_address;
@@ -2155,11 +2167,7 @@ static int nfs_validate_text_mount_data(void *options,nfs_set_port (sap, &args->nfs_server.port, port); -return nfs_parse_devname (dev_name, - &args->nfs_server.hostname, - max_namelen, - &args->nfs_server.export_path, - max_pathlen); +return rc;#if !IS_ENABLED (CONFIG_NFS_V4 ) out_v4_not_compiled:
DQoNCk9uIDYvNi8xNiwgMTg6NTgsICJSb2IgTGFuZGxleSIgPHJvYkBsYW5kbGV5Lm5ldD4gd3Jv dGU6DQoNCj5Gcm9tOiBSb2IgTGFuZGxleSA8cm9iQGxhbmRsZXkubmV0Pg0KPg0KPlRoZSBrZXJu ZWwgaGFzIHN0cmluZyBwYXJzaW5nIGNvZGUgZm9yIE5GUyBtb3VudCBvcHRpb25zLCBidXQgaXQg c2VlbXMNCj50byBoYXZlIGJpdC1yb3R0ZWQgb3ZlciB0aGUgeWVhcnMsIHNvIHRveWJveCBtb3Vu dCBuZWVkcyB0aGUgZm9sbG93aW5nDQo+cGF0Y2ggdG8gYmUgYWJsZSB0byBtb3VudCBuZnMuIFdp dGhvdXQgaXQsIHRoZSBrZXJuZWwgcmV0dXJucyAiaW52YWxpZA0KPmFyZ3VtZW50IiBiZWZvcmUg c2VuZGluZyBhbnkgbmV0d29yayB0cmFmZmljLg0KPg0KPkZvciBtb3JlIGluZm9ybWF0aW9uLCBz ZWUNCj5odHRwOi8vbGlzdHMubGFuZGxleS5uZXQvcGlwZXJtYWlsL3RveWJveC1sYW5kbGV5Lm5l dC8yMDE2LU1hcmNoLzAwNDc5MC5odG1sDQo+DQo+U2lnbmVkLW9mZi1ieTogUm9iIExhbmRsZXkg PHJvYkBsYW5kbGV5Lm5ldD4NCj4tLS0NCj4NCj4gZnMvbmZzL3N1cGVyLmMgfCAgIDE4ICsrKysr KysrKysrKystLS0tLQ0KPiAxIGZpbGUgY2hhbmdlZCwgMTMgaW5zZXJ0aW9ucygrKSwgNSBkZWxl dGlvbnMoLSkNCj4NCj5kaWZmIC0tZ2l0IGEvZnMvbmZzL3N1cGVyLmMgYi9mcy9uZnMvc3VwZXIu Yw0KPmluZGV4IDIxMzdlMDIuLjg2MzU4NWQgMTAwNjQ0DQo+LS0tIGEvZnMvbmZzL3N1cGVyLmMN Cj4rKysgYi9mcy9uZnMvc3VwZXIuYw0KPkBAIC0yMTMwLDExICsyMTMwLDIzIEBAIHN0YXRpYyBp bnQgbmZzX3ZhbGlkYXRlX3RleHRfbW91bnRfZGF0YSh2b2lkICpvcHRpb25zLA0KPiAJaW50IHBv cnQgPSAwOw0KPiAJaW50IG1heF9uYW1lbGVuID0gUEFHRV9TSVpFOw0KPiAJaW50IG1heF9wYXRo bGVuID0gTkZTX01BWFBBVEhMRU47DQo+KwlpbnQgcmM7DQo+IAlzdHJ1Y3Qgc29ja2FkZHIgKnNh cCA9IChzdHJ1Y3Qgc29ja2FkZHIgKikmYXJncy0+bmZzX3NlcnZlci5hZGRyZXNzOw0KPiANCj4g CWlmIChuZnNfcGFyc2VfbW91bnRfb3B0aW9ucygoY2hhciAqKW9wdGlvbnMsIGFyZ3MpID09IDAp DQo+IAkJcmV0dXJuIC1FSU5WQUw7DQo+IA0KPisJcmMgPSBuZnNfcGFyc2VfZGV2bmFtZShkZXZf bmFtZSwNCj4rCQkJCSAgICZhcmdzLT5uZnNfc2VydmVyLmhvc3RuYW1lLA0KPisJCQkJICAgbWF4 X25hbWVsZW4sDQo+KwkJCQkgICAmYXJncy0+bmZzX3NlcnZlci5leHBvcnRfcGF0aCwNCj4rCQkJ CSAgIG1heF9wYXRobGVuKTsNCj4rDQo+KwlhcmdzLT5uZnNfc2VydmVyLmFkZHJsZW4gPSBycGNf cHRvbihhcmdzLT5uZXQsDQo+KwkJCWFyZ3MtPm5mc19zZXJ2ZXIuaG9zdG5hbWUsDQo+KwkJCXN0 cmxlbihhcmdzLT5uZnNfc2VydmVyLmhvc3RuYW1lKSwNCj4rCQkJc2FwLCBzaXplb2YoYXJncy0+ bmZzX3NlcnZlci5hZGRyZXNzKSk7DQo+Kw0KDQpUaGF0IHdpbGwgc2NyaWJibGUgb3ZlciB0aGUg cGFyc2VkIGFkZHJlc3MuDQoNClRyb25kDQoNCg0K
Quoted 11 lines · Trond Myklebust
Quoted 9 lines · Rob Landley
From: Rob Landley <[email protected]>
The kernel has string parsing code for NFS mount options, but it seems to have bit-rotted over the years, so toybox mount needs the following patch to be able to mount nfs. Without it, the kernel returns "invalid argument" before sending any network traffic.
For more information, see
http://lists.landley.net/pipermail/toybox-landley.net/2016-March/004790.html
...
Quoted 1 line
That will scribble over the parsed address.
You mean if you supply both -o addr=host and host:/ path name? Because you can't leave off host:/ or it errors. If you do:
mount("/tmp","/mnt","nfs",1,
"port=9999,mountport=9999,nolock,v3,udp,addr=10.0.2.2");It barfs because nfs_parse_devname() does:
end = strchr(dev_name, ':');
if (end == NULL)
goto out_bad_devname;
...out_bad_devname:
dfprintk(MOUNT, "NFS: device name not in host:path format\n"); return -EINVAL;
So addr= is at _best_ redundant. You MUST supply host:/ always, the current code just wasn't using it. My patch makes it use it.
There's no other way to get the address set for the text parsing path, when we enter nfs_validate_text_mount_data() (the function I patched), mount_info.parsed will always be zero.
But sure, he's a gratuitous for loop checking that you haven't redundantly supplied both -o addr=host and host://path
fs/nfs/super.c +18 -5
@@ -2130,11 +2130,28 @@ static int nfs_validate_text_mount_data(void *options,int port =0 ;int max_namelen =PAGE_SIZE ;int max_pathlen =NFS_MAXPATHLEN ; +int rc; +int i; +char *c;struct sockaddr *sap = (struct sockaddr *)&args->nfs_server.address;if (nfs_parse_mount_options ((char *)options, args) ==0 )return -EINVAL ; + rc =nfs_parse_devname (dev_name, + &args->nfs_server.hostname, + max_namelen, + &args->nfs_server.export_path, + max_pathlen); + +for (i =0 , c = (void *)sap; i<sizeof (*sap); i++) +if (c[i])break ; +if (i ==sizeof (*sap)) + args->nfs_server.addrlen =rpc_pton (args->net, + args->nfs_server.hostname, +strlen (args->nfs_server.hostname), + sap,sizeof (args->nfs_server.address)); +if (!nfs_verify_server_address (sap)) goto out_no_address;
@@ -2155,11 +2172,7 @@ static int nfs_validate_text_mount_data(void *options,nfs_set_port (sap, &args->nfs_server.port, port); -return nfs_parse_devname (dev_name, - &args->nfs_server.hostname, - max_namelen, - &args->nfs_server.export_path, - max_pathlen); +return rc;#if !IS_ENABLED (CONFIG_NFS_V4 ) out_v4_not_compiled:
DQoNCk9uIDYvOC8xNiwgMTU6NDEsICJSb2IgTGFuZGxleSIgPHJvYkBsYW5kbGV5Lm5ldD4gd3Jv dGU6DQo+U28gYWRkcj0gaXMgYXQgX2Jlc3RfIHJlZHVuZGFudC4gWW91IE1VU1Qgc3VwcGx5IGhv c3Q6LyBhbHdheXMsDQo+dGhlIGN1cnJlbnQgY29kZSBqdXN0IHdhc24ndCB1c2luZyBpdC4NCg0K VGhhdOKAmXMgYmVjYXVzZSB5b3UganVzdCBoYXBwZW4gdG8gYmUgc3VwcGx5aW5nIGFuIElQIGFk ZHJlc3MgaW5zdGVhZCBvZiBhIGhvc3RuYW1lLiBUaGUga2VybmVsIGhhcyBubyBETlMgcmVzb2x2 aW5nIGZ1bmN0aW9uYWxpdHkuIEl0IGNhbm5vdCByZXNvbHZlIGhvc3RuYW1lcyBpbnRvIElQIGFk ZHJlc3NlcyB3aXRob3V0IGhlbHAgZWl0aGVyIGJ5IHRoZSBjYWxsZXIgb3IgYnkgbWVhbnMgb2Yg YW4gdXBjYWxsLiBUaGF0IGlzIHdoeSDigJhhZGRyPeKAmSBoYXMgYmVlbiBhIG1hbmRhdG9yeSBw YXJhbWV0ZXIgZXZlciBzaW5jZSB3ZSBpbnRyb2R1Y2VkIHRoZSB0ZXh0IGJhc2VkIHBhcnNlci4N Cg0KTm93IHdlIGNvdWxkLCB0aGVvcmV0aWNhbGx5LCBoYXZlIHRoZSBjbGllbnQgY2FsbCBuZnNf ZG5zX3Jlc29sdmVfbmFtZSgpIG9uIHRoZSBob3N0bmFtZSB0byByZXNvbHZlIGl0LiBIb3dldmVy IHRoYXQgYnJlYWtzIHdoZW4geW91IGhhdmUgbmV0IG5hbWVzcGFjZXMgYW5kIHN1Y2gsIHNpbmNl IHRoZSBrZXJuZWzigJlzIGRuc19xdWVyeSgpIGNhbGwgaXMgbm90IGNvbnRhaW5lciBhd2FyZS4N Cg0KVHJvbmQNCg0K
Quoted 7 lines · Trond Myklebust
=20
=20Quoted 2 lines · Rob Landley
So addr=3D is at _best_ redundant. You MUST supply host:/ always, the current code just wasn't using it.
=20
That=E2=80=99s because you just happen to be supplying an IP address inst=
ead of
Quoted 3 lines
a hostname. The kernel has no DNS resolving functionality. It cannot resolve hostnames into IP addresses without help either by the caller or by means of an upcall. That is why =E2=80=98addr=3D=E2=80=99 has been a m=
andatory
Quoted 1 line
parameter ever since we introduced the text based parser.
Then why is host: mandatory in the dev name?
Rob