2012-04-24 14:58:10

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH] nfs4: fix referrals on mounts that use IPv6 addrs

All referrals (IPv4 addr, IPv6 addr, and DNS) are broken on mounts of
IPv6 addresses, because validation code uses a path that is parsed
from the dev_name ("<server>:<path>") by splitting on the first colon and
colons are used in IPv6 addrs.
This patch ignores colons within IPv6 addresses that are escaped by '[' and ']'.

Signed-off-by: Weston Andros Adamson <[email protected]>
---

This depends on commit 2f8e4bd91488f286e83e8abb14683102efaafb05
"nfs: Enclose hostname in brackets when needed in nfs_do_root_mount".

fs/nfs/nfs4namespace.c | 35 ++++++++++++++++++++++++++++++++---
1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 9c8eca3..307743d 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -52,6 +52,35 @@ Elong:
}

/*
+ * parse the path component of an nfs path ("<server>:<path>").
+ * nfspath - the "<server>:<path>" string
+ * end - pointer to end of devname component of 'nfspath'
+ * returns NULL on failure
+ */
+static inline char *nfs_parse_path_component(char *nfspath, char *end)
+{
+ bool ipv6_esc = false;
+ char *p;
+
+ /* find first colon not in IPv6 addr */
+ for (p = nfspath; p < end && *p; p++) {
+ switch (*p) {
+ case '[':
+ ipv6_esc = true;
+ break;
+ case ']':
+ ipv6_esc = false;
+ break;
+ case ':':
+ if (!ipv6_esc)
+ return (p + 1);
+ break;
+ }
+ }
+ return NULL;
+}
+
+/*
* Determine the mount path as a string
*/
static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
@@ -59,9 +88,9 @@ static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
char *limit;
char *path = nfs_path(&limit, dentry, buffer, buflen);
if (!IS_ERR(path)) {
- char *colon = strchr(path, ':');
- if (colon && colon < limit)
- path = colon + 1;
+ char *path_component = nfs_parse_path_component(path, limit);
+ if (path_component)
+ return path_component;
}
return path;
}
--
1.7.4.4



2012-04-24 15:33:25

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] nfs4: fix referrals on mounts that use IPv6 addrs

