Re: [PATCH] NFS: fix nfs_parse_ip_address() corner case

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Sep 03, 2008 at 04:35:57PM -0400, Chuck Lever wrote:
> Bruce observed that nfs_parse_ip_address() will successfully parse an
> IPv6 address that looks like this:
> 
>   "::1%"
> 
> A scope delimiter is present, but there is no scope ID following it.
> This is harmless, as it would simply set the scope ID to zero.  However,
> in some cases we would like to flag this as an improperly formed
> address.
> 
> We are now also careful to reject addresses where garbage follows the
> address (up to the length of the string), instead of ignoring the
> non-address characters; and where the scope ID is nonsense (not a valid
> device name, but also not numeric).  Before, both of these cases would
> result in a harmless zero scope ID.

Slightly irrelevant, but same comment as before--wouldn't it be easier
to follow the logic if instead of:

	p = kstrndup(...)
	if (p) {
		do stuff for successful case
		....
		return 1;
	}

	return 0;

it were:

	p = kstrndup(...)
	if (!p)
		return 0;
	do stuff for successful case
	...
	return 1;

e.g., the below, on top of yours. (Untested.)

Might also combine the two final exits in the usual way:

	out:
		kfree(p);
		return ret;

--b.

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index a9f5a12..fcee897 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -733,6 +733,8 @@ static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 {
 	size_t len = (string + str_len) - delim - 1;
 	char *p;
+	unsigned long scope_id = 0;
+	struct net_device *dev;
 
 	if (len == 0)
 		return 1;
@@ -744,29 +746,25 @@ static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 		return 0;
 
 	p = kstrndup(delim + 1, len, GFP_KERNEL);
-	if (p) {
-		unsigned long scope_id = 0;
-		struct net_device *dev;
-
-		dev = dev_get_by_name(&init_net, p);
-		if (dev != NULL) {
-			scope_id = dev->ifindex;
-			dev_put(dev);
-		} else {
-			if (strict_strtoul(p, 10, &scope_id) == 0) {
-				kfree(p);
-				return 0;
-			}
-		}
-
-		kfree(p);
+	if (!p)
+		return 0;
 
-		sin6->sin6_scope_id = scope_id;
-		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
-		return 1;
+	dev = dev_get_by_name(&init_net, p);
+	if (dev != NULL) {
+		scope_id = dev->ifindex;
+		dev_put(dev);
+	} else {
+		if (strict_strtoul(p, 10, &scope_id) == 0) {
+			kfree(p);
+			return 0;
+		}
 	}
 
-	return 0;
+	kfree(p);
+
+	sin6->sin6_scope_id = scope_id;
+	dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
+	return 1;
 }
 
 static void nfs_parse_ipv6_address(char *string, size_t str_len,