From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [PATCH] NFS: fix nfs_parse_ip_address() corner case Date: Tue, 26 Aug 2008 16:28:08 -0400 Message-ID: <20080826202808.GO4380@fieldses.org> References: <20080822182419.19572.34705.stgit@manray.1015granger.net> <20080826183942.GE4380@fieldses.org> <4C1CDB1D-8EE6-4303-B8CE-814F059E5BAD@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-nfs@vger.kernel.org To: Chuck Lever Return-path: Received: from mail.fieldses.org ([66.93.2.214]:45437 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751036AbYHZU2J (ORCPT ); Tue, 26 Aug 2008 16:28:09 -0400 In-Reply-To: <4C1CDB1D-8EE6-4303-B8CE-814F059E5BAD@oracle.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: 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 >>> --- >>> >>> 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. > 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? --b. > > If you agree I will repost with corrections.