2012-04-24 20:36:08

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]>
---
Reposted to address comments from Trond and Jim

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

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

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 9c8eca3..8c84e73 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -52,6 +52,30 @@ Elong:
}

/*
+ * return the path component of "<server>:<path>"
+ * nfspath - the "<server>:<path>" string
+ * end - one past the last char that could contain "<server>:"
+ * returns NULL on failure
+ */
+static char *nfs_path_component(const char *nfspath, const char *end)
+{
+ char *p;
+
+ if (*nfspath == '[') {
+ /* parse [] escaped IPv6 addrs */
+ p = strchr(nfspath, ']');
+ if (p != NULL && (p + 1) < end && *(++p) == ':')
+ return p + 1;
+ } else {
+ /* otherwise split on first colon */
+ p = strchr(nfspath, ':');
+ if (p != NULL && p < end)
+ return p + 1;
+ }
+ return NULL;
+}
+
+/*
* Determine the mount path as a string
*/
static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen)
@@ -59,9 +83,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_path_component(path, limit);
+ if (path_component)
+ return path_component;
}
return path;
}
--
1.7.4.4



2012-04-24 15:18:14

by Jim Rees

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

Weston Andros Adamson wrote:

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
+ */

The comment seems odd to me. You're not parsing the path component, you're
parsing the nfspath to return the path. And the term "devname component"
hasn't been defined anywhere. How about this:

Return the path component of an nfs path ("<server>:<path>").
nfspath - the "<server>:<path>" string
end - pointer to end of nfspath
returns NULL on failure

2012-04-24 20:42:16

by Myklebust, Trond

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

