netdev - Re: [DISCUSSION] rt6i

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 19 Jul 2013 08:01:47 +0800
From:	Fan Du <fan.du@...driver.com>
To:	<nicolas.dichtel@...nd.com>
CC:	netdev <netdev@...r.kernel.org>
Subject: Re: [DISCUSSION] rt6i_genid



On 2013年07月18日 23:12, Nicolas Dichtel wrote:
> Le 18/07/2013 11:28, Fan Du a écrit :
>>
>> Thanks for replying :)
>>
>> On 2013年07月18日 17:13, Nicolas Dichtel wrote:
>>> Le 18/07/2013 05:22, Fan Du a écrit :
>>>> Hello Nicolas
>>>>
>>>> Commit 6f3118b571b8a4c06c7985dc3172c3526cb86253: "ipv6: use net->rt_genid to
>>>> check dst validity"
>>>> makes ip6_dst_check to check rt6i_genid against with struct net->rt_genid,
>>>> As a matter of fact, struct net->rt_genid could only be modified by two places,
>>>> first is adding/delete IPv4 address, second is inserting new XFRM policy.
>>>>
>>>> Is there any other considerations that adding/deleting IPv4 address would
>>>> invalid all IPv6 dst
>>>> as well? because I'm working a patch which actually depends on the result of
>>>> this question.
>>> No, the goal was to cover the IPsec case, ie invalidate dst entries when an
>>> xfrm policy is inserted/deleted.
>>
>> Ok, then how about we only checking rt6i_genid against rt_genid *only*
>> when XFRM is enabled for IPv6, because when XFRM is not enabled for IPv6
>> ip6_dst_check for rt_genid is really not necessary.
>>
>> So what do you think of below modifications?
> Seems good. Just a small comment below.

Will send v2 for your reviewing when net-next is reopen.

Thanks

>>
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index 2a601e7..4ae35fd 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -119,7 +119,9 @@ struct rt6_info {
>> struct inet6_dev *rt6i_idev;
>> unsigned long _rt6i_peer;
>>
>> +#ifdef CONFIG_XFRM
>> u32 rt6i_genid;
>> +#endif
>>
>> /* more non-fragment space at head required */
>> unsigned short rt6i_nfheader_len;
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index bd5fd70..1b64406 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -277,7 +277,9 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
>>
>> memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
>> rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
>> +#ifdef CONFIG_XFRM
>> rt->rt6i_genid = rt_genid(net);
>> +#endif
>> INIT_LIST_HEAD(&rt->rt6i_siblings);
>> rt->rt6i_nsiblings = 0;
>> }
>> @@ -1041,12 +1043,15 @@ static struct dst_entry *ip6_dst_check(struct dst_entry
>> *dst, u32 cookie)
>>
>> rt = (struct rt6_info *) dst;
>>
>> +#ifdef CONFIG_XFRM
>> /* All IPV6 dsts are created with ->obsolete set to the value
>> * DST_OBSOLETE_FORCE_CHK which forces validation calls down
>> * into this function always.
>> + * Note: for IPv6, rt6i_genid is noly used when XFRM enabled.
> indentation is wrong, but I'm not sure this comment is really needed ... you've put the ifdef 4 lines above.
>
>> */
>> if (rt->rt6i_genid != rt_genid(dev_net(rt->dst.dev)))
>> return NULL;
>> +#endif
>>
>> if (rt->rt6i_node && (rt->rt6i_node->fn_sernum == cookie))
>> return dst;
>

-- 
浮沉随浪只记今朝笑

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