On Wed, Oct 23, 2019 at 11:18:11AM +0200, Martin Pieuchot wrote:
> On 23/10/19(Wed) 08:43, Peter J. Philipp wrote:
> > Hi Holger & Tech,
>
> Hello Peter,
>
> > I have made my octeon router work again and I have a patch. But I'm not an
> > openbsd developer, nor is this patch official in any way.
>
> Could you explain in words what is the issue? Why does your diff
> prevent it?
>
> Thanks!
Hi,
The system has a trap 2, which I looked up as:
#define T_TLB_LD_MISS 2 /* TLB miss on load or ifetch */
what happens before this patch, I think, is that there is a varargs size_t
(which is size 8 in mips64), that gets promoted (I think) in varargs to int
(which would likely be size 4). Then what happens is the char * that is
va_arg'ed after that is somehow corrupted on length 1, bcopy would trap #2
on this.
If it is a varargs corruption in the builtin mips64 clang code then I don't
want to have to fix that, as I don't even know where to look. Instead I
laid out every buffer very carefully and using int's and char *'s so that
trap 2 is not called. I made a bitmask in the called function that usually
gets the trap that it will treat length of one as an int and the rest as a
char *.
I admit it was a lot of trial and error I must have gone through over 20+
compile/reboot cycles to get it right. I tested this on a unifi security
gateway and an ER-8. Both agreed to everything and at that spot is not trap 2.
I may have avoided the condition, but the condition may still be there on
other code. I didn't look. To easen debug of this I'll attach some config
files for npppd that I used in my lab that you can see for yourself on any
octeon pppoe.
I don't know if these were standard example npppd's or if they were left over
from an earlier project.
Thanks!
-peter
beta$ more npppd.conf
#######npppd.conf
tunnel PPPOE protocol pppoe {
listen on interface ix1
pppoe-desc-in-pktdump yes
pppoe-desc-out-pktdump yes
pppoe-session-in-pktdump yes
pppoe-session-out-pktdump yes
authentication-method pap
}
ipcp IPCP {
pool-address 10.0.0.2-10.0.0.254
dns-servers 192.168.177.40
}
interface tun1 address 10.0.0.1 ipcp IPCP
authentication LOCAL type local {
users-file "/etc/npppd/npppd-users"
}
bind tunnel from PPPOE authenticated by LOCAL to tun1
##############################################################
beta# more /etc/npppd/npppd-users
# $OpenBSD: npppd-users,v 1.1 2012/09/20 12:51:43 yasuoka Exp $
# sample npppd-users file. see npppd-users(5)
testcaller:\
:password=Verizon:\
:framed-ip-address=10.0.0.103
##############################################################
> > --- if_spppsubr.c.orig Tue Oct 22 18:49:47 2019
> > +++ if_spppsubr.c Wed Oct 23 08:03:35 2019
> > @@ -64,6 +64,7 @@
> > #endif
> >
> > #include <net/if_sppp.h>
> > +#include <lib/libkern/libkern.h>
> >
> > # define UNTIMEOUT(fun, arg, handle) \
> > timeout_del(&(handle))
> > @@ -233,7 +234,7 @@
> > int newstate);
> > void sppp_auth_send(const struct cp *cp,
> > struct sppp *sp, unsigned int type, u_int id,
> > - ...);
> > + u_int bitmask, ...);
> >
> > void sppp_up_event(const struct cp *cp, struct sppp *sp);
> > void sppp_down_event(const struct cp *cp, struct sppp *sp);
> > @@ -3277,7 +3278,8 @@
> > STDDCL;
> > struct lcp_header *h;
> > int len, x;
> > - u_char *value, *name, digest[AUTHCHALEN], dsize;
> > + u_char *value, *name, digest[AUTHCHALEN];
> > + int dsize;
> > int value_len, name_len;
> > MD5_CTX ctx;
> >
> > @@ -3335,8 +3337,8 @@
> > MD5Final(digest, &ctx);
> > dsize = sizeof digest;
> >
> > - sppp_auth_send(&chap, sp, CHAP_RESPONSE, h->ident,
> > - sizeof dsize, (const char *)&dsize,
> > + sppp_auth_send(&chap, sp, CHAP_RESPONSE, h->ident, 0x1,
> > + 1, dsize,
> > sizeof digest, digest,
> > strlen(sp->myauth.name),
> > sp->myauth.name,
> > @@ -3458,7 +3460,7 @@
> > if (value_len != sizeof digest ||
> > timingsafe_bcmp(digest, value, value_len) != 0) {
> > /* action scn, tld */
> > - sppp_auth_send(&chap, sp, CHAP_FAILURE, h->ident,
> > + sppp_auth_send(&chap, sp, CHAP_FAILURE, h->ident, 0,
> > sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
> > 0);
> > chap.tld(sp);
> > @@ -3467,7 +3469,7 @@
> > /* action sca, perhaps tlu */
> > if (sp->state[IDX_CHAP] == STATE_REQ_SENT ||
> > sp->state[IDX_CHAP] == STATE_OPENED)
> > - sppp_auth_send(&chap, sp, CHAP_SUCCESS, h->ident,
> > + sppp_auth_send(&chap, sp, CHAP_SUCCESS, h->ident, 0,
> > sizeof(SUCCMSG) - 1, (u_char *)SUCCMSG,
> > 0);
> > if (sp->state[IDX_CHAP] == STATE_REQ_SENT) {
> > @@ -3634,7 +3636,7 @@
> > void
> > sppp_chap_scr(struct sppp *sp)
> > {
> > - u_char clen;
> > + int clen;
> >
> > /* Compute random challenge. */
> > arc4random_buf(sp->chap_challenge, sizeof(sp->chap_challenge));
> > @@ -3642,8 +3644,8 @@
> >
> > sp->confid[IDX_CHAP] = ++sp->pp_seq;
> >
> > - sppp_auth_send(&chap, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP],
> > - sizeof clen, (const char *)&clen,
> > + sppp_auth_send(&chap, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP], 0x1,
> > + 1, clen,
> > (size_t)AUTHCHALEN, sp->chap_challenge,
> > strlen(sp->myauth.name),
> > sp->myauth.name,
> > @@ -3671,7 +3673,8 @@
> > STDDCL;
> > struct lcp_header *h;
> > int len, x;
> > - u_char *name, *passwd, mlen;
> > + u_char *name, *passwd;
> > + int mlen;
> > int name_len, passwd_len;
> >
> > len = m->m_pkthdr.len;
> > @@ -3724,7 +3727,8 @@
> > /* action scn, tld */
> > mlen = sizeof(FAILMSG) - 1;
> > sppp_auth_send(&pap, sp, PAP_NAK, h->ident,
> > - sizeof mlen, (const char *)&mlen,
> > + 0x1,
> > + 1, mlen,
> > sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
> > 0);
> > pap.tld(sp);
> > @@ -3735,7 +3739,8 @@
> > sp->state[IDX_PAP] == STATE_OPENED) {
> > mlen = sizeof(SUCCMSG) - 1;
> > sppp_auth_send(&pap, sp, PAP_ACK, h->ident,
> > - sizeof mlen, (const char *)&mlen,
> > + 0x1,
> > + 1, mlen,
> > sizeof(SUCCMSG) - 1, (u_char *)SUCCMSG,
> > 0);
> > }
> > @@ -3941,17 +3946,19 @@
> > void
> > sppp_pap_scr(struct sppp *sp)
> > {
> > - u_char idlen, pwdlen;
> > + int s_id, s_pwd;
> >
> > sp->confid[IDX_PAP] = ++sp->pp_seq;
> > - pwdlen = strlen(sp->myauth.secret);
> > - idlen = strlen(sp->myauth.name);
> >
> > + s_pwd = strlen(sp->myauth.secret);
> > + s_id = strlen(sp->myauth.name);
> > +
> > sppp_auth_send(&pap, sp, PAP_REQ, sp->confid[IDX_PAP],
> > - sizeof idlen, (const char *)&idlen,
> > - (size_t)idlen, sp->myauth.name,
> > - sizeof pwdlen, (const char *)&pwdlen,
> > - (size_t)pwdlen, sp->myauth.secret,
> > + 0x5,
> > + 1, s_id,
> > + s_id, sp->myauth.name,
> > + 1, s_pwd,
> > + s_pwd, sp->myauth.secret,
> > 0);
> > }
> > /*
> > @@ -3968,15 +3975,16 @@
> >
> > void
> > sppp_auth_send(const struct cp *cp, struct sppp *sp,
> > - unsigned int type, u_int id, ...)
> > + unsigned int type, u_int id, u_int bitmask, ...)
> > {
> > STDDCL;
> > struct lcp_header *lh;
> > struct mbuf *m;
> > - u_char *p;
> > - int len, s;
> > - unsigned int mlen;
> > - const char *msg;
> > + char *p;
> > + int s, myval;
> > + u_int seq = 1;
> > + unsigned int mlen, len;
> > + char *msg;
> > va_list ap;
> >
> > MGETHDR (m, M_DONTWAIT, MT_DATA);
> > @@ -3992,11 +4000,15 @@
> > lh->ident = id;
> > p = (u_char*) (lh+1);
> >
> > - va_start(ap, id);
> > + va_start(ap, bitmask);
> > len = 0;
> >
> > - while ((mlen = (unsigned int)va_arg(ap, size_t)) != 0) {
> > - msg = va_arg(ap, const char *);
> > + while ((mlen = (unsigned int)va_arg(ap, int)) != 0) {
> > + if (bitmask & seq)
> > + myval = va_arg(ap, int);
> > + else
> > + msg = va_arg(ap, char *);
> > +
> > len += mlen;
> > if (len > MHLEN - PKTHDRLEN - LCP_HEADER_LEN) {
> > va_end(ap);
> > @@ -4004,10 +4016,16 @@
> > return;
> > }
> >
> > - bcopy(msg, p, mlen);
> > + if (bitmask & seq)
> > + *p = myval;
> > + else
> > + memcpy(p, msg, mlen);
> > +
> > p += mlen;
> > + seq <<= 1;
> > }
> > va_end(ap);
> > +
> >
> > m->m_pkthdr.len = m->m_len = PKTHDRLEN + LCP_HEADER_LEN + len;
> > lh->len = htons (LCP_HEADER_LEN + len);
> >
> >
