On 27/09/18(Thu) 09:29, Claudio Jeker wrote:
> On Wed, Sep 26, 2018 at 04:57:02PM -0300, Martin Pieuchot wrote:
> > On 26/09/18(Wed) 15:56, Élie Bouttier wrote:
> > > On 9/26/18 12:42 PM, Martin Pieuchot wrote:
> > > > Hello,
> > > >
> > > > On 26/09/18(Wed) 11:59, e...@bouttier.eu wrote:
> > > >>> Synopsis: Page fault in rt_setgwroute triggered by npppd under
> > > >>> specific circumstances
> > > >>> Category: kernel (networking)
> > > >>> Environment:
> > > >> System : OpenBSD 6.3
> > > >> Details : OpenBSD 6.3 (GENERIC) #100: Sat Mar 24 14:17:45
> > > >> MDT 2018
> > > >>
> > > >> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC
> > > >>
> > > >> Architecture: OpenBSD.amd64
> > > >> Machine : amd64
> > > >>> Description:
> > > >> On client connection, npppd cause a panic. It seems the panic
> > > >> occurs when the
> > > >> selected address for the tun interface is already used on another
> > > >> interface. For instance,
> > > >> if I use 10.0.0.254/24 on a vlan interface, using 10.0.0.254 on tun0
> > > >> trigger the panic.
> > > >> Using 10.0.0.253 for tun0 do not trigger the panic.
> > > >>
> > > >>> How-To-Repeat:
> > > >> - Configure an interface with 10.0.0.254/24 (in my case a vlan
> > > >> interface)
> > > >> - Configure npppd like this:
> > > >>
> > > >> ipcp interco {
> > > >> pool-address 10.0.0.64-10.0.0.127 for static
> > > >> dns-servers 10.0.0.254
> > > >> allow-user-selected-address no
> > > >> }
> > > >> interface tun0 address 10.0.0.254 ipcp interco
> > > >>
> > > >> - Connect the L2TP client (in my case through IPsec)
> > > >
> > > > Could you include the output of "# route -n show -inet" before
> > > > connecting the L2TP client in your next report?
> > >
> > > Please see at the end of the mail. The concurrent network is
> > > 10.0.252.0/24, shared between vlan252 and tun0.
> > >
> > > >
> > > > Does the diff below help? Even if it does, please insert the route(8)
> > > > output.
> > >
> > > It help, no panic, and the following line in the logs:
> > >
> > > npppd[44212]: ppp id=2 layer=ipcp logtype=Opened ip=10.0.252.65
> > > assignType=static
> > > npppd[44212]: write() failed in in_route0 on RTM_ADD : No route to host
> > >
> > > And no route for the client so the connection is not working.
> > >
> > > I tried manually (without -ifp tun0, carp252 is prefered):
> > >
> > > # route add 10.0.252.65 10.0.252.254 -ifp tun0
> > > add host 10.0.252.65: gateway 10.0.252.254: No route to host
> > >
> > > Without the patch, the same route command trigger the same page fault.
> >
> > Thanks the problem is that the current code assume that every RTF_LLINFO
> > route is a cloned one, which is not always true. Diff below uses the
> > correct check for this assertion, and I'd like to commit it before 6.4.
> >
> > Any ok?
> >
> > Now even if this will fix the reported panic, the logic could be
> > improved. However I'm not sure if the complexity is worth it.
> >
> > Why are you using the same address on tun and carp?
> >
> > Index: net/route.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/route.c,v
> > retrieving revision 1.377
> > diff -u -p -r1.377 route.c
> > --- net/route.c 11 Jul 2018 19:52:19 -0000 1.377
> > +++ net/route.c 26 Sep 2018 19:56:47 -0000
> > @@ -396,10 +396,11 @@ rt_setgwroute(struct rtentry *rt, u_int
> > struct sockaddr_in6 sa_mask;
> >
> > /*
> > - * If we found a non-L2 entry on a different interface
> > - * there's nothing we can do.
> > + * If we found a non cloned L2 entry on a different
> > + * interface, don't try to find a corresponding gateway.
> > */
> > - if (!ISSET(nhrt->rt_flags, RTF_LLINFO)) {
> > + if (!ISSET(nhrt->rt_flags, RTF_LLINFO) ||
> > + !ISSET(nhrt->rt_flags, RTF_CLONED)) {
>
> I would prefer if (!(ISSET(nhrt->rt_flags, RTF_LLINFO) &&
> ISSET(nhrt->rt_flags, RTF_CLONED))) {
> For me this is easier to understand considering the comment above.
The comment is wrong/misleading. I'll remove it. The one just below
explains it all.
