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

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

 



On Aug 26, 2008, at Aug 26, 2008, 4:28 PM, J. Bruce Fields wrote:
On Tue, Aug 26, 2008 at 04:24:12PM -0400, Chuck Lever wrote:
On Aug 26, 2008, at Aug 26, 2008, 2:39 PM, J. Bruce Fields wrote:
On Fri, Aug 22, 2008 at 02:24:22PM -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.

Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---

fs/nfs/super.c |   24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 5b2aa04..f73e068 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -727,19 +727,21 @@ static void nfs_parse_ipv4_address(char
*string, size_t str_len,
#define IPV6_SCOPE_DELIMITER	'%'

#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-static void nfs_parse_ipv6_scope_id(const char *string, const
size_t str_len,
-				    const char *delim,
-				    struct sockaddr_in6 *sin6)
+static int nfs_parse_ipv6_scope_id(const char *string, const size_t
str_len,
+				   const char *delim,
+				   struct sockaddr_in6 *sin6)
{
	char *p;
	size_t len;

	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
-		return ;
+		return 0;
	if (*delim != IPV6_SCOPE_DELIMITER)
-		return;
-
+		return 0;

What happens in the case where there's no scope delimiter?  In that
case
can't *delim correctly point to something else here?

When we get to nfs_parse_ipv6_scope_id(), *delim points to the first
character following the 128-bit IPv6 address string. We should fail if
*delim doesn't point to either '%' or '\0'.  So we need another check
here -- succeed immediately if *delim points to '\0'.

The string isn't necessarily null-delimited.

OK, we just need to take str_len into account.

Then, I think we should check if the address is link-local _after_ we
know we have a valid scope delimiter.

Arguably kstrndup() and dev_get_by_name() failures should also result
in
parser failures. It seems safer to me to reject bad addresses than to
try to use them anyway (possibly resulting in mounting a different
server from what was intended).

Well, if kstrndup() fails, that doesn't necessarily mean we have a bad
address; simply that there wasn't memory to parse it.  But it's
reasonable to return 0 in that case.

If dev_get_by_name() fails, then the next step is to check if we were
passed a numeric value instead of a device name. If the strtoul() call
fails to find a real numeric there, then yes, address parsing should
fail.

What does %numeric-value mean?

'%eth0' means find and use the scope ID of the eth0 device. '%2' means use the scope ID 2. If eth0 has a interface index of 2, then both of these are equivalent. The numeric index is the scope ID.

--
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