T24gVHVlLCAyMDEyLTA0LTI0IGF0IDE2OjM1IC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IEFsbCByZWZlcnJhbHMgKElQdjQgYWRkciwgSVB2NiBhZGRyLCBhbmQgRE5TKSBh
cmUgYnJva2VuIG9uIG1vdW50cyBvZg0KPiBJUHY2IGFkZHJlc3NlcywgYmVjYXVzZSB2YWxpZGF0
aW9uIGNvZGUgdXNlcyBhIHBhdGggdGhhdCBpcyBwYXJzZWQNCj4gZnJvbSB0aGUgZGV2X25hbWUg
KCI8c2VydmVyPjo8cGF0aD4iKSBieSBzcGxpdHRpbmcgb24gdGhlIGZpcnN0IGNvbG9uIGFuZA0K
PiBjb2xvbnMgYXJlIHVzZWQgaW4gSVB2NiBhZGRycy4NCj4gVGhpcyBwYXRjaCBpZ25vcmVzIGNv
bG9ucyB3aXRoaW4gSVB2NiBhZGRyZXNzZXMgdGhhdCBhcmUgZXNjYXBlZCBieSAnWycgYW5kICdd
Jy4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFdlc3RvbiBBbmRyb3MgQWRhbXNvbiA8ZHJvc0BuZXRh
cHAuY29tPg0KPiAtLS0NCj4gUmVwb3N0ZWQgdG8gYWRkcmVzcyBjb21tZW50cyBmcm9tIFRyb25k
IGFuZCBKaW0NCj4gDQo+IFRoaXMgZGVwZW5kcyBvbiBjb21taXQgMmY4ZTRiZDkxNDg4ZjI4NmU4
M2U4YWJiMTQ2ODMxMDJlZmFhZmIwNQ0KPiAibmZzOiBFbmNsb3NlIGhvc3RuYW1lIGluIGJyYWNr
ZXRzIHdoZW4gbmVlZGVkIGluIG5mc19kb19yb290X21vdW50Ii4NCj4gDQo+ICBmcy9uZnMvbmZz
NG5hbWVzcGFjZS5jIHwgICAzMCArKysrKysrKysrKysrKysrKysrKysrKysrKystLS0NCj4gIDEg
ZmlsZXMgY2hhbmdlZCwgMjcgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCj4gDQo+IGRp
ZmYgLS1naXQgYS9mcy9uZnMvbmZzNG5hbWVzcGFjZS5jIGIvZnMvbmZzL25mczRuYW1lc3BhY2Uu
Yw0KPiBpbmRleCA5YzhlY2EzLi44Yzg0ZTczIDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvbmZzNG5h
bWVzcGFjZS5jDQo+ICsrKyBiL2ZzL25mcy9uZnM0bmFtZXNwYWNlLmMNCj4gQEAgLTUyLDYgKzUy
LDMwIEBAIEVsb25nOg0KPiAgfQ0KPiAgDQo+ICAvKg0KPiArICogcmV0dXJuIHRoZSBwYXRoIGNv
bXBvbmVudCBvZiAiPHNlcnZlcj46PHBhdGg+Ig0KPiArICogIG5mc3BhdGggLSB0aGUgIjxzZXJ2
ZXI+OjxwYXRoPiIgc3RyaW5nDQo+ICsgKiAgZW5kIC0gb25lIHBhc3QgdGhlIGxhc3QgY2hhciB0
aGF0IGNvdWxkIGNvbnRhaW4gIjxzZXJ2ZXI+OiINCj4gKyAqIHJldHVybnMgTlVMTCBvbiBmYWls
dXJlDQo+ICsgKi8NCj4gK3N0YXRpYyBjaGFyICpuZnNfcGF0aF9jb21wb25lbnQoY29uc3QgY2hh
ciAqbmZzcGF0aCwgY29uc3QgY2hhciAqZW5kKQ0KPiArew0KPiArCWNoYXIgKnA7DQo+ICsNCj4g
KwlpZiAoKm5mc3BhdGggPT0gJ1snKSB7DQo+ICsJCS8qIHBhcnNlIFtdIGVzY2FwZWQgSVB2NiBh
ZGRycyAqLw0KPiArCQlwID0gc3RyY2hyKG5mc3BhdGgsICddJyk7DQo+ICsJCWlmIChwICE9IE5V
TEwgJiYgKHAgKyAxKSA8IGVuZCAmJiAqKCsrcCkgPT0gJzonKQ0KCQkJCV5eXl5eXl5eXiB3aHkg
bm90ICsrcD8NCg0KPiArCQkJcmV0dXJuIHAgKyAxOw0KPiArCX0gZWxzZSB7DQo+ICsJCS8qIG90
aGVyd2lzZSBzcGxpdCBvbiBmaXJzdCBjb2xvbiAqLw0KPiArCQlwID0gc3RyY2hyKG5mc3BhdGgs
ICc6Jyk7DQo+ICsJCWlmIChwICE9IE5VTEwgJiYgcCA8IGVuZCkNCj4gKwkJCXJldHVybiBwICsg
MTsNCj4gKwl9DQo+ICsJcmV0dXJuIE5VTEw7DQo+ICt9DQo+ICsNCj4gKy8qDQo+ICAgKiBEZXRl
cm1pbmUgdGhlIG1vdW50IHBhdGggYXMgYSBzdHJpbmcNCj4gICAqLw0KPiAgc3RhdGljIGNoYXIg
Km5mczRfcGF0aChzdHJ1Y3QgZGVudHJ5ICpkZW50cnksIGNoYXIgKmJ1ZmZlciwgc3NpemVfdCBi
dWZsZW4pDQo+IEBAIC01OSw5ICs4Myw5IEBAIHN0YXRpYyBjaGFyICpuZnM0X3BhdGgoc3RydWN0
IGRlbnRyeSAqZGVudHJ5LCBjaGFyICpidWZmZXIsIHNzaXplX3QgYnVmbGVuKQ0KPiAgCWNoYXIg
KmxpbWl0Ow0KPiAgCWNoYXIgKnBhdGggPSBuZnNfcGF0aCgmbGltaXQsIGRlbnRyeSwgYnVmZmVy
LCBidWZsZW4pOw0KPiAgCWlmICghSVNfRVJSKHBhdGgpKSB7DQo+IC0JCWNoYXIgKmNvbG9uID0g
c3RyY2hyKHBhdGgsICc6Jyk7DQo+IC0JCWlmIChjb2xvbiAmJiBjb2xvbiA8IGxpbWl0KQ0KPiAt
CQkJcGF0aCA9IGNvbG9uICsgMTsNCj4gKwkJY2hhciAqcGF0aF9jb21wb25lbnQgPSBuZnNfcGF0
aF9jb21wb25lbnQocGF0aCwgbGltaXQpOw0KPiArCQlpZiAocGF0aF9jb21wb25lbnQpDQo+ICsJ
CQlyZXR1cm4gcGF0aF9jb21wb25lbnQ7DQo+ICAJfQ0KPiAgCXJldHVybiBwYXRoOw0KPiAgfQ0K
DQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5l
dEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-04-24 16:20:45

by Adamson, Dros

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


On Apr 24, 2012, at 11:19 AM, Jim Rees wrote:

> Weston Andros Adamson wrote:
>
> 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
> + */
>
> The comment seems odd to me. You're not parsing the path component, you're
> parsing the nfspath to return the path. And the term "devname component"
> hasn't been defined anywhere. How about this:
>
> Return the path component of an nfs path ("<server>:<path>").

Yeah, OK.

> nfspath - the "<server>:<path>" string
> end - pointer to end of nfspath

Well, that's not really true. "end" comes from the call to nfs_path() and points to the end of the devname component of the nfs_path, i.e.:

if "[fc00::10]:/export" is mounted:

[fc00::10]:/export/foo/bar
^end

> returns NULL on failure

-dros


Attachments:
smime.p7s (1.34 kB)