T24gVHVlLCAyMDEyLTA0LTI0IGF0IDEwOjU3IC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IEFsbCByZWZlcnJhbHMgKElQdjQgYWRkciwgSVB2NiBhZGRyLCBhbmQgRE5TKSBh
cmUgYnJva2VuIG9uIG1vdW50cyBvZg0KPiBJUHY2IGFkZHJlc3NlcywgYmVjYXVzZSB2YWxpZGF0
aW9uIGNvZGUgdXNlcyBhIHBhdGggdGhhdCBpcyBwYXJzZWQNCj4gZnJvbSB0aGUgZGV2X25hbWUg
KCI8c2VydmVyPjo8cGF0aD4iKSBieSBzcGxpdHRpbmcgb24gdGhlIGZpcnN0IGNvbG9uIGFuZA0K
PiBjb2xvbnMgYXJlIHVzZWQgaW4gSVB2NiBhZGRycy4NCj4gVGhpcyBwYXRjaCBpZ25vcmVzIGNv
bG9ucyB3aXRoaW4gSVB2NiBhZGRyZXNzZXMgdGhhdCBhcmUgZXNjYXBlZCBieSAnWycgYW5kICdd
Jy4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFdlc3RvbiBBbmRyb3MgQWRhbXNvbiA8ZHJvc0BuZXRh
cHAuY29tPg0KPiAtLS0NCj4gDQo+IFRoaXMgZGVwZW5kcyBvbiBjb21taXQgMmY4ZTRiZDkxNDg4
ZjI4NmU4M2U4YWJiMTQ2ODMxMDJlZmFhZmIwNQ0KPiAibmZzOiBFbmNsb3NlIGhvc3RuYW1lIGlu
IGJyYWNrZXRzIHdoZW4gbmVlZGVkIGluIG5mc19kb19yb290X21vdW50Ii4NCj4gDQo+ICBmcy9u
ZnMvbmZzNG5hbWVzcGFjZS5jIHwgICAzNSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
Ky0tLQ0KPiAgMSBmaWxlcyBjaGFuZ2VkLCAzMiBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygt
KQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0bmFtZXNwYWNlLmMgYi9mcy9uZnMvbmZz
NG5hbWVzcGFjZS5jDQo+IGluZGV4IDljOGVjYTMuLjMwNzc0M2QgMTAwNjQ0DQo+IC0tLSBhL2Zz
L25mcy9uZnM0bmFtZXNwYWNlLmMNCj4gKysrIGIvZnMvbmZzL25mczRuYW1lc3BhY2UuYw0KPiBA
QCAtNTIsNiArNTIsMzUgQEAgRWxvbmc6DQo+ICB9DQo+ICANCj4gIC8qDQo+ICsgKiBwYXJzZSB0
aGUgcGF0aCBjb21wb25lbnQgb2YgYW4gbmZzIHBhdGggKCI8c2VydmVyPjo8cGF0aD4iKS4NCj4g
KyAqICBuZnNwYXRoIC0gdGhlICI8c2VydmVyPjo8cGF0aD4iIHN0cmluZw0KPiArICogIGVuZCAt
IHBvaW50ZXIgdG8gZW5kIG9mIGRldm5hbWUgY29tcG9uZW50IG9mICduZnNwYXRoJw0KPiArICog
cmV0dXJucyBOVUxMIG9uIGZhaWx1cmUNCj4gKyAqLw0KPiArc3RhdGljIGlubGluZSBjaGFyICpu
ZnNfcGFyc2VfcGF0aF9jb21wb25lbnQoY2hhciAqbmZzcGF0aCwgY2hhciAqZW5kKQ0KDQpOaXQ6
IHRoaXMgaXMgcmVhbGx5IHBhcnNpbmcgdGhlIHNlcnZlciBuYW1lIGNvbXBvbmVudCBvZiB0aGUg
ZGV2bmFtZS4NCg0KQWxzbywgcGxlYXNlIG1ha2UgdGhlICdlbmQnIGFyZ3VtZW50ICdjb25zdCBj
aGFyIConLCBhbmQgIGRyb3AgdGhlDQonaW5saW5lJyBkZWNsYXJhdGlvbi4NCg0KPiArew0KPiAr
CWJvb2wgaXB2Nl9lc2MgPSBmYWxzZTsNCj4gKwljaGFyICpwOw0KPiArDQo+ICsJLyogZmluZCBm
aXJzdCBjb2xvbiBub3QgaW4gSVB2NiBhZGRyICovDQo+ICsJZm9yIChwID0gbmZzcGF0aDsgcCA8
IGVuZCAmJiAqcDsgcCsrKSB7DQo+ICsJCXN3aXRjaCAoKnApIHsNCj4gKwkJY2FzZSAnWyc6DQo+
ICsJCQlpcHY2X2VzYyA9IHRydWU7DQoNCk5vdGUgdGhhdCBpZiBwICE9IG5mc3BhdGgsIHRoZW4g
d2UgaGF2ZSBhIHByb2JsZW0uIFRoZSAnWycgX211c3RfIGJlIHRoZQ0KZmlyc3QgY2hhcmFjdGVy
IGluIHRoZSBzZXJ2ZXIgbmFtZSwgYW5kIHRoZXJlIF9tdXN0XyBiZSBhIG1hdGNoaW5nICddJw0K
YXQgdGhlIGVuZC4NClNvIGhvdyBhYm91dCBvcHRpbWlzaW5nIHRoaXMgdXNpbmcgc29tZXRoaW5n
IGFsb25nIHRoZSBsaW5lcyBvZjoNCg0KCXAgPSBuZnNwYXRoOw0KCS8qIEhhbmRsZSBlc2NhcGVk
IGhvc3RuYW1lcyAqLw0KCWlmICgqcCA9PSAnWycpIHsNCgkJcCA9IHN0cmNocihwLCAnXScpOw0K
CQlpZiAocCA9PSBOVUxMKQ0KCQkJcmV0dXJuIE5VTEw7DQoJfQ0KCXAgPSBzdHJjaHIocCwgJzon
KTsNCglpZiAocCAhPSBOVUxMKQ0KCQlwKys7DQoJcmV0dXJuIHA7DQoNCj4gKwkJCWJyZWFrOw0K
PiArCQljYXNlICc6JzoNCj4gKwkJCWlmICghaXB2Nl9lc2MpDQo+ICsJCQkJcmV0dXJuIChwICsg
MSk7DQo+ICsJCQlicmVhazsNCj4gKwkJfQ0KPiArCX0NCj4gKwlyZXR1cm4gTlVMTDsNCj4gK30N
Cj4gKw0KPiArLyoNCj4gICAqIERldGVybWluZSB0aGUgbW91bnQgcGF0aCBhcyBhIHN0cmluZw0K
PiAgICovDQo+ICBzdGF0aWMgY2hhciAqbmZzNF9wYXRoKHN0cnVjdCBkZW50cnkgKmRlbnRyeSwg
Y2hhciAqYnVmZmVyLCBzc2l6ZV90IGJ1ZmxlbikNCj4gQEAgLTU5LDkgKzg4LDkgQEAgc3RhdGlj
IGNoYXIgKm5mczRfcGF0aChzdHJ1Y3QgZGVudHJ5ICpkZW50cnksIGNoYXIgKmJ1ZmZlciwgc3Np
emVfdCBidWZsZW4pDQo+ICAJY2hhciAqbGltaXQ7DQo+ICAJY2hhciAqcGF0aCA9IG5mc19wYXRo
KCZsaW1pdCwgZGVudHJ5LCBidWZmZXIsIGJ1Zmxlbik7DQo+ICAJaWYgKCFJU19FUlIocGF0aCkp
IHsNCj4gLQkJY2hhciAqY29sb24gPSBzdHJjaHIocGF0aCwgJzonKTsNCj4gLQkJaWYgKGNvbG9u
ICYmIGNvbG9uIDwgbGltaXQpDQo+IC0JCQlwYXRoID0gY29sb24gKyAxOw0KPiArCQljaGFyICpw
YXRoX2NvbXBvbmVudCA9IG5mc19wYXJzZV9wYXRoX2NvbXBvbmVudChwYXRoLCBsaW1pdCk7DQo+
ICsJCWlmIChwYXRoX2NvbXBvbmVudCkNCj4gKwkJCXJldHVybiBwYXRoX2NvbXBvbmVudDsNCj4g
IAl9DQo+ICAJcmV0dXJuIHBhdGg7DQo+ICB9DQoNClNvIHRoaXMgZml4ZXMgdGhlIHByb2JsZW0g
b2YgcGFyc2luZyB0aGUgZGV2bmFtZSwgYnV0IGhvdyBhYm91dCB0aGUNCmlzc3VlIG9mIHNldHRp
bmcgdGhlIGNvcnJlY3QgZGV2bmFtZSBpZiB0aGUgc2VydmVyIHN1cHBsaWVzIGFuIElQdjYNCmFk
ZHJlc3MgYXMgdGhlIGhvc3RuYW1lPw0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZT
IGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20N
Cnd3dy5uZXRhcHAuY29tDQoNCg==

