netdev - Re: [PATCH net 3/3] ipv6: Fix dst_entry refcnt bugs in ip6

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]

Message-ID: <1441138460.8932.182.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Tue, 01 Sep 2015 13:14:20 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Martin KaFai Lau <kafai@...com>
Cc:	netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Kernel Team <kernel-team@...com>
Subject: Re: [PATCH net 3/3] ipv6: Fix dst_entry refcnt bugs in ip6_tunnel

On Tue, 2015-09-01 at 11:55 -0700, Martin KaFai Lau wrote:
> Problems in the current dst_entry cache in the ip6_tunnel:
> 
> 1. ip6_tnl_dst_set is racy.  There is no lock to protect it:
>    - One major problem is that the dst refcnt gets messed up. F.e.
>      the same dst_cache can be released multiple times and then
>      triggering the infamous dst refcnt < 0 warning message.
>    - Another issue is the inconsistency between dst_cache and
>      dst_cookie.
> 
>    It can be reproduced by adding and removing the ip6gre tunnel
>    while running a super_netperf TCP_CRR test.
> 
> 2. In ip6_tnl_xmit2() and ip6gre_xmit2(), the outgoing skb does
>    not hold a dst_entry's refcnt.

It should not be a problem. refcnt is taken when/if necessary (skb
queued on a qdisc for example)

We have other uses of skb_dst_set_noref()

Please describe the problem ?

> 
> This patch:
> 1. Create a percpu dst_entry cache in ip6_tnl
OK
> 2. Use a spinlock to protect the dst_cache operations

Well, a seqlock would be better : No need for an atomic operation in
fast path.

> 3. The outgoing skb always holds the dst_entry's refcnt
> 

Not clear why it is needed. Inexpensive thanks to per cpu dst, but
still...



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