Re: [PATCH net] ipv6: don't let tb6_root node share routes with other node

[Ido Schimmel <idosch@idosch.org>]

On Fri, Jan 19, 2018 at 01:46:02PM -0800, Wei Wang wrote:
> On Fri, Jan 19, 2018 at 1:36 PM, Wei Wang <weiwan@google.com> wrote:
> >
> >
> > On Fri, Jan 19, 2018 at 1:13 PM, Ido Schimmel <idosch@idosch.org> wrote:
> >> Hi Wei, Martin,
> >>
> >> On Thu, Jan 18, 2018 at 03:31:29PM -0800, Wei Wang wrote:
> >>> On Thu, Jan 18, 2018 at 2:47 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> >>> > On Thu, Jan 18, 2018 at 10:40:03AM -0800, Wei Wang wrote:
> >>> >> From: Wei Wang <weiwan@google.com>
> >>> >>
> >>> >> After commit 4512c43eac7e, if we add a route to the subtree of
> >>> >> tb6_root
> >>> >> which does not have any route attached to it yet, the current code
> >>> >> will
> >>> >> let tb6_root and the node in the subtree share the same route.
> >>> >> This could cause problem cause tb6_root has RTN_INFO flag marked and
> >>> >> the
> >>> > You meant the RTN_RTINFO check in fib6_purge_rt()?
> >>> >
> >>> Yes. Exactly.
> >>
> >> The check in fib6_purge_rt() is indeed problematic as tb6_root will not
> >> release its reference on the deleted route. I can easily reproduce that
> >> on my system. However, I don't understand how come we end up with a
> >> use-after-free given tb6_root takes a reference on the route?
> >>
> 
> (Resending with plain txt format)
> 
> Hi Ido,
> 
> I think the use-after-free does not really happen on the route that is being
> falsely shared, but on the route which that route's rt6i_next is pointing to.
> Nothing could prevent rt->rt6i_next from being released.

Yep, I considered it, then confused myself and disqualified the
possibility, but you're right. FWIW, here's the reproducer:

ip -6 route add default from 2001:db8::/64 dev dummy0 metric 1
ip -6 route append default from 2001:db8::/64 dev dummy0 metric 2
ip -6 route del default from 2001:db8::/64 dev dummy0 metric 1
ip -6 route del default from 2001:db8::/64 dev dummy0 metric 2
ip -6 route show

Thanks!