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

["Adamson, Dros" <Weston.Adamson@xxxxxxxxxx>]

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 <dros@xxxxxxxxxx>
>> ---
>> 
>> 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

Attachment: smime.p7s
Description: S/MIME cryptographic signature