OK florian@
On Fri, Sep 02, 2016 at 11:21:33AM +0200, Vincent Gross wrote:
> in6_selectroute() checks whether the struct route it received contains
> a valid route whose AF is not AF_INET6, "in case the cache is shared".
> Well, is this cache shared or not ?
>
> There's only two ways to get to in6_selectroute()
> 1) in6_pcbselsrc() -> in6_selectif() -> in6_selectroute()
> It is trivial to check that only inet6 is handled here, and that any
> other AF is obviously an error.
>
> 2) ip6_output() -> in6_selectroute()
> a. If the struct route * arg of ip6_output() is NULL, then
> ip6_output() zeroes a struct route from the stack, it will never
> be valid thus there is no need to check its AF.
> b. If the struct route * arg is not NULL, it is passed to
> ip6_output().
>
> ip6_output() is called with a non-NULL struct route * in 5 places only:
>
> netinet/tcp_output.c:1124: error = ip6_output(m,
> tp->t_inpcb->inp_outputopts6,
> netinet/tcp_output.c-1125- &tp->t_inpcb->inp_route6,
> netinet/tcp_output.c-1126- 0, NULL, tp->t_inpcb);
>
> netinet/tcp_subr.c:399: ip6_output(m, tp ?
> tp->t_inpcb->inp_outputopts6 : NULL,
> netinet/tcp_subr.c-400- tp ? &tp->t_inpcb->inp_route6 : NULL,
> netinet/tcp_subr.c-401- 0, NULL,
> netinet/tcp_subr.c-402- tp ? tp->t_inpcb : NULL);
>
> netinet/tcp_input.c:4386: error = ip6_output(m, NULL /*XXX*/,
> &sc->sc_route6, 0,
> netinet/tcp_input.c-4387- NULL, NULL);
>
> netinet6/ip6_divert.c:167: error = ip6_output(m, NULL,
> &inp->inp_route6,
> netinet6/ip6_divert.c-168- IP_ALLOWBROADCAST | IP_RAWOUTPUT,
> NULL, NULL);
>
> netinet6/raw_ip6.c:457: error = ip6_output(m, optp, &in6p->inp_route6, flags,
> netinet6/raw_ip6.c-458- in6p->inp_moptions6, in6p);
>
> Each time, the struct route is only used in an inet6 context.
>
> I think it is safe to add this KASSERT() to in6_selectroute(). A few
> other things can be tightened, they will be addressed later.
>
> Ok ?
>
>
> Index: netinet6/in6_src.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 in6_src.c
> --- netinet6/in6_src.c 4 Aug 2016 20:46:24 -0000 1.79
> +++ netinet6/in6_src.c 2 Sep 2016 09:17:10 -0000
> @@ -302,13 +302,13 @@ in6_selectroute(struct sockaddr_in6 *dst
>
> /*
> * Use a cached route if it exists and is valid, else try to allocate
> - * a new one. Note that we should check the address family of the
> - * cached destination, in case of sharing the cache with IPv4.
> + * a new one.
> */
> if (ro) {
> + if (rtisvalid(ro->ro_rt))
> + KASSERT(sin6tosa(&ro->ro_dst)->sa_family == AF_INET6);
> if (!rtisvalid(ro->ro_rt) ||
> - sin6tosa(&ro->ro_dst)->sa_family != AF_INET6 ||
> - !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
> + !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
> rtfree(ro->ro_rt);
> ro->ro_rt = NULL;
> }
>
--
I'm not entirely sure you are real.