2012-04-24 16:18:14

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] nfs4: fix referrals on mounts that use IPv6 addrs


On Apr 24, 2012, at 11:33 AM, Myklebust, Trond wrote:

> On Tue, 2012-04-24 at 10:57 -0400, Weston Andros Adamson wrote:
>> All referrals (IPv4 addr, IPv6 addr, and DNS) are broken on mounts of
>> IPv6 addresses, because validation code uses a path that is parsed
>> from the dev_name ("<server>:<path>") by splitting on the first colon and
>> colons are used in IPv6 addrs.
>> This patch ignores colons within IPv6 addresses that are escaped by '[' and ']'.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>>
>> This depends on commit 2f8e4bd91488f286e83e8abb14683102efaafb05
>> "nfs: Enclose hostname in brackets when needed in nfs_do_root_mount".
>>
>> fs/nfs/nfs4namespace.c | 35 ++++++++++++++++++++++++++++++++---
>> 1 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>> index 9c8eca3..307743d 100644
>> --- a/fs/nfs/nfs4namespace.c
>> +++ b/fs/nfs/nfs4namespace.c
>> @@ -52,6 +52,35 @@ Elong:
>> }
>>
>> /*
>> + * parse the path component of an nfs path ("<server>:<path>").
>> + * nfspath - the "<server>:<path>" string
>> + * end - pointer to end of devname component of 'nfspath'
>> + * returns NULL on failure
>> + */
>> +static inline char *nfs_parse_path_component(char *nfspath, char *end)
>
> Nit: this is really parsing the server name component of the devname.
>
> Also, please make the 'end' argument 'const char *', and drop the
> 'inline' declaration.

OK.

>
>> +{
>> + bool ipv6_esc = false;
>> + char *p;
>> +
>> + /* find first colon not in IPv6 addr */
>> + for (p = nfspath; p < end && *p; p++) {
>> + switch (*p) {
>> + case '[':
>> + ipv6_esc = true;
>
> Note that if p != nfspath, then we have a problem. The '[' _must_ be the
> first character in the server name, and there _must_ be a matching ']'
> at the end.
> So how about optimising this using something along the lines of:
>
> p = nfspath;
> /* Handle escaped hostnames */
> if (*p == '[') {
> p = strchr(p, ']');
> if (p == NULL)
> return NULL;
> }
> p = strchr(p, ':');
> if (p != NULL)
> p++;
> return p;

OK, I'll change it to be more strict. Note that your suggestion doesn't enforce that the matching ']' is at the end of the hostname (next char ':').

>> + break;
>> + case ':':
>> + if (!ipv6_esc)
>> + return (p + 1);
>> + break;
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +/*
>> * Determine the mount path as a string
>> */
>> static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
>> @@ -59,9 +88,9 @@ static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
>> char *limit;
>> char *path = nfs_path(&limit, dentry, buffer, buflen);
>> if (!IS_ERR(path)) {
>> - char *colon = strchr(path, ':');
>> - if (colon && colon < limit)
>> - path = colon + 1;
>> + char *path_component = nfs_parse_path_component(path, limit);
>> + if (path_component)
>> + return path_component;
>> }
>> return path;
>> }
>
> So this fixes the problem of parsing the devname, but how about the
> issue of setting the correct devname if the server supplies an IPv6
> address as the hostname?

That is covered by the patch you sent me (commit 2f8e4bd91488f286e83e8abb14683102efaafb05). I thought you'd just use that patch - it's what I tested with.

-dros


Attachments:
smime.p7s (1.34 kB)