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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On May 16, 2008, at 3:53 PM, J. Bruce Fields wrote:
On Sat, May 10, 2008 at 09:07:23PM -0400, david m. richter wrote:
On Sat, May 10, 2008 at 7:50 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
On May 10, 2008, at 10:32 AM, Trond Myklebust wrote:

On Fri, 2008-05-09 at 19:29 -0700, Chuck Lever wrote:

Should you use in4_pton() instead?

Can we rather convert this to use nfs_parse_server_address? We don't
need 10 different ways to parse text addresses...

I'm OK with that, as long as there isn't a technical problem with using
in4_pton().

nfs_parse_server_address() uses in4_pton(), it just also groks ipv6.

This is all a bit orthogonal to the actual bug, as all those functions
want null-terminated strings too.

We could apply the below (compile-tested only) and then add ipv6 support
and converting to nfs_parse_server_address() in a subsequent patch.

I'm looking at this code for other reasons, but it would be very easy to teach nfs_parse_server_address() to take a string length and not assume the passed-in address string is null-terminated. Both in4_pton and in6_pton will take a string length.

From 530b441f2239d8bcedf9456c3c570d9c179cb406 Mon Sep 17 00:00:00 2001
From: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
Date: Fri, 9 May 2008 15:10:56 -0700
Subject: [PATCH] nfs: Fix misparsing of nfsv4 fs_locations attribute

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@xxxxxxxxxxxxxx>
---
fs/nfs/nfs4namespace.c |   13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 5f9ba41..40a0209 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -93,14 +93,21 @@ static int nfs4_validate_fspath(const struct vfsmount *mnt_parent,
	return 0;
}

+#define MAX_IPADDR_STRLEN 40
/*
 * Check if the string represents a "valid" IPv4 address
 */
-static inline int valid_ipaddr4(const char *buf)
+static inline int valid_ipaddr4(const struct nfs4_string *buf)
{
	int rc, count, in[4];
+	char str[MAX_IPADDR_STRLEN];

-	rc = sscanf(buf, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
+	if (buf->len >= MAX_IPADDR_STRLEN)
+		return -EINVAL;
+	memcpy(str, buf->data, buf->len);
+	str[buf->len] = '\0';
+
+	rc = sscanf(str, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
	if (rc != 4)
		return -EINVAL;
	for (count = 0; count < 4; count++) {
@@ -178,7 +185,7 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
			};

			if (location->servers[s].len <= 0 ||
-			    valid_ipaddr4(location->servers[s].data) < 0) {
+			    valid_ipaddr4(&location->servers[s]) < 0) {
				s++;
				continue;
			}
--
1.5.5.rc1


--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux