Re: Bug in OpenBSD 6.3 /usr/bin/wc
> On Sep 27, 2018, at 10:26, Todd C. Miller wrote: > >> On Thu, 27 Sep 2018 16:19:46 +0100, Dave Hines wrote: >> >> Oops - my patch contained a bug (though it worked on my machine). >> Here is a corrected patch for /usr/src/usr.bin/wc/wc.c > > That fix looks correct to me. Ditto, works here. I will commit tonight if someone else doesn't chime in. (ok cheloha@ if someone else commits first)
Re: Bug in OpenBSD 6.3 /usr/bin/wc
On Thu, 27 Sep 2018 16:19:46 +0100, Dave Hines wrote: > Oops - my patch contained a bug (though it worked on my machine). > Here is a corrected patch for /usr/src/usr.bin/wc/wc.c That fix looks correct to me. - todd
Re: Bug in OpenBSD 6.3 /usr/bin/wc
Oops - my patch contained a bug (though it worked on my machine).
Here is a corrected patch for /usr/src/usr.bin/wc/wc.c
diff -r a60ac28f79ee wc.c
--- a/wc.c Wed Sep 26 17:49:53 2018 +0100
+++ b/wc.c Thu Sep 27 16:16:02 2018 +0100
@@ -208,7 +208,8 @@
gotsp = 1;
while ((len = getline(, , stream)) > 0) {
if (multibyte) {
- for (C = buf; *C != '\0'; C += len) {
+ const char *end = buf + len;
+ for (C = buf; C < end; C += len) {
++charct;
len = mbtowc(, C, MB_CUR_MAX);
if (len == -1) {
@@ -228,7 +229,7 @@
}
} else {
charct += len;
- for (C = buf; *C != '\0'; ++C) {
+ for (C = buf; len--; ++C) {
if (isspace((unsigned char)*C)) {
gotsp = 1;
if (*C == '\n')
Re: Page fault in rt_setgwroute triggered by npppd
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 - 1.377
> > +++ net/route.c 26 Sep 2018 19:56:47 -
> > @@ -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 preferif (!(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.
Re: Page fault in rt_setgwroute triggered by npppd
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 - 1.377
> +++ net/route.c 26 Sep 2018 19:56:47 -
> @@ -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.
> rtfree(nhrt);
> return (EHOSTUNREACH);
> }
>
OK claudio@
--
:wq Claudio
