On Tue, Aug 26, 2008 at 04:36:10PM -0400, Chuck Lever wrote:
> 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.
OK. A minor nit, but I'd also find this a little easier to read if it
attempted to stick to the pattern
if (something_bad)
fail;
if (something_else_bad)
fail;
...
succeed;
rather than
if (successful) {
if (still_successful) {
...
succeed;
}
}
fail;
--b.
>
>>> 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
