Re: [PATCH 6/6] nfs: Fix misparsing of nfsv4 fs_locations attribute

[Trond Myklebust <Trond.Myklebust@netapp.com>]

On Mon, 2008-06-09 at 16:51 -0400, J. Bruce Fields wrote:
> The code incorrectly assumes here that the server name (or ip address)
> is null-terminated.  This can cause referrals to fail in some cases.
> 
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  fs/nfs/nfs4namespace.c |   34 ++++++++++------------------------
>  1 files changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index b112857..2f3eabe 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -93,23 +93,6 @@ static int nfs4_validate_fspath(const struct vfsmount *mnt_parent,
>  	return 0;
>  }
>  
> -/*
> - * Check if the string represents a "valid" IPv4 address
> - */
> -static inline int valid_ipaddr4(const char *buf)
> -{
> -	int rc, count, in[4];
> -
> -	rc = sscanf(buf, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
> -	if (rc != 4)
> -		return -EINVAL;
> -	for (count = 0; count < 4; count++) {
> -		if (in[count] > 255)
> -			return -EINVAL;
> -	}
> -	return 0;
> -}
> -
>  /**
>   * nfs_follow_referral - set up mountpoint when hitting a referral on moved error
>   * @mnt_parent - mountpoint of parent directory
> @@ -172,19 +155,20 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
>  
>  		s = 0;
>  		while (s < location->nservers) {
> +			const struct nfs4_string *buf = &location->servers[s];
>  			struct sockaddr_in addr = {
>  				.sin_family	= AF_INET,
>  				.sin_port	= htons(NFS_PORT),
>  			};
> +			u8 *ip = (u8 *)addr.sin_addr.s_addr;
>  
> -			if (location->servers[s].len <= 0 ||
> -			    valid_ipaddr4(location->servers[s].data) < 0) {
> -				s++;
> -				continue;
> -			}
> +			if (buf->len <= 0 || buf->len >= PAGE_SIZE)
> +				goto next;
> +			if (!in4_pton(buf->data, buf->len, ip, '\0', NULL))
> +				goto next;

What if it is an IPv6 address? As I've said before, could we please just
adapt nfs_parse_server_address to deal with all these cases?

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com