nfs4_path() was parsing the path component by splitting on the first colon.
This is wrong when an IPv6 address is used to mount a server.
For example, having mounted 'fc00::10:/export', nfs4_path() returned
':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
in nfs4_validate_fspath().
Parsing the path component by using the *last* colon works with
IPv6 as well as IPv4 addrs.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4namespace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 9c8eca3..dd3dd30 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -59,7 +59,7 @@ 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, ':');
+ char *colon = strrchr(path, ':');
if (colon && colon < limit)
path = colon + 1;
}
--
1.7.4.4
T24gRnJpLCAyMDEyLTA0LTIwIGF0IDIwOjUxICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBBcHIgMjAsIDIwMTIsIGF0IDQ6MTMgUE0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+
IA0KPiA+IE9uIEZyaSwgMjAxMi0wNC0yMCBhdCAxOTo1NyArMDAwMCwgQWRhbXNvbiwgRHJvcyB3
cm90ZToNCj4gPj4gT24gQXByIDIwLCAyMDEyLCBhdCAzOjQ1IFBNLCBKaW0gUmVlcyB3cm90ZToN
Cj4gPj4gDQo+ID4+PiBXZXN0b24gQW5kcm9zIEFkYW1zb24gd3JvdGU6DQo+ID4+PiANCj4gPj4+
IG5mczRfcGF0aCgpIHdhcyBwYXJzaW5nIHRoZSBwYXRoIGNvbXBvbmVudCBieSBzcGxpdHRpbmcg
b24gdGhlIGZpcnN0IGNvbG9uLg0KPiA+Pj4gVGhpcyBpcyB3cm9uZyB3aGVuIGFuIElQdjYgYWRk
cmVzcyBpcyB1c2VkIHRvIG1vdW50IGEgc2VydmVyLg0KPiA+Pj4gDQo+ID4+PiBGb3IgZXhhbXBs
ZSwgaGF2aW5nIG1vdW50ZWQgJ2ZjMDA6OjEwOi9leHBvcnQnLCBuZnM0X3BhdGgoKSByZXR1cm5l
ZA0KPiA+Pj4gJzoxMDovZXhwb3J0Jy4gIFRoaXMgY2F1c2VzIHJlZmVycmFscyAodXNpbmcgSVB2
NCBvciBJUHY2IGFkZHJlc3NlcykgdG8gZmFpbA0KPiA+Pj4gaW4gbmZzNF92YWxpZGF0ZV9mc3Bh
dGgoKS4NCj4gPj4+IA0KPiA+Pj4gUGFyc2luZyB0aGUgcGF0aCBjb21wb25lbnQgYnkgdXNpbmcg
dGhlICpsYXN0KiBjb2xvbiB3b3JrcyB3aXRoDQo+ID4+PiBJUHY2IGFzIHdlbGwgYXMgSVB2NCBh
ZGRycy4NCj4gPj4+IA0KPiA+Pj4gV2hhdCBpZiB5b3VyIG1vdW50IGlzOg0KPiA+Pj4gDQo+ID4+
PiBzZXJ2ZXIuZWR1Oi9leHBvcnQvOkktbGlrZS1jb2xvbnM6DQo+ID4+PiANCj4gPj4+IEl0IHNl
ZW1zIHRvIG1lIHNvbWV0aGluZyBoYXMgdG8gZ2l2ZS4gIEVpdGhlciB3ZSByZXF1aXJlIHY2IGFk
ZHJlc3NlcyBiZQ0KPiA+Pj4gZW5jbG9zZWQgaW4gW10sIGV4cG9ydCBkaXJzIHN0YXJ0IHdpdGgg
Ii8iLCBvciBleHBvcnRzIGhhdmUgbm8gIjoiLg0KPiA+PiANCj4gPj4gDQo+ID4+IFllYWgsIHlv
dSdyZSByaWdodC4NCj4gPj4gDQo+ID4+IEFsdGhvdWdoIElQdjYgYWRkcnMgbXVzdCBiZSBlbmNs
b3NlZCBpbiBbXSB0byB3b3JrIHdpdGggbW91bnQsIGl0J3MgYWx3YXlzIGRpc3BsYXllZCBpbiB0
aGUga2VybmVsIHdpdGhvdXQgdGhlIFtdLg0KPiA+PiANCj4gPj4gSSBzdXBwb3NlIHRoZSBjb3Jy
ZWN0IGZpeCBpcyB0byBhbHdheXMgZGlzcGxheSBJUHY2IGFkZHJlc3NlcyBlbmNsb3NlZCBpbiBb
XSwgd2hpY2ggbWlnaHQgdG91Y2ggYSAqbG90KiBvZiBwbGFjZXMuDQo+ID4+IA0KPiA+PiBUaG91
Z2h0cz8gIEknbGwgd2FpdCBmb3Igb3RoZXJzIHRvIGNoaW1lIGluIGJlZm9yZSBJIGdvIGZpeGlu
ZyB0aGlzLiBJIGhhdmUgYSBzZXQgb2YgbmZzZCBhbmQgbW91bnRkIHBhdGNoZXMgdGhhdCBkZWFs
IHdpdGggc2ltaWxhciBpc3N1ZXMgb24gdGhlIHNlcnZlciBzaWRlLiAgSSdsbCBjbGVhbiB0aG9z
ZSB1cCBhbmQgc3VibWl0IHRoZW0gYmVmb3JlIGdldHRpbmcgYmFjayB0byB0aGlzLg0KPiA+IA0K
PiA+IFdlIGRvIGtub3cgd2hpY2ggcGFydCBpcyB0aGUgaG9zdG5hbWUsIGFuZCB3aGljaCBpcyB0
aGUgcGF0aG5hbWUuIElmIHlvdQ0KPiA+IGxvb2sgYXQgInRyeV9sb2NhdGlvbigpIiwgeW91J2xs
IHNlZSB0aGF0IHRoZSBob3N0bmFtZSBpcyBzdG9yZWQgaW4NCj4gPiBsb2NhdGlvbi0+c2VydmVy
cywgYW5kIGlzIHRoZW4gY29waWVkIGludG8gdGhpcyBlbXB0eSBidWZmZXIuDQo+ID4gDQo+ID4g
SWYgeW91IHdhbnQgdG8gdGVzdCBpZiB0aGF0IGlzIGFuIElQdjYgYWRkcmVzcyBzbyB0aGF0IHlv
dSBjYW4gZW5jbG9zZQ0KPiA+IGl0IGluIFtdLCB0aGVuIHRoYXQgc2hvdWxkIGJlIGZhaXJseSBl
YXN5IHRvIGRvIHJpZ2h0IHRoZXJl4oCmDQo+IA0KPiBSaWdodCwgd2UgaGF2ZSBzZXBhcmF0ZSBo
b3N0bmFtZSBhbmQgcGF0aG5hbWUgZm9yIGVhY2ggZnNfbG9jYXRpb240LCBidXQgSSdtIHRhbGtp
bmcgYWJvdXQgdGhlIHJldHVybiB2YWx1ZSBvZiBuZnNfcGF0aCgpIHdoaWNoIGlzIHVzZWQgdG8g
ZGV0ZXJtaW5lIHRoZSBzZXJ2ZXItc2lkZSBwYXRoIG9uIGFuIGFyYml0cmFyeSBkZW50cnkgb24g
dGhlICpjdXJyZW50KiBtb3VudHBvaW50IChub3QgdGhlIHJlZmVycmFsIHNlcnZlcikuDQo+IFRo
ZSBwYXRoIHBhcnQgb2YgbmZzX3BhdGgoKSBpcyB0aGVuIGNvbXBhcmVkIGFnYWluc3QgdGhlIGZz
X3BhdGggb2YgdGhlIG5mczRfZnNfbG9jYXRpb25zIHN0cnVjdCAodGhpcyBpcyBhbGwgaW4gbmZz
NF92YWxpZGF0ZV9mc3BhdGgpLg0KPiANCj4gSGF2aW5nIG5mc19wYXRoIHJldHVybiBbXSB3cmFw
cGVkIElQdjYgYWRkcmVzc2VzIGJhc2ljYWxseSBtZWFucyBjaGFuZ2luZyB0aGUgZGV2bmFtZSB0
byB0aGF0IGZvcm1hdCAoZF9mc2RhdGEgaXMgdXNlZCBieSBuZnNfcGF0aCgpKS4gIEknbSBjb25j
ZXJuZWQgYWJvdXQgY2hhbmdpbmcgdGhlIGZvcm1hdCBvZiBkZXZuYW1lIC0gaXRzIHVzZWQgYWxs
IG92ZXIgdGhlIHBsYWNlLg0KPiANCj4gU28gdGhlIG9wdGlvbnMgYXJlOg0KPiAgMSkgY2hhbmdl
IHRoZSBmb3JtYXQgb2YgZGV2bmFtZQ0KPiAgMikgY2hhbmdlIGhvdyBuZnM0X3ZhbGlkYXRlX2Zz
cGF0aCBnZXRzIHRoZSBzZXJ2ZXItc2lkZSBwYXRoIGNvbXBvbmVudCBvZiB0aGUgY3VycmVudCBt
b3VudCAoZG9uJ3QgdXNlIG5mc19wYXRoKCkpDQo+IA0KPiBJJ2xsIHRyeSBvcHRpb24gMSBhbmQg
c2VlIHdoYXQgYnJlYWtzLg0KDQpUaGF0IHJlYWxseSBzaG91bGQgbm90IGJlIG5lY2Vzc2FyeS4N
Cg0KSWYgeW91IGFyZSB1c2luZyBhbiBJUHY2IGFkZHJlc3MsIHlvdSBhcmUgc3VwcG9zZWQgdG8g
bW91bnQgdXNpbmcgdGhlIFtdDQpmb3JtYXQuIElmIG5vdCwgdGhlbiBleHBlY3QgdGhpbmdzIHRv
IGJyZWFrLg0KDQpUaGUgcmVhbCBidWcgaGVyZSBpcyBpbiBuZnM0X3BhdGg6IGl0IHRyaWVzIHRv
IHBhcnNlIHRoZSAnOicgdGhhdCBhcmUNCnBhcnQgb2YgdGhlIElQdjYgYWRkcmVzcyBldmVuIGlm
IHdlIHVzZSB0aGUgc3F1YXJlIGJyYWNrZXQgZGVsaW1pdGVycy4NCkluc3RlYWQsIHdlIHNob3Vs
ZCBiZSBmb2xsb3dpbmcgdGhlIGV4YW1wbGUgb2YgbmZzX3BhcnNlX2Rldm5hbWUuDQoNCkkgc3Vn
Z2VzdCBicmVha2luZyBvdXQgdGhlIGZpcnN0IGhhbGYgb2YgbmZzX3BhcnNlX2Rldm5hbWUgaW50
byBhDQpmdW5jdGlvbiB0aGF0IHJldHVybnMgdGhlIGhvc3QgbmFtZSBwYXJ0IG9mIGEgbW91bnQg
cGF0aCwgYW5kIHRoYXQgY2FuDQpiZSBjYWxsZWQgYnkgbmZzNF9wYXRoIHRvby4NCg0KLS0gDQpU
cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=
On Fri, Apr 20, 2012 at 9:21 PM, Weston Andros Adamson <[email protected]> wrote:
> nfs4_path() was parsing the path component by splitting on the first colon.
> This is wrong when an IPv6 address is used to mount a server.
>
> For example, having mounted 'fc00::10:/export', nfs4_path() returned
> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
> in nfs4_validate_fspath().
>
> Parsing the path component by using the *last* colon works with
> IPv6 as well as IPv4 addrs.
>
what about
http://www.ietf.org/rfc/rfc2732.txt
Tigran.
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4namespace.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index 9c8eca3..dd3dd30 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -59,7 +59,7 @@ 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, ':');
> + char *colon = strrchr(path, ':');
> if (colon && colon < limit)
> path = colon + 1;
> }
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gRnJpLCAyMDEyLTA0LTIwIGF0IDIxOjMxICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBBcHIgMjAsIDIwMTIsIGF0IDU6MjUgUE0sIEFkYW1zb24sIERyb3Mgd3JvdGU6DQo+IFRv
IGJlIGNsZWFyOiB0aGUgcHJvYmxlbSBpcyAqbm90KiB3aXRoIElQdjYgYWRkcnMgaW4gRlNfTE9D
QVRJT05TIGF0dHJzLiAgVGhleSB3b3JrIGlmIHRoZSBwYXJlbnQgbW91bnQgaXMgSVB2NC9kbnMu
ICBUaGUgcHJvYmxlbSBpcyB3aGVuIHRoZSBwYXJlbnQgbW91bnQgaXMgSVB2NiAqbm8qIHJlZmVy
cmFscyB3b3JrIChJUHY0LCBJUHY2IG9yIEROUyksIGJlY2F1c2Ugb2YgdGhlIGlzc3VlIGRlc2Ny
aWJlZCBhYm92ZS4NCg0KVGhlcmUgaXMgYSBwcm9ibGVtIHRoZXJlIHRvby4gSWYgeW91IGRvbid0
IGJyYWNrZXQgdGhlIElQdjYgYWRkcmVzcyB0aGF0DQp5b3UgZ2V0IGZyb20gZnNfbG9jYXRpb25z
LCB0aGVuIHlvdSdsbCBoaXQgdGhlIHNhbWUgcHJvYmxlbSBpZiB5b3UgaGl0IGENCnJlZmVycmFs
IG9uIHRoZSByZWZlcnJlZC10byBzZXJ2ZXIuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51
eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBw
LmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K
Adamson, Dros wrote:
Who the hell thought it was a good idea for IPv6 to use ':' as a
delimiter!? Is it paranoid to think they designed it that way to make our
lives harder? ;)
I remember commenting on this at an IETF in, like, 1992. Even back then
before the web ":" was often used to separate IP address from port. The
response, as I recall, was that we'd never use numeric addresses at the
application level, only domain names.
On Apr 20, 2012, at 3:45 PM, Jim Rees wrote:
> Weston Andros Adamson wrote:
>
> nfs4_path() was parsing the path component by splitting on the first colon.
> This is wrong when an IPv6 address is used to mount a server.
>
> For example, having mounted 'fc00::10:/export', nfs4_path() returned
> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
> in nfs4_validate_fspath().
>
> Parsing the path component by using the *last* colon works with
> IPv6 as well as IPv4 addrs.
>
> What if your mount is:
>
> server.edu:/export/:I-like-colons:
>
> It seems to me something has to give. Either we require v6 addresses be
> enclosed in [], export dirs start with "/", or exports have no ":".
Yeah, you're right.
Although IPv6 addrs must be enclosed in [] to work with mount, it's always displayed in the kernel without the [].
I suppose the correct fix is to always display IPv6 addresses enclosed in [], which might touch a *lot* of places.
Thoughts? I'll wait for others to chime in before I go fixing this. I have a set of nfsd and mountd patches that deal with similar issues on the server side. I'll clean those up and submit them before getting back to this.
Who the hell thought it was a good idea for IPv6 to use ':' as a delimiter!? Is it paranoid to think they designed it that way to make our lives harder? ;)
-dros
Weston Andros Adamson wrote:
nfs4_path() was parsing the path component by splitting on the first colon.
This is wrong when an IPv6 address is used to mount a server.
For example, having mounted 'fc00::10:/export', nfs4_path() returned
':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
in nfs4_validate_fspath().
Parsing the path component by using the *last* colon works with
IPv6 as well as IPv4 addrs.
What if your mount is:
server.edu:/export/:I-like-colons:
It seems to me something has to give. Either we require v6 addresses be
enclosed in [], export dirs start with "/", or exports have no ":".
On Fri, 2012-04-20 at 21:48 +0000, Adamson, Dros wrote:
> On Apr 20, 2012, at 5:43 PM, Myklebust, Trond wrote:
>
> > On Fri, 2012-04-20 at 21:38 +0000, Adamson, Dros wrote:
> >
> >> Yes, something like that could work, but we still have the same issue - dentry->d_fsdata never has [] escaping.
> >
> > That's a bug too...
>
> Right! This is what I've been talking about the whole time - I guess I didn't do a good job explaining it :) The original bug (splitting return of nfs_path()) would be trivial to fix if d_fsdata had [] escaping.
>
> So I guess that answers my earlier question - we *do* want to change dev_name to include [] escaping.
>
> -dros
Here is a patch from Jan Kara that does just this. I'll start by
applying that...
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Apr 20, 2012, at 5:39 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 21:31 +0000, Adamson, Dros wrote:
>> On Apr 20, 2012, at 5:25 PM, Adamson, Dros wrote:
>> To be clear: the problem is *not* with IPv6 addrs in FS_LOCATIONS attrs. They work if the parent mount is IPv4/dns. The problem is when the parent mount is IPv6 *no* referrals work (IPv4, IPv6 or DNS), because of the issue described above.
>
> There is a problem there too. If you don't bracket the IPv6 address that
> you get from fs_locations, then you'll hit the same problem if you hit a
> referral on the referred-to server.
Ok, but that's a moot point at this stage as [] escaping is always getting stripped from dentry->d_fsdata (really from dev_name passed to mount as far as I can tell).
-dros
T24gRnJpLCAyMDEyLTA0LTIwIGF0IDE5OjU3ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBBcHIgMjAsIDIwMTIsIGF0IDM6NDUgUE0sIEppbSBSZWVzIHdyb3RlOg0KPiANCj4gPiBX
ZXN0b24gQW5kcm9zIEFkYW1zb24gd3JvdGU6DQo+ID4gDQo+ID4gIG5mczRfcGF0aCgpIHdhcyBw
YXJzaW5nIHRoZSBwYXRoIGNvbXBvbmVudCBieSBzcGxpdHRpbmcgb24gdGhlIGZpcnN0IGNvbG9u
Lg0KPiA+ICBUaGlzIGlzIHdyb25nIHdoZW4gYW4gSVB2NiBhZGRyZXNzIGlzIHVzZWQgdG8gbW91
bnQgYSBzZXJ2ZXIuDQo+ID4gDQo+ID4gIEZvciBleGFtcGxlLCBoYXZpbmcgbW91bnRlZCAnZmMw
MDo6MTA6L2V4cG9ydCcsIG5mczRfcGF0aCgpIHJldHVybmVkDQo+ID4gICc6MTA6L2V4cG9ydCcu
ICBUaGlzIGNhdXNlcyByZWZlcnJhbHMgKHVzaW5nIElQdjQgb3IgSVB2NiBhZGRyZXNzZXMpIHRv
IGZhaWwNCj4gPiAgaW4gbmZzNF92YWxpZGF0ZV9mc3BhdGgoKS4NCj4gPiANCj4gPiAgUGFyc2lu
ZyB0aGUgcGF0aCBjb21wb25lbnQgYnkgdXNpbmcgdGhlICpsYXN0KiBjb2xvbiB3b3JrcyB3aXRo
DQo+ID4gIElQdjYgYXMgd2VsbCBhcyBJUHY0IGFkZHJzLg0KPiA+IA0KPiA+IFdoYXQgaWYgeW91
ciBtb3VudCBpczoNCj4gPiANCj4gPiBzZXJ2ZXIuZWR1Oi9leHBvcnQvOkktbGlrZS1jb2xvbnM6
DQo+ID4gDQo+ID4gSXQgc2VlbXMgdG8gbWUgc29tZXRoaW5nIGhhcyB0byBnaXZlLiAgRWl0aGVy
IHdlIHJlcXVpcmUgdjYgYWRkcmVzc2VzIGJlDQo+ID4gZW5jbG9zZWQgaW4gW10sIGV4cG9ydCBk
aXJzIHN0YXJ0IHdpdGggIi8iLCBvciBleHBvcnRzIGhhdmUgbm8gIjoiLg0KPiANCj4gDQo+IFll
YWgsIHlvdSdyZSByaWdodC4NCj4gDQo+IEFsdGhvdWdoIElQdjYgYWRkcnMgbXVzdCBiZSBlbmNs
b3NlZCBpbiBbXSB0byB3b3JrIHdpdGggbW91bnQsIGl0J3MgYWx3YXlzIGRpc3BsYXllZCBpbiB0
aGUga2VybmVsIHdpdGhvdXQgdGhlIFtdLg0KPiANCj4gSSBzdXBwb3NlIHRoZSBjb3JyZWN0IGZp
eCBpcyB0byBhbHdheXMgZGlzcGxheSBJUHY2IGFkZHJlc3NlcyBlbmNsb3NlZCBpbiBbXSwgd2hp
Y2ggbWlnaHQgdG91Y2ggYSAqbG90KiBvZiBwbGFjZXMuDQo+IA0KPiBUaG91Z2h0cz8gIEknbGwg
d2FpdCBmb3Igb3RoZXJzIHRvIGNoaW1lIGluIGJlZm9yZSBJIGdvIGZpeGluZyB0aGlzLiBJIGhh
dmUgYSBzZXQgb2YgbmZzZCBhbmQgbW91bnRkIHBhdGNoZXMgdGhhdCBkZWFsIHdpdGggc2ltaWxh
ciBpc3N1ZXMgb24gdGhlIHNlcnZlciBzaWRlLiAgSSdsbCBjbGVhbiB0aG9zZSB1cCBhbmQgc3Vi
bWl0IHRoZW0gYmVmb3JlIGdldHRpbmcgYmFjayB0byB0aGlzLg0KDQpXZSBkbyBrbm93IHdoaWNo
IHBhcnQgaXMgdGhlIGhvc3RuYW1lLCBhbmQgd2hpY2ggaXMgdGhlIHBhdGhuYW1lLiBJZiB5b3UN
Cmxvb2sgYXQgInRyeV9sb2NhdGlvbigpIiwgeW91J2xsIHNlZSB0aGF0IHRoZSBob3N0bmFtZSBp
cyBzdG9yZWQgaW4NCmxvY2F0aW9uLT5zZXJ2ZXJzLCBhbmQgaXMgdGhlbiBjb3BpZWQgaW50byB0
aGlzIGVtcHR5IGJ1ZmZlci4NCg0KSWYgeW91IHdhbnQgdG8gdGVzdCBpZiB0aGF0IGlzIGFuIElQ
djYgYWRkcmVzcyBzbyB0aGF0IHlvdSBjYW4gZW5jbG9zZQ0KaXQgaW4gW10sIHRoZW4gdGhhdCBz
aG91bGQgYmUgZmFpcmx5IGVhc3kgdG8gZG8gcmlnaHQgdGhlcmUuLi4NCg0KLS0gDQpUcm9uZCBN
eWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15
a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=
On Apr 20, 2012, at 5:25 PM, Adamson, Dros wrote:
> On Apr 20, 2012, at 3:30 PM, Chuck Lever wrote:
>
>>
>> On Apr 20, 2012, at 3:21 PM, Weston Andros Adamson wrote:
>>
>>> nfs4_path() was parsing the path component by splitting on the first colon.
>>> This is wrong when an IPv6 address is used to mount a server.
>>>
>>> For example, having mounted 'fc00::10:/export', nfs4_path() returned
>>> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
>>> in nfs4_validate_fspath().
>>>
>>> Parsing the path component by using the *last* colon works with
>>> IPv6 as well as IPv4 addrs.
>>
>> These ad hoc fixes give me the willies.
>>
>> During a referral, how is the server name and export path getting recombined? In fs_locations data, these are separate. In the forward mount path, IPv6 addresses are escaped via square brackets.
>
> Again, the string that needs to be split into (server, path) is not the fs_locations side -- those are already separated. What's not separated is the result of nfs_path().
>
>> (Having not looked at this code in years), is there some way we can keep the server name and path separate here so we don't have to reparse?
>
> nfs_path() walks up the dentry tree until it hits the root dentry of the nfs mount, then it uses dentry.d_fsdata to get the "dev_name" of the mountpoint (<server>:<path>) to make <server>:<rootpath + path to the dentry>.
>
> The problem I'm having is how we can generate the string "<rootpath of parent mount>/<path-to-current-dentry>" without adding [] escaping to dev_name, or adding another variable to dentry (not likely!).
>
To be clear: the problem is *not* with IPv6 addrs in FS_LOCATIONS attrs. They work if the parent mount is IPv4/dns. The problem is when the parent mount is IPv6 *no* referrals work (IPv4, IPv6 or DNS), because of the issue described above.
> -dros
>
>>
>>> Signed-off-by: Weston Andros Adamson <[email protected]>
>>> ---
>>> fs/nfs/nfs4namespace.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>>> index 9c8eca3..dd3dd30 100644
>>> --- a/fs/nfs/nfs4namespace.c
>>> +++ b/fs/nfs/nfs4namespace.c
>>> @@ -59,7 +59,7 @@ 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, ':');
>>> + char *colon = strrchr(path, ':');
>>> if (colon && colon < limit)
>>> path = colon + 1;
>>> }
>>> --
>>> 1.7.4.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Apr 20, 2012, at 3:21 PM, Weston Andros Adamson wrote:
> nfs4_path() was parsing the path component by splitting on the first colon.
> This is wrong when an IPv6 address is used to mount a server.
>
> For example, having mounted 'fc00::10:/export', nfs4_path() returned
> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
> in nfs4_validate_fspath().
>
> Parsing the path component by using the *last* colon works with
> IPv6 as well as IPv4 addrs.
These ad hoc fixes give me the willies.
During a referral, how is the server name and export path getting recombined? In fs_locations data, these are separate. In the forward mount path, IPv6 addresses are escaped via square brackets.
(Having not looked at this code in years), is there some way we can keep the server name and path separate here so we don't have to reparse?
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4namespace.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index 9c8eca3..dd3dd30 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -59,7 +59,7 @@ 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, ':');
> + char *colon = strrchr(path, ':');
> if (colon && colon < limit)
> path = colon + 1;
> }
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Apr 20, 2012, at 4:13 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 19:57 +0000, Adamson, Dros wrote:
>> On Apr 20, 2012, at 3:45 PM, Jim Rees wrote:
>>
>>> Weston Andros Adamson wrote:
>>>
>>> nfs4_path() was parsing the path component by splitting on the first colon.
>>> This is wrong when an IPv6 address is used to mount a server.
>>>
>>> For example, having mounted 'fc00::10:/export', nfs4_path() returned
>>> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
>>> in nfs4_validate_fspath().
>>>
>>> Parsing the path component by using the *last* colon works with
>>> IPv6 as well as IPv4 addrs.
>>>
>>> What if your mount is:
>>>
>>> server.edu:/export/:I-like-colons:
>>>
>>> It seems to me something has to give. Either we require v6 addresses be
>>> enclosed in [], export dirs start with "/", or exports have no ":".
>>
>>
>> Yeah, you're right.
>>
>> Although IPv6 addrs must be enclosed in [] to work with mount, it's always displayed in the kernel without the [].
>>
>> I suppose the correct fix is to always display IPv6 addresses enclosed in [], which might touch a *lot* of places.
>>
>> Thoughts? I'll wait for others to chime in before I go fixing this. I have a set of nfsd and mountd patches that deal with similar issues on the server side. I'll clean those up and submit them before getting back to this.
>
> We do know which part is the hostname, and which is the pathname. If you
> look at "try_location()", you'll see that the hostname is stored in
> location->servers, and is then copied into this empty buffer.
>
> If you want to test if that is an IPv6 address so that you can enclose
> it in [], then that should be fairly easy to do right there?
Right, we have separate hostname and pathname for each fs_location4, but I'm talking about the return value of nfs_path() which is used to determine the server-side path on an arbitrary dentry on the *current* mountpoint (not the referral server).
The path part of nfs_path() is then compared against the fs_path of the nfs4_fs_locations struct (this is all in nfs4_validate_fspath).
Having nfs_path return [] wrapped IPv6 addresses basically means changing the devname to that format (d_fsdata is used by nfs_path()). I'm concerned about changing the format of devname - its used all over the place.
So the options are:
1) change the format of devname
2) change how nfs4_validate_fspath gets the server-side path component of the current mount (don't use nfs_path())
I'll try option 1 and see what breaks.
-dros
On Apr 20, 2012, at 5:58 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 21:48 +0000, Adamson, Dros wrote:
>> On Apr 20, 2012, at 5:43 PM, Myklebust, Trond wrote:
>>
>>> On Fri, 2012-04-20 at 21:38 +0000, Adamson, Dros wrote:
>>>
>>>> Yes, something like that could work, but we still have the same issue - dentry->d_fsdata never has [] escaping.
>>>
>>> That's a bug too...
>>
>> Right! This is what I've been talking about the whole time - I guess I didn't do a good job explaining it :) The original bug (splitting return of nfs_path()) would be trivial to fix if d_fsdata had [] escaping.
>>
>> So I guess that answers my earlier question - we *do* want to change dev_name to include [] escaping.
>>
>> -dros
>
> Here is a patch from Jan Kara that does just this. I'll start by
> applying that...
Cool! dprintk debugging just led me there!
Why wasn't this patch applied earlier? Just curious.
I'll post a new patch to fix splitting nfs_path().
-dros
Oops, this should probably CC stable@ too.
-dros
On Apr 20, 2012, at 3:21 PM, Weston Andros Adamson wrote:
> nfs4_path() was parsing the path component by splitting on the first colon.
> This is wrong when an IPv6 address is used to mount a server.
>
> For example, having mounted 'fc00::10:/export', nfs4_path() returned
> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
> in nfs4_validate_fspath().
>
> Parsing the path component by using the *last* colon works with
> IPv6 as well as IPv4 addrs.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4namespace.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index 9c8eca3..dd3dd30 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -59,7 +59,7 @@ 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, ':');
> + char *colon = strrchr(path, ':');
> if (colon && colon < limit)
> path = colon + 1;
> }
> --
> 1.7.4.4
>
T24gRnJpLCAyMDEyLTA0LTIwIGF0IDIxOjM4ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
DQo+IFllcywgc29tZXRoaW5nIGxpa2UgdGhhdCBjb3VsZCB3b3JrLCBidXQgd2Ugc3RpbGwgaGF2
ZSB0aGUgc2FtZSBpc3N1ZSAtIGRlbnRyeS0+ZF9mc2RhdGEgbmV2ZXIgaGFzIFtdIGVzY2FwaW5n
Lg0KDQpUaGF0J3MgYSBidWcgdG9vLi4uIA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXgg
TkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5j
b20NCnd3dy5uZXRhcHAuY29tDQoNCg==
On Apr 20, 2012, at 5:35 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 20:51 +0000, Adamson, Dros wrote:
>> On Apr 20, 2012, at 4:13 PM, Myklebust, Trond wrote:
>>
>>> On Fri, 2012-04-20 at 19:57 +0000, Adamson, Dros wrote:
>>>> On Apr 20, 2012, at 3:45 PM, Jim Rees wrote:
>>>>
>>>>> Weston Andros Adamson wrote:
>>>>>
>>>>> nfs4_path() was parsing the path component by splitting on the first colon.
>>>>> This is wrong when an IPv6 address is used to mount a server.
>>>>>
>>>>> For example, having mounted 'fc00::10:/export', nfs4_path() returned
>>>>> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
>>>>> in nfs4_validate_fspath().
>>>>>
>>>>> Parsing the path component by using the *last* colon works with
>>>>> IPv6 as well as IPv4 addrs.
>>>>>
>>>>> What if your mount is:
>>>>>
>>>>> server.edu:/export/:I-like-colons:
>>>>>
>>>>> It seems to me something has to give. Either we require v6 addresses be
>>>>> enclosed in [], export dirs start with "/", or exports have no ":".
>>>>
>>>>
>>>> Yeah, you're right.
>>>>
>>>> Although IPv6 addrs must be enclosed in [] to work with mount, it's always displayed in the kernel without the [].
>>>>
>>>> I suppose the correct fix is to always display IPv6 addresses enclosed in [], which might touch a *lot* of places.
>>>>
>>>> Thoughts? I'll wait for others to chime in before I go fixing this. I have a set of nfsd and mountd patches that deal with similar issues on the server side. I'll clean those up and submit them before getting back to this.
>>>
>>> We do know which part is the hostname, and which is the pathname. If you
>>> look at "try_location()", you'll see that the hostname is stored in
>>> location->servers, and is then copied into this empty buffer.
>>>
>>> If you want to test if that is an IPv6 address so that you can enclose
>>> it in [], then that should be fairly easy to do right there?
>>
>> Right, we have separate hostname and pathname for each fs_location4, but I'm talking about the return value of nfs_path() which is used to determine the server-side path on an arbitrary dentry on the *current* mountpoint (not the referral server).
>> The path part of nfs_path() is then compared against the fs_path of the nfs4_fs_locations struct (this is all in nfs4_validate_fspath).
>>
>> Having nfs_path return [] wrapped IPv6 addresses basically means changing the devname to that format (d_fsdata is used by nfs_path()). I'm concerned about changing the format of devname - its used all over the place.
>>
>> So the options are:
>> 1) change the format of devname
>> 2) change how nfs4_validate_fspath gets the server-side path component of the current mount (don't use nfs_path())
>>
>> I'll try option 1 and see what breaks.
>
> That really should not be necessary.
>
> If you are using an IPv6 address, you are supposed to mount using the []
> format. If not, then expect things to break.
>
Yes, I am mounting with a [] escaped IPv6 addr.
> The real bug here is in nfs4_path: it tries to parse the ':' that are
> part of the IPv6 address even if we use the square bracket delimiters.
> Instead, we should be following the example of nfs_parse_devname.
The [] escaping is stripped well before nfs4_path.
>
> I suggest breaking out the first half of nfs_parse_devname into a
> function that returns the host name part of a mount path, and that can
> be called by nfs4_path too.
Yes, something like that could work, but we still have the same issue - dentry->d_fsdata never has [] escaping.
-dros
On Apr 20, 2012, at 5:43 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 21:38 +0000, Adamson, Dros wrote:
>
>> Yes, something like that could work, but we still have the same issue - dentry->d_fsdata never has [] escaping.
>
> That's a bug too...
Right! This is what I've been talking about the whole time - I guess I didn't do a good job explaining it :) The original bug (splitting return of nfs_path()) would be trivial to fix if d_fsdata had [] escaping.
So I guess that answers my earlier question - we *do* want to change dev_name to include [] escaping.
-dros
On Apr 20, 2012, at 4:13 PM, Myklebust, Trond wrote:
> On Fri, 2012-04-20 at 19:57 +0000, Adamson, Dros wrote:
>> On Apr 20, 2012, at 3:45 PM, Jim Rees wrote:
>>
>>> Weston Andros Adamson wrote:
>>>
>>> nfs4_path() was parsing the path component by splitting on the first colon.
>>> This is wrong when an IPv6 address is used to mount a server.
>>>
>>> For example, having mounted 'fc00::10:/export', nfs4_path() returned
>>> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
>>> in nfs4_validate_fspath().
>>>
>>> Parsing the path component by using the *last* colon works with
>>> IPv6 as well as IPv4 addrs.
>>>
>>> What if your mount is:
>>>
>>> server.edu:/export/:I-like-colons:
>>>
>>> It seems to me something has to give. Either we require v6 addresses be
>>> enclosed in [], export dirs start with "/", or exports have no ":".
>>
>>
>> Yeah, you're right.
>>
>> Although IPv6 addrs must be enclosed in [] to work with mount, it's always displayed in the kernel without the [].
>>
>> I suppose the correct fix is to always display IPv6 addresses enclosed in [], which might touch a *lot* of places.
>>
>> Thoughts? I'll wait for others to chime in before I go fixing this. I have a set of nfsd and mountd patches that deal with similar issues on the server side. I'll clean those up and submit them before getting back to this.
>
> We do know which part is the hostname, and which is the pathname. If you
> look at "try_location()", you'll see that the hostname is stored in
> location->servers, and is then copied into this empty buffer.
>
> If you want to test if that is an IPv6 address so that you can enclose
> it in [], then that should be fairly easy to do right there...
Yes, that's kind of what was I was thinking.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Apr 20, 2012, at 3:30 PM, Chuck Lever wrote:
>
> On Apr 20, 2012, at 3:21 PM, Weston Andros Adamson wrote:
>
>> nfs4_path() was parsing the path component by splitting on the first colon.
>> This is wrong when an IPv6 address is used to mount a server.
>>
>> For example, having mounted 'fc00::10:/export', nfs4_path() returned
>> ':10:/export'. This causes referrals (using IPv4 or IPv6 addresses) to fail
>> in nfs4_validate_fspath().
>>
>> Parsing the path component by using the *last* colon works with
>> IPv6 as well as IPv4 addrs.
>
> These ad hoc fixes give me the willies.
>
> During a referral, how is the server name and export path getting recombined? In fs_locations data, these are separate. In the forward mount path, IPv6 addresses are escaped via square brackets.
Again, the string that needs to be split into (server, path) is not the fs_locations side -- those are already separated. What's not separated is the result of nfs_path().
> (Having not looked at this code in years), is there some way we can keep the server name and path separate here so we don't have to reparse?
nfs_path() walks up the dentry tree until it hits the root dentry of the nfs mount, then it uses dentry.d_fsdata to get the "dev_name" of the mountpoint (<server>:<path>) to make <server>:<rootpath + path to the dentry>.
The problem I'm having is how we can generate the string "<rootpath of parent mount>/<path-to-current-dentry>" without adding [] escaping to dev_name, or adding another variable to dentry (not likely!).
-dros
>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> fs/nfs/nfs4namespace.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>> index 9c8eca3..dd3dd30 100644
>> --- a/fs/nfs/nfs4namespace.c
>> +++ b/fs/nfs/nfs4namespace.c
>> @@ -59,7 +59,7 @@ 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, ':');
>> + char *colon = strrchr(path, ':');
>> if (colon && colon < limit)
>> path = colon + 1;
>> }
>> --
>> 1.7.4.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html