Fwd: [patch] ifconfig.c
Unfortunantly it wasn't that easy. This version doesn't segfault with
bad input :) Also just noticed there is a bunch of stuff going on with
ifconfig in current. So I guess this can just be a conversation starter,
and perhaps whomever is doing the work can possibly put something like
this in.
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.362
diff -u -p -u -r1.362 ifconfig.c
--- ifconfig.c 27 Feb 2018 22:32:26 - 1.362
+++ ifconfig.c 10 Aug 2018 01:31:10 -
@@ -793,9 +793,13 @@ main(int argc, char *argv[])
} else
noarg = 0;
- if (noarg == 0)
- (*p->c_func)(NULL, 0);
- else
+ if (noarg == 0) {
+ if (strcmp(p->c_name, "scan") == 0) {
+ (*p->c_func)(NULL, 0);
+ goto done;
+ } else
+ (*p->c_func)(NULL, 0);
+ } else
goto nextarg;
} else if (p->c_parameter == NEXTARG) {
nextarg:
@@ -863,6 +867,7 @@ nextarg:
if (ioctl(s, rafp->af_aifaddr, rafp->af_addreq) < 0)
err(1, "SIOCAIFADDR");
}
+done:
return (0);
}
@@ -1994,9 +1999,7 @@ setifchan(const char *val, int d)
void
setifscan(const char *val, int d)
{
- if (shownet80211chans || shownet80211nodes)
- usage();
- shownet80211nodes = 1;
+ return(ieee80211_listnodes());
}
#ifndef SMALL
@@ -2201,7 +2204,6 @@ ieee80211_status(void)
putchar(' ');
printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
}
-
putchar('\n');
if (shownet80211chans)
ieee80211_listchans();
Forwarded Message
Subject:[patch] ifconfig.c
Date: Thu, 9 Aug 2018 18:20:47 -0500
From: Edgar Pettijohn III
To: tech@openbsd.org
I hate to assume, but I'm going to assume that if one wants to scan for
ap's for their wifi interface to connect to they don't care about
anything else. I also removed what to me is one too many tabs.
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.362
diff -u -p -u -r1.362 ifconfig.c
--- ifconfig.c 27 Feb 2018 22:32:26 - 1.362
+++ ifconfig.c 9 Aug 2018 23:16:59 -
@@ -772,6 +772,11 @@ main(int argc, char *argv[])
return bridge_rule(argc, argv, -1);
}
#endif
+ if (strcmp(p->c_name, "scan") == 0) {
+ ieee80211_listnodes();
+ return 0;
+ }
+
if (p->c_name == 0 && setaddr)
for (i = setaddr; i > 0; i--) {
p++;
@@ -2288,7 +2293,7 @@ ieee80211_listnodes(void)
qsort(nr, na.na_nodes, sizeof(*nr), rssicmp);
for (i = 0; i < na.na_nodes; i++) {
- printf("\t\t");
+ printf("\t");
ieee80211_printnode(&nr[i]);
putchar('\n');
}
[patch] ifconfig.c
I hate to assume, but I'm going to assume that if one wants to scan for
ap's for their wifi interface to connect to they don't care about
anything else. I also removed what to me is one too many tabs.
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.362
diff -u -p -u -r1.362 ifconfig.c
--- ifconfig.c 27 Feb 2018 22:32:26 - 1.362
+++ ifconfig.c 9 Aug 2018 23:16:59 -
@@ -772,6 +772,11 @@ main(int argc, char *argv[])
return bridge_rule(argc, argv, -1);
}
#endif
+ if (strcmp(p->c_name, "scan") == 0) {
+ ieee80211_listnodes();
+ return 0;
+ }
+
if (p->c_name == 0 && setaddr)
for (i = setaddr; i > 0; i--) {
p++;
@@ -2288,7 +2293,7 @@ ieee80211_listnodes(void)
qsort(nr, na.na_nodes, sizeof(*nr), rssicmp);
for (i = 0; i < na.na_nodes; i++) {
- printf("\t\t");
+ printf("\t");
ieee80211_printnode(&nr[i]);
putchar('\n');
}
openssl s_time: refactor benchmark loops
Hi,
s_time -- everyone's perennial pick for favorite openssl app -- has a
lot of copy & paste. Here's a first shot at refactoring that away.
We can merge the two benchmark loops, the timing stuff, and the report
printing, as all of it is nearly identical.
I modified as little as possible when moving and merging things so it's
obvious to the reader that the behavior is completely unchanged. There's
cleanup and minor bugfixing I'd like to do, but that belongs in a separate
diff to keep this review easy.
Thoughts & feedback? ok?
--
Scott Cheloha
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.24
diff -u -p -r1.24 s_time.c
--- usr.bin/openssl/s_time.c13 Jul 2018 18:36:56 - 1.24
+++ usr.bin/openssl/s_time.c9 Aug 2018 22:45:20 -
@@ -91,6 +91,7 @@ extern int verify_depth;
static void s_time_usage(void);
static SSL *doConnection(SSL * scon);
+static int benchmark(int);
static SSL_CTX *tm_ctx = NULL;
static const SSL_METHOD *s_time_meth = NULL;
@@ -244,13 +245,7 @@ tm_Time_F(int op)
int
s_time_main(int argc, char **argv)
{
- double totalTime = 0.0;
- int nConn = 0;
- SSL *scon = NULL;
- time_t finishtime;
int ret = 1;
- char buf[1024 * 8];
- int ver;
if (single_execution) {
if (pledge("stdio rpath inet dns", NULL) == -1) {
@@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
- bytes_read = 0;
- finishtime = time(NULL) + s_time_config.maxtime;
- tm_Time_F(START);
- for (;;) {
- if (finishtime < time(NULL))
- break;
- if ((scon = doConnection(NULL)) == NULL)
- goto end;
-
- if (s_time_config.www_path != NULL) {
- int i, retval = snprintf(buf, sizeof buf,
- "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
- bytes_read += i;
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
- SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn += 1;
- if (SSL_session_reused(scon))
- ver = 'r';
- else {
- ver = SSL_version(scon);
- if (ver == TLS1_VERSION)
- ver = 't';
- else if (ver == SSL3_VERSION)
- ver = '3';
- else if (ver == SSL2_VERSION)
- ver = '2';
- else
- ver = '*';
- }
- fputc(ver, stdout);
- fflush(stdout);
-
- SSL_free(scon);
- scon = NULL;
- }
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
-
- printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes
read %ld\n",
- nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per
connection\n",
- nConn,
- (long long)(time(NULL) - finishtime + s_time_config.maxtime),
- bytes_read / nConn);
+ if (benchmark(0))
+ goto end;
/*
* Now loop and time connections using the same session id over and
@@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
goto end;
printf("\n\nNow timing with session id reuse.\n");
- /* Get an SSL object so we can reuse the session id */
- if ((scon = doConnection(NULL)) == NULL) {
- fprintf(stderr, "Unable to get connection\n");
+ if (benchmark(1))
goto end;
- }
- if (s_time_config.www_path != NULL) {
- int retval = snprintf(buf, sizeof buf,
- "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while (SSL_read(scon, buf, sizeof(buf)) > 0);
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
- SSL_RECEIVED_SHUTDOWN);
-
lo(4) automatic ::1 setting and multiple loopbacks
While looking into something unrelated I found a strange extra ::1 address on lo1 (I usually hang my loopback addresses for IBGP off lo1). lo1: flags=8049 mtu 32768 index 12 priority 0 llprio 3 groups: lo inet xxx.xx.xxx.1 netmask 0x inet6 ::1 prefixlen 128 inet6 fe80::1%lo1 prefixlen 64 scopeid 0xc inet6 ::x:xxx::1 prefixlen 128 I haven't run into problems as a result of this (fortunately ospf6d picked the correct address to redistribute) but it doesn't seem right. $ ifconfig lo | grep -e ^lo -e 'inet6 ::1' lo0: flags=8049 mtu 32768 inet6 ::1 prefixlen 128 lo1: flags=8049 mtu 32768 inet6 ::1 prefixlen 128 It seems we did this a while ago to fix breakage after NOINET6 but reading commit log, it definitely doesn't seem expected that ::1 would appear on additional loopback interfaces (see lines marked with >) - : $ acvs log -N -r1.315 if.c : : RCS file: /cvs/src/sys/net/if.c,v : Working file: if.c : head: 1.559 : branch: : locks: strict : access list: : keyword substitution: kv : total revisions: 580; selected revisions: 1 : description: : : revision 1.315 : date: 2015/01/27 10:31:19; author: mpi; state: Exp; lines: +12 -21; commitid: 5QOPq50YTGxsLtYH; : Ensure that link-local addresses are correctly configured on loopback : interfaces. : : When the kernel automagically configures IPv6 addresses on loopback : interfaces, start by assigning a link-local address and then try to : assign "::1". : > Only the first configured loopback interface per rdomain can have the > "::1" address. But even if other loopback interfaces failed to get : this address, because it is already taken, give them a chance to have : a link-local address. : : While here change in6_ifattach() to return an error value and remove : duplicated code. : : Fix a regression introduced by the NOINET6 flag removal. : : ok henning@, stsp@, florian@, benno@ : = Any thoughts on how bad this is and whether it needs fixing or can be ignored? (I suppose I could add "inet6 ::1 delete" to hostname.lo1..)
Re: bgpd: refine source-as matching
On Thu, Aug 09, 2018 at 10:37:50PM +0200, Sebastian Benoit wrote:
> Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.08.09 21:59:16 +0200:
> > On Thu, Aug 09, 2018 at 03:10:11PM +0200, Claudio Jeker wrote:
> > > Per rfc6472 AS_SET should no longer be used but some AS still do.
> > > Until now source-as would take the rightmost AS number of an AS_PATH no
> > > matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
> > > because AS_SET are used in aggregation source-as should match against the
> > > aggregator AS (which it the rightmost as of the previous AS_PATH segment).
> > > At the same time move the peer-as check out of the loop, there is no
> > > reason to have that inside the for loop.
> > >
> > > This diff implements this and also makes aspath_extract() and the lenght
> > > checks a bit more paranoid.
> >
> > OK, updated diff here that adds three things.
> > a) it passes the filter_as struct to as_compare
> > b) it ensures that if for whatever crazy reason AS 0 is passed to the
> >filter it will not match (no matter what). This is more paranoia.
> >I decided to do that since util.c is shared between bgpd and bgpctl
> >and so the easy way of calling fatalx() is not an option here.
> > c) fix the souce-as 42 >< 4242 case. Currently the >< operator is not
> >correctly implemented it is more a <> as in inside but excluding the
> >edges. I think 42 >< 4242 should match for AS 1 - 41 and 4243 and
> >higher.
> >
> > Is this still OK?
>
> hm. yes. ok.
>
> although this will cause bizarre effects:
>
> deny from any AS 0
>
> wont match even for as == 0.
>
Yes, this rule should never match since there should be never an AS 0 in
any path that makes it to the filter. This is why it is kind of valid to
ingore that rule completly and move to the next one.
> i have to admit that we check for AS 0 in the path somewhere else.
> still...
Yes, aspath with a 0 AS number in them are soft rejected by aspath_verify.
I currently don't want to disallow AS 0 in the parse.y because ROA may
need it in one way or the other. But even there it is a make sure it never
matches thing. Once I have those bits together it may be possible to
restrict this further.
In the end having aspath_extract return 0 is in all current usecases of
bgpd impossible. There is always some additional code that ensures that
there is no overflow but we tripped into that hole at least once and I
would prefer to not do it again. In general I would throw in a fatalx()
there but as mentioned that won't fly in bgpctl which uses this code as
well.
I will commit this without the aspath_extract() chunk and the as == 0
check. Will put that into a new diff for later.
> > --
> > :wq Claudio
> >
> >
> > Index: bgpd.h
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> > retrieving revision 1.329
> > diff -u -p -r1.329 bgpd.h
> > --- bgpd.h 8 Aug 2018 14:29:05 - 1.329
> > +++ bgpd.h 9 Aug 2018 19:43:39 -
> > @@ -1156,8 +1156,6 @@ intaspath_snprint(char *, size_t, voi
> > int aspath_asprint(char **, void *, u_int16_t);
> > size_t aspath_strlen(void *, u_int16_t);
> > int aspath_match(void *, u_int16_t, struct filter_as *,
> > u_int32_t);
> > -int as_compare(u_int8_t, u_int32_t, u_int32_t, u_int32_t,
> > - u_int32_t);
> > u_int32_t aspath_extract(const void *, int);
> > int aspath_verify(void *, u_int16_t, int);
> > #define AS_ERR_LEN -1
> > Index: util.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> > retrieving revision 1.28
> > diff -u -p -r1.28 util.c
> > --- util.c 22 Jul 2018 16:52:27 - 1.28
> > +++ util.c 9 Aug 2018 19:38:34 -
> > @@ -312,6 +312,22 @@ aspath_strlen(void *data, u_int16_t len)
> > return (total_size);
> > }
> >
> > +static int
> > +as_compare(struct filter_as *f, u_int32_t as, u_int32_t match)
> > +{
> > + if (as == 0)/* AS 0 is invalid and therefor never matches */
> > + return (0);
> > + if ((f->op == OP_NONE || f->op == OP_EQ) && as == match)
> > + return (1);
> > + else if (f->op == OP_NE && as != match)
> > + return (1);
> > + else if (f->op == OP_RANGE && as >= f->as_min && as <= f->as_max)
> > + return (1);
> > + else if (f->op == OP_XRANGE && (as < f->as_min || as > f->as_max))
> > + return (1);
> > + return (0);
> > +}
> > +
> > /* we need to be able to search more than one as */
> > int
> > aspath_match(void *data, u_int16_t len, struct filter_as *f, u_int32_t
> > match)
> > @@ -320,7 +336,7 @@ aspath_match(void *data, u_int16_t len,
> > int final;
> > u_int16_tseg_size;
> > u_int8_t i, seg_len;
> > - u_int32_tas;
> > + u_int32_tas = 0, preas;
> >
Re: bgpd: refine source-as matching
Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.08.09 21:59:16 +0200:
> On Thu, Aug 09, 2018 at 03:10:11PM +0200, Claudio Jeker wrote:
> > Per rfc6472 AS_SET should no longer be used but some AS still do.
> > Until now source-as would take the rightmost AS number of an AS_PATH no
> > matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
> > because AS_SET are used in aggregation source-as should match against the
> > aggregator AS (which it the rightmost as of the previous AS_PATH segment).
> > At the same time move the peer-as check out of the loop, there is no
> > reason to have that inside the for loop.
> >
> > This diff implements this and also makes aspath_extract() and the lenght
> > checks a bit more paranoid.
>
> OK, updated diff here that adds three things.
> a) it passes the filter_as struct to as_compare
> b) it ensures that if for whatever crazy reason AS 0 is passed to the
>filter it will not match (no matter what). This is more paranoia.
>I decided to do that since util.c is shared between bgpd and bgpctl
>and so the easy way of calling fatalx() is not an option here.
> c) fix the souce-as 42 >< 4242 case. Currently the >< operator is not
>correctly implemented it is more a <> as in inside but excluding the
>edges. I think 42 >< 4242 should match for AS 1 - 41 and 4243 and
>higher.
>
> Is this still OK?
hm. yes. ok.
although this will cause bizarre effects:
deny from any AS 0
wont match even for as == 0.
i have to admit that we check for AS 0 in the path somewhere else.
still...
> --
> :wq Claudio
>
>
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.329
> diff -u -p -r1.329 bgpd.h
> --- bgpd.h8 Aug 2018 14:29:05 - 1.329
> +++ bgpd.h9 Aug 2018 19:43:39 -
> @@ -1156,8 +1156,6 @@ int aspath_snprint(char *, size_t, voi
> int aspath_asprint(char **, void *, u_int16_t);
> size_taspath_strlen(void *, u_int16_t);
> int aspath_match(void *, u_int16_t, struct filter_as *, u_int32_t);
> -int as_compare(u_int8_t, u_int32_t, u_int32_t, u_int32_t,
> - u_int32_t);
> u_int32_t aspath_extract(const void *, int);
> int aspath_verify(void *, u_int16_t, int);
> #define AS_ERR_LEN -1
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 util.c
> --- util.c22 Jul 2018 16:52:27 - 1.28
> +++ util.c9 Aug 2018 19:38:34 -
> @@ -312,6 +312,22 @@ aspath_strlen(void *data, u_int16_t len)
> return (total_size);
> }
>
> +static int
> +as_compare(struct filter_as *f, u_int32_t as, u_int32_t match)
> +{
> + if (as == 0)/* AS 0 is invalid and therefor never matches */
> + return (0);
> + if ((f->op == OP_NONE || f->op == OP_EQ) && as == match)
> + return (1);
> + else if (f->op == OP_NE && as != match)
> + return (1);
> + else if (f->op == OP_RANGE && as >= f->as_min && as <= f->as_max)
> + return (1);
> + else if (f->op == OP_XRANGE && (as < f->as_min || as > f->as_max))
> + return (1);
> + return (0);
> +}
> +
> /* we need to be able to search more than one as */
> int
> aspath_match(void *data, u_int16_t len, struct filter_as *f, u_int32_t match)
> @@ -320,7 +336,7 @@ aspath_match(void *data, u_int16_t len,
> int final;
> u_int16_tseg_size;
> u_int8_t i, seg_len;
> - u_int32_tas;
> + u_int32_tas = 0, preas;
>
> if (f->type == AS_EMPTY) {
> if (len == 0)
> @@ -330,27 +346,35 @@ aspath_match(void *data, u_int16_t len,
> }
>
> seg = data;
> - for (; len > 0; len -= seg_size, seg += seg_size) {
> +
> + /* just check the first (leftmost) AS */
> + if (f->type == AS_PEER && len >= 6) {
> + as = aspath_extract(seg, 0);
> + if (as_compare(f, as, match))
> + return (1);
> + else
> + return (0);
> + }
> +
> + for (; len >= 6; len -= seg_size, seg += seg_size) {
> seg_len = seg[1];
> seg_size = 2 + sizeof(u_int32_t) * seg_len;
>
> final = (len == seg_size);
>
> - /* just check the first (leftmost) AS */
> - if (f->type == AS_PEER) {
> - as = aspath_extract(seg, 0);
> - if (as_compare(f->op, as, match, f->as_min, f->as_max))
> - return (1);
> - else
> - return (0);
> - }
> /* just check the final (rightmost) AS */
> if (f->type == AS_SOURCE) {
> + /* extract the
Re: bgpd: refine source-as matching
On Thu, Aug 09, 2018 at 03:10:11PM +0200, Claudio Jeker wrote:
> Per rfc6472 AS_SET should no longer be used but some AS still do.
> Until now source-as would take the rightmost AS number of an AS_PATH no
> matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
> because AS_SET are used in aggregation source-as should match against the
> aggregator AS (which it the rightmost as of the previous AS_PATH segment).
> At the same time move the peer-as check out of the loop, there is no
> reason to have that inside the for loop.
>
> This diff implements this and also makes aspath_extract() and the lenght
> checks a bit more paranoid.
OK, updated diff here that adds three things.
a) it passes the filter_as struct to as_compare
b) it ensures that if for whatever crazy reason AS 0 is passed to the
filter it will not match (no matter what). This is more paranoia.
I decided to do that since util.c is shared between bgpd and bgpctl
and so the easy way of calling fatalx() is not an option here.
c) fix the souce-as 42 >< 4242 case. Currently the >< operator is not
correctly implemented it is more a <> as in inside but excluding the
edges. I think 42 >< 4242 should match for AS 1 - 41 and 4243 and
higher.
Is this still OK?
--
:wq Claudio
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.329
diff -u -p -r1.329 bgpd.h
--- bgpd.h 8 Aug 2018 14:29:05 - 1.329
+++ bgpd.h 9 Aug 2018 19:43:39 -
@@ -1156,8 +1156,6 @@ intaspath_snprint(char *, size_t, voi
int aspath_asprint(char **, void *, u_int16_t);
size_t aspath_strlen(void *, u_int16_t);
int aspath_match(void *, u_int16_t, struct filter_as *, u_int32_t);
-int as_compare(u_int8_t, u_int32_t, u_int32_t, u_int32_t,
- u_int32_t);
u_int32_t aspath_extract(const void *, int);
int aspath_verify(void *, u_int16_t, int);
#define AS_ERR_LEN -1
Index: util.c
===
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.28
diff -u -p -r1.28 util.c
--- util.c 22 Jul 2018 16:52:27 - 1.28
+++ util.c 9 Aug 2018 19:38:34 -
@@ -312,6 +312,22 @@ aspath_strlen(void *data, u_int16_t len)
return (total_size);
}
+static int
+as_compare(struct filter_as *f, u_int32_t as, u_int32_t match)
+{
+ if (as == 0)/* AS 0 is invalid and therefor never matches */
+ return (0);
+ if ((f->op == OP_NONE || f->op == OP_EQ) && as == match)
+ return (1);
+ else if (f->op == OP_NE && as != match)
+ return (1);
+ else if (f->op == OP_RANGE && as >= f->as_min && as <= f->as_max)
+ return (1);
+ else if (f->op == OP_XRANGE && (as < f->as_min || as > f->as_max))
+ return (1);
+ return (0);
+}
+
/* we need to be able to search more than one as */
int
aspath_match(void *data, u_int16_t len, struct filter_as *f, u_int32_t match)
@@ -320,7 +336,7 @@ aspath_match(void *data, u_int16_t len,
int final;
u_int16_tseg_size;
u_int8_t i, seg_len;
- u_int32_tas;
+ u_int32_tas = 0, preas;
if (f->type == AS_EMPTY) {
if (len == 0)
@@ -330,27 +346,35 @@ aspath_match(void *data, u_int16_t len,
}
seg = data;
- for (; len > 0; len -= seg_size, seg += seg_size) {
+
+ /* just check the first (leftmost) AS */
+ if (f->type == AS_PEER && len >= 6) {
+ as = aspath_extract(seg, 0);
+ if (as_compare(f, as, match))
+ return (1);
+ else
+ return (0);
+ }
+
+ for (; len >= 6; len -= seg_size, seg += seg_size) {
seg_len = seg[1];
seg_size = 2 + sizeof(u_int32_t) * seg_len;
final = (len == seg_size);
- /* just check the first (leftmost) AS */
- if (f->type == AS_PEER) {
- as = aspath_extract(seg, 0);
- if (as_compare(f->op, as, match, f->as_min, f->as_max))
- return (1);
- else
- return (0);
- }
/* just check the final (rightmost) AS */
if (f->type == AS_SOURCE) {
+ /* extract the rightmost AS and keep the one before */
+ preas = as;
+ as = aspath_extract(seg, seg_len - 1);
/* not yet in the final segment */
if (!final)
continue;
- as = aspath_extract(seg, seg_len - 1);
- if (as_compare(f
Re: Pass more EFI related information from bootloader to kernel
On Mon, Aug 06, 2018 at 11:38:28PM +0200, Mark Kettenis wrote:
> I suspect that at some point in the future we'll have to bite the
> bullet and call into EFI runtime services on amd64 as well. The diff
> below paves the road by passing the necessary information from our
> bootloader to the kernel.
>
> There is a potential issue with this diff. The EFI memory map is
> stored into the bootloader "heap" which is allocated with the
> EfiLoaderData attribute. Memory allocated with that attribute is
> treated as Free/Available by the kernel. So this data may be
> overwritten and the kernel would need to move it somewhere safe early
> in the boot process. It's not clear to me how early though? Can
> somebody who is more familiar with the amd64 and kernel and how it
> uses physical memory during early bootstrap shed some light on this?
>
That's probably me, but I'm a bit backed up at the moment. I'll take a
look this weekend.
-ml
>
> Index: arch/amd64/stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 efiboot.c
> --- arch/amd64/stand/efiboot/efiboot.c6 Jul 2018 07:55:50 -
> 1.30
> +++ arch/amd64/stand/efiboot/efiboot.c6 Aug 2018 21:22:33 -
> @@ -279,6 +279,7 @@ efi_device_path_ncmp(EFI_DEVICE_PATH *dp
> * Memory
> ***/
> bios_memmap_t bios_memmap[64];
> +bios_efiinfo_tbios_efiinfo;
>
> static void
> efi_heap_init(void)
> @@ -335,6 +336,9 @@ efi_memprobe_internal(void)
> cnvmem = extmem = 0;
> bios_memmap[0].type = BIOS_MAP_END;
>
> + if (bios_efiinfo.mmap_start != 0)
> + free((void *)bios_efiinfo.mmap_start, bios_efiinfo.mmap_size);
> +
> siz = 0;
> status = EFI_CALL(BS->GetMemoryMap, &siz, NULL, &mapkey, &mmsiz,
> &mmver);
> @@ -400,7 +404,11 @@ efi_memprobe_internal(void)
> bm->addr / 1024 == extmem + 1024)
> extmem += bm->size / 1024;
> }
> - free(mm0, siz);
> +
> + bios_efiinfo.mmap_desc_ver = mmver;
> + bios_efiinfo.mmap_desc_size = mmsiz;
> + bios_efiinfo.mmap_size = siz;
> + bios_efiinfo.mmap_start = (uintptr_t)mm0;
> }
>
> /***
> @@ -733,22 +741,21 @@ efi_makebootargs(void)
> EFI_STATUS status;
> EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
> *gopi;
> - bios_efiinfo_t ei;
> + bios_efiinfo_t *ei = &bios_efiinfo;
> int curmode;
> UINTNsz, gopsiz, bestsiz = 0;
>
> - memset(&ei, 0, sizeof(ei));
> /*
>* ACPI, BIOS configuration table
>*/
> for (i = 0; i < ST->NumberOfTableEntries; i++) {
> if (efi_guidcmp(&acpi_guid,
> &ST->ConfigurationTable[i].VendorGuid) == 0)
> - ei.config_acpi = (intptr_t)
> + ei->config_acpi = (uintptr_t)
> ST->ConfigurationTable[i].VendorTable;
> else if (efi_guidcmp(&smbios_guid,
> &ST->ConfigurationTable[i].VendorGuid) == 0)
> - ei.config_smbios = (intptr_t)
> + ei->config_smbios = (uintptr_t)
> ST->ConfigurationTable[i].VendorTable;
> }
>
> @@ -781,35 +788,44 @@ efi_makebootargs(void)
> gopi = gop->Mode->Info;
> switch (gopi->PixelFormat) {
> case PixelBlueGreenRedReserved8BitPerColor:
> - ei.fb_red_mask = 0x00ff;
> - ei.fb_green_mask= 0xff00;
> - ei.fb_blue_mask = 0x00ff;
> - ei.fb_reserved_mask = 0xff00;
> + ei->fb_red_mask = 0x00ff;
> + ei->fb_green_mask= 0xff00;
> + ei->fb_blue_mask = 0x00ff;
> + ei->fb_reserved_mask = 0xff00;
> break;
> case PixelRedGreenBlueReserved8BitPerColor:
> - ei.fb_red_mask = 0x00ff;
> - ei.fb_green_mask= 0xff00;
> - ei.fb_blue_mask = 0x00ff;
> - ei.fb_reserved_mask = 0xff00;
> + ei->fb_red_mask = 0x00ff;
> + ei->fb_green_mask= 0xff00;
> + ei->fb_blue_mask = 0x00ff;
> + ei->fb_reserved_mask = 0xff00;
> break;
> case PixelBitMask:
> - ei.fb_red_mask = gopi->PixelInformation.RedMask;
> - ei.fb_green_mask = gopi->PixelInformation.GreenM
diff: add USB ANT-m Stick via uscom(4)
Hi, the following diff adds support for an USB ANT+ receiver which is used to communicate with wireless fitness tracking devices. The USB device appears as a serial interface. I just add the device ID to the existing uscom(4) driver and enabled it in GENERIC. I don't know why it was deactivated. But, I tested the uscom(4) driver with my ANT device with OpenBSD-current on amd64, i386, macppc and sparc64. The device attache with this message: uscom0 at uhub0 port 1 configuration 1 interface 0 "Dynastream Innovations ANT USB-m Stick" rev 2.00/1.00 addr 2 ucom0 at uscom0 portno 0 It works well on every tested architecture. I got a valid binary error message from the ANT device by the following test command: # cu -l /dev/cuaU0 | od -h ... a4 01 ae 00 0b ... Did I test enough to enable this driver by default in GENERIC? Or did I miss something else? bye, Jan Index: share/man/man4/uscom.4 === RCS file: /cvs/src/share/man/man4/uscom.4,v retrieving revision 1.2 diff -u -p -r1.2 uscom.4 --- share/man/man4/uscom.4 25 Mar 2014 07:10:34 - 1.2 +++ share/man/man4/uscom.4 9 Aug 2018 18:24:50 - @@ -34,6 +34,7 @@ driver: .Bd -literal -offset indent HP 39G HP 49G +Dynastream ANT USB-m Stick .Ed .Sh SEE ALSO .Xr tty 4 , Index: sys/arch/amd64/conf/GENERIC === RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v retrieving revision 1.457 diff -u -p -r1.457 GENERIC --- sys/arch/amd64/conf/GENERIC 3 Aug 2018 01:50:14 - 1.457 +++ sys/arch/amd64/conf/GENERIC 9 Aug 2018 18:22:33 - @@ -230,6 +230,8 @@ umct* at uhub?# MCT USB-RS232 serial a ucom* at umct? uslcom*at uhub?# Silicon Laboratories CP210x serial ucom* at uslcom? +uscom* at uhub?# Simple USB serial adapters +ucom* at uscom? uark* at uhub?# Arkmicro ARK3116 serial ucom* at uark? moscom*at uhub?# MosChip MCS7703 serial Index: sys/arch/i386/conf/GENERIC === RCS file: /cvs/src/sys/arch/i386/conf/GENERIC,v retrieving revision 1.833 diff -u -p -r1.833 GENERIC --- sys/arch/i386/conf/GENERIC 3 Aug 2018 01:50:14 - 1.833 +++ sys/arch/i386/conf/GENERIC 9 Aug 2018 18:22:33 - @@ -246,6 +246,8 @@ umct* at uhub?# MCT USB-RS232 serial a ucom* at umct? uslcom*at uhub?# Silicon Laboratories CP210x serial ucom* at uslcom? +uscom* at uhub?# Simple USB serial adapters +ucom* at uscom? uark* at uhub?# Arkmicro ARK3116 serial ucom* at uark? moscom*at uhub?# MosChip MCS7703 serial Index: sys/arch/macppc/conf/GENERIC === RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC,v retrieving revision 1.264 diff -u -p -r1.264 GENERIC --- sys/arch/macppc/conf/GENERIC14 Feb 2018 23:51:49 - 1.264 +++ sys/arch/macppc/conf/GENERIC9 Aug 2018 18:22:33 - @@ -223,6 +223,8 @@ umct* at uhub?# MCT USB-RS232 serial a ucom* at umct? uslcom*at uhub?# Silicon Laboratories CP210x serial ucom* at uslcom? +uscom* at uhub?# Simple USB serial adapters +ucom* at uscom? uark* at uhub?# Arkmicro ARK3116 serial ucom* at uark? moscom*at uhub?# MosChip MCS7703 serial Index: sys/arch/sparc64/conf/GENERIC === RCS file: /cvs/src/sys/arch/sparc64/conf/GENERIC,v retrieving revision 1.307 diff -u -p -r1.307 GENERIC --- sys/arch/sparc64/conf/GENERIC 28 Aug 2017 19:32:53 - 1.307 +++ sys/arch/sparc64/conf/GENERIC 9 Aug 2018 18:22:33 - @@ -193,6 +193,8 @@ umct* at uhub?# MCT USB-RS232 serial a ucom* at umct? uslcom*at uhub?# Silicon Laboratories CP210x serial ucom* at uslcom? +uscom* at uhub?# Simple USB serial adapters +ucom* at uscom? uark* at uhub?# Arkmicro ARK3116 serial ucom* at uark? uipaq* at uhub?# iPAQ serial adapter Index: sys/dev/usb/usbdevs === RCS file: /cvs/src/sys/dev/usb/usbdevs,v retrieving revision 1.690 diff -u -p -r1.690 usbdevs --- sys/dev/usb/usbdevs 19 Jul 2018 17:33:26 - 1.690 +++ sys/dev/usb/usbdevs 9 Aug 2018 18:22:33 - @@ -1638,6 +1638,7 @@ product DVICO RT3070 0xb307 RT3070 product DYNASTREAM ANTDEVBOARD 0x1003 ANT dev board product DYNASTREAM ANT2USB 0x1004 ANT2USB product DYNASTREAM ANTDEVBOARD20x1006 ANT dev board +product DYNASTREAM ANTUSBM 0x1009 ANTUSB-m Stick /* EasyDisk products */ product EASYDISK EASYDISK 0x0005 Flash
Re: bgpd: refine source-as matching
On Thu, Aug 09, 2018 at 03:10:11PM +0200, Claudio Jeker wrote:
> Per rfc6472 AS_SET should no longer be used but some AS still do.
> Until now source-as would take the rightmost AS number of an AS_PATH
> no matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct.
Indeed, good find!
> Also because AS_SET are used in aggregation source-as should match
> against the aggregator AS (which it the rightmost as of the previous
> AS_PATH segment). At the same time move the peer-as check out of the
> loop, there is no reason to have that inside the for loop.
>
> This diff implements this and also makes aspath_extract() and the
> lenght checks a bit more paranoid.
tested:
$ bgpctl show rib source-as 8530 5.39.176.0/21
flags: * = Valid, > = Selected, I = via IBGP, A = Announced, S = Stale
origin: i = IGP, e = EGP, ? = Incomplete
flags destination gateway lpref med aspath origin
I*> 5.39.176.0/21192.147.168.1 100 0 2914 8530 {
198753 } ?
OK job@
Kind regards,
Job
Re: bgpd: refine source-as matching
Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.08.09 15:10:11 +0200:
> Per rfc6472 AS_SET should no longer be used but some AS still do.
> Until now source-as would take the rightmost AS number of an AS_PATH no
> matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
> because AS_SET are used in aggregation source-as should match against the
> aggregator AS (which it the rightmost as of the previous AS_PATH segment).
> At the same time move the peer-as check out of the loop, there is no
> reason to have that inside the for loop.
>
> This diff implements this and also makes aspath_extract() and the lenght
> checks a bit more paranoid.
> --
> :wq Claudio
ok benno@
with two comments on comments which you can choose to ignore.
>
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 util.c
> --- util.c22 Jul 2018 16:52:27 - 1.28
> +++ util.c9 Aug 2018 12:58:57 -
> @@ -320,7 +320,7 @@ aspath_match(void *data, u_int16_t len,
> int final;
> u_int16_tseg_size;
> u_int8_t i, seg_len;
> - u_int32_tas;
> + u_int32_tas = 0, preas;
>
> if (f->type == AS_EMPTY) {
> if (len == 0)
> @@ -330,26 +330,34 @@ aspath_match(void *data, u_int16_t len,
> }
>
> seg = data;
> - for (; len > 0; len -= seg_size, seg += seg_size) {
> +
In my head, the peer-as is the last as (that is added to the path), not the
first. Because i learned in school that as-paths are read from right to
left:
> + /* just check the first (leftmost) AS */
/* just check the last (leftmost) AS */
> + if (f->type == AS_PEER && len >= 6) {
> + as = aspath_extract(seg, 0);
> + if (as_compare(f->op, as, match, f->as_min, f->as_max))
> + return (1);
> + else
> + return (0);
> + }
> +
> + for (; len >= 6; len -= seg_size, seg += seg_size) {
> seg_len = seg[1];
> seg_size = 2 + sizeof(u_int32_t) * seg_len;
>
> final = (len == seg_size);
>
> - /* just check the first (leftmost) AS */
> - if (f->type == AS_PEER) {
> - as = aspath_extract(seg, 0);
> - if (as_compare(f->op, as, match, f->as_min, f->as_max))
> - return (1);
> - else
> - return (0);
> - }
> /* just check the final (rightmost) AS */
/* just check the origin (rightmost) AS */
> if (f->type == AS_SOURCE) {
> + /* extract the rightmost AS and keep the one before */
> + preas = as;
> + as = aspath_extract(seg, seg_len - 1);
> /* not yet in the final segment */
> if (!final)
> continue;
> - as = aspath_extract(seg, seg_len - 1);
> + if (seg[0] == AS_SET)
> + /* use aggregator AS per rfc6472 */
> + if (preas)
> + as = preas;
> if (as_compare(f->op, as, match, f->as_min, f->as_max))
> return (1);
> else
> @@ -389,7 +397,7 @@ as_compare(u_int8_t op, u_int32_t as, u_
> /*
> * Extract the asnum out of the as segment at the specified position.
> * Direct access is not possible because of non-aligned reads.
> - * ATTENTION: no bounds checks are done.
> + * Only works on verified and 4byte ASN paths.
> */
> u_int32_t
> aspath_extract(const void *seg, int pos)
> @@ -397,6 +405,9 @@ aspath_extract(const void *seg, int pos)
> const u_char*ptr = seg;
> u_int32_tas;
>
> + /* minimal overflow check, return 0 since that is an invalid ASN */
> + if (pos >= ptr[1])
> + return (0);
> ptr += 2 + sizeof(u_int32_t) * pos;
> memcpy(&as, ptr, sizeof(u_int32_t));
> return (ntohl(as));
>
Re: Very strange condition
On Thu, Aug 9, 2018 at 10:40 AM Mark Kettenis wrote:
>
> > From: sven falempin
> > Date: Thu, 9 Aug 2018 10:10:14 -0400
> >
> > In ACPI.c
> >
> >
> > if ((pm1 & ACPI_PM1_SCI_EN) == 0 && sc->sc_fadt->smi_cmd &&
> > (!sc->sc_fadt->acpi_enable && !sc->sc_fadt->acpi_disable)) {
> > printf(", ACPI control unavailable\n");
> > acpi_unmap_pmregs(sc);
> > return;
> > }
> >
> >
> > the condition test that the sc_fadt is NOT ENABLE AND NOT DISABLE
> >
> > wut ?
>
> No. It tests whether the magic that's needed to enable and disable is
> provided. If both are absent it decides that the ACPI implementation
> is probably borked and bails out.
>
Me heads hurt. Yes the system is screaming at me ;-)
I m gonna see if there is multple fadt table , as rev 6 ( my hardware
is rev 6 hdr )
say they could have a 64 and 32 bits and that you must prefer the bigger one ...
Wörse diff ever enables device to reboot properly when asked
Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.341
diff -u -p -r1.341 acpi.c
--- dev/acpi/acpi.c 27 Mar 2018 21:11:16 - 1.341
+++ dev/acpi/acpi.c 9 Aug 2018 15:26:59 -
@@ -980,11 +980,18 @@ acpi_attach(struct device *parent, struc
/*
* Find the FADT
*/
+ size_t fadt_s = sizeof(FADT_SIG) - 1;
SIMPLEQ_FOREACH(entry, &sc->sc_tables, q_next) {
- if (memcmp(entry->q_table, FADT_SIG,
- sizeof(FADT_SIG) - 1) == 0) {
- sc->sc_fadt = entry->q_table;
- break;
+ if (memcmp(entry->q_table, FADT_SIG,fadt_s) == 0) {
+ if (sc->sc_fadt == NULL) {
+ sc->sc_fadt = entry->q_table;
+ } else {
+ struct acpi_fadt*p = entry->q_table;
+ printf(", more than one FADT old s:
%d,new s: %d\n",sc->sc_fadt->hdr_length, p->hdr_length);
+ if (sc->sc_fadt->hdr_length < p->hdr_length) {
+ sc->sc_fadt = entry->q_table;
+ }
+ }
}
}
if (sc->sc_fadt == NULL) {
@@ -999,6 +1006,8 @@ acpi_attach(struct device *parent, struc
if (sc->sc_fadt->hdr_revision >= 5 &&
sc->sc_fadt->flags & FADT_HW_REDUCED_ACPI)
sc->sc_hw_reduced = 1;
+printf(", hdr_revision %d", sc->sc_fadt->hdr_revision);
+ printf(", ACPI FATD %08X\n", sc->sc_fadt->flags);
/* Map Power Management registers */
acpi_map_pmregs(sc);
@@ -1093,7 +1102,7 @@ acpi_attach(struct device *parent, struc
if ((pm1 & ACPI_PM1_SCI_EN) == 0 && sc->sc_fadt->smi_cmd) {
if (acpi_enable(sc)) {
printf(", can't enable ACPI\n");
- return;
+ // return;
}
}
dmesg:
OpenBSD 6.3-current (M.MP) #12: Thu Aug 9 11:11:50 EDT 2018
r...@imeee.com:/usr/src/sys/arch/amd64/compile/M .MP
real mem = 17130721280 (16337MB)
avail mem = 16604438528 (15835MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f1fc000 (18 entries)
bios0: vendor American Megatrends Inc. version "0ACHIA01" date 06/14/2018
bios0: NF699 NF699
acpi0 at bios0: rev 2, hdr_revision 6, ACPI FATD 0002C4A5
acpi0: sleep states S0 S4 S5, can't enable ACPI
acpi0: tables DSDT FACP FPDT FIDT MCFG WDAT APIC BDAT HPET UEFI SSDT
DMAR HEST BERT ERST EINJ WSMT
acpi0: wakeup devices PEX0(S4) PEX1(S4) PEX2(S4) PEX3(S4) PEX4(S4)
PEX5(S4) PEX6(S4) PEX7(S4) XHC1(S4) LAN0(S4) LAN1(S4) LAN2(S4)
LAN3(S4)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0 addr 0xe000, bus 0-255
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Atom(TM) CPU C3758 @ 2.20GHz, 2200.43 MHz
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,FSGSBASE,SMEP,ERMS,MPX,RDSEED,SMAP,CLFLUSHOPT,PT,SHA,IBRS,IBPB,STIBP,SENSOR,ARAT
cpu0: 2MB 64b/line 16-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 25MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.2, IBE
cpu1 at mainbus0: apid 4 (application processor)
cpu1: Intel(R) Atom(TM) CPU C3758 @ 2.20GHz, 2200.02 MHz
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,P
Re: Very strange condition
> From: sven falempin
> Date: Thu, 9 Aug 2018 10:10:14 -0400
>
> In ACPI.c
>
>
> if ((pm1 & ACPI_PM1_SCI_EN) == 0 && sc->sc_fadt->smi_cmd &&
> (!sc->sc_fadt->acpi_enable && !sc->sc_fadt->acpi_disable)) {
> printf(", ACPI control unavailable\n");
> acpi_unmap_pmregs(sc);
> return;
> }
>
>
> the condition test that the sc_fadt is NOT ENABLE AND NOT DISABLE
>
> wut ?
No. It tests whether the magic that's needed to enable and disable is
provided. If both are absent it decides that the ACPI implementation
is probably borked and bails out.
Very strange condition
In ACPI.c
if ((pm1 & ACPI_PM1_SCI_EN) == 0 && sc->sc_fadt->smi_cmd &&
(!sc->sc_fadt->acpi_enable && !sc->sc_fadt->acpi_disable)) {
printf(", ACPI control unavailable\n");
acpi_unmap_pmregs(sc);
return;
}
the condition test that the sc_fadt is NOT ENABLE AND NOT DISABLE
wut ?
--
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do
bgpd: refine source-as matching
Per rfc6472 AS_SET should no longer be used but some AS still do.
Until now source-as would take the rightmost AS number of an AS_PATH no
matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
because AS_SET are used in aggregation source-as should match against the
aggregator AS (which it the rightmost as of the previous AS_PATH segment).
At the same time move the peer-as check out of the loop, there is no
reason to have that inside the for loop.
This diff implements this and also makes aspath_extract() and the lenght
checks a bit more paranoid.
--
:wq Claudio
Index: util.c
===
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.28
diff -u -p -r1.28 util.c
--- util.c 22 Jul 2018 16:52:27 - 1.28
+++ util.c 9 Aug 2018 12:58:57 -
@@ -320,7 +320,7 @@ aspath_match(void *data, u_int16_t len,
int final;
u_int16_tseg_size;
u_int8_t i, seg_len;
- u_int32_tas;
+ u_int32_tas = 0, preas;
if (f->type == AS_EMPTY) {
if (len == 0)
@@ -330,26 +330,34 @@ aspath_match(void *data, u_int16_t len,
}
seg = data;
- for (; len > 0; len -= seg_size, seg += seg_size) {
+
+ /* just check the first (leftmost) AS */
+ if (f->type == AS_PEER && len >= 6) {
+ as = aspath_extract(seg, 0);
+ if (as_compare(f->op, as, match, f->as_min, f->as_max))
+ return (1);
+ else
+ return (0);
+ }
+
+ for (; len >= 6; len -= seg_size, seg += seg_size) {
seg_len = seg[1];
seg_size = 2 + sizeof(u_int32_t) * seg_len;
final = (len == seg_size);
- /* just check the first (leftmost) AS */
- if (f->type == AS_PEER) {
- as = aspath_extract(seg, 0);
- if (as_compare(f->op, as, match, f->as_min, f->as_max))
- return (1);
- else
- return (0);
- }
/* just check the final (rightmost) AS */
if (f->type == AS_SOURCE) {
+ /* extract the rightmost AS and keep the one before */
+ preas = as;
+ as = aspath_extract(seg, seg_len - 1);
/* not yet in the final segment */
if (!final)
continue;
- as = aspath_extract(seg, seg_len - 1);
+ if (seg[0] == AS_SET)
+ /* use aggregator AS per rfc6472 */
+ if (preas)
+ as = preas;
if (as_compare(f->op, as, match, f->as_min, f->as_max))
return (1);
else
@@ -389,7 +397,7 @@ as_compare(u_int8_t op, u_int32_t as, u_
/*
* Extract the asnum out of the as segment at the specified position.
* Direct access is not possible because of non-aligned reads.
- * ATTENTION: no bounds checks are done.
+ * Only works on verified and 4byte ASN paths.
*/
u_int32_t
aspath_extract(const void *seg, int pos)
@@ -397,6 +405,9 @@ aspath_extract(const void *seg, int pos)
const u_char*ptr = seg;
u_int32_tas;
+ /* minimal overflow check, return 0 since that is an invalid ASN */
+ if (pos >= ptr[1])
+ return (0);
ptr += 2 + sizeof(u_int32_t) * pos;
memcpy(&as, ptr, sizeof(u_int32_t));
return (ntohl(as));
Re: pfctl: use mask parameter and zap bits in host_v4()
On Wed, Aug 01, 2018 at 01:27:48AM +0200, Klemens Nanni wrote:
> Updated diff bringing this function in line with its counterparts by
> using `mask' the same way.
>
> If a mask was specified, `mask' would always equal to `bits' as returned
> by inet_net_pton(), so avoid the duplicate.
>
> While here, directly use the destination size in memcpy() instead of
> hardcoding its type.
I'd still like to see this in as it makes host_v4() consistent with
host_v6() with regard to `mask' handling.
OK?
Index: pfctl_parser.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v
retrieving revision 1.326
diff -u -p -r1.326 pfctl_parser.c
--- pfctl_parser.c 31 Jul 2018 22:48:04 - 1.326
+++ pfctl_parser.c 31 Jul 2018 23:25:01 -
@@ -1727,11 +1727,10 @@ host_v4(const char *s, int mask)
{
struct node_host*h = NULL;
struct in_addr ina;
- int bits = 32;
- memset(&ina, 0, sizeof(struct in_addr));
- if (strrchr(s, '/') != NULL) {
- if ((bits = inet_net_pton(AF_INET, s, &ina, sizeof(ina))) == -1)
+ memset(&ina, 0, sizeof(ina));
+ if (mask > -1) {
+ if (inet_net_pton(AF_INET, s, &ina, sizeof(ina)) == -1)
return (NULL);
} else {
if (inet_pton(AF_INET, s, &ina) != 1)
@@ -1744,7 +1743,7 @@ host_v4(const char *s, int mask)
h->ifname = NULL;
h->af = AF_INET;
h->addr.v.a.addr.addr32[0] = ina.s_addr;
- set_ipmask(h, bits);
+ set_ipmask(h, mask > -1 ? mask : 32);
h->next = NULL;
h->tail = h;
Re: openssl(1) passwd chunck canary corrupted
On Thu, Aug 09, 2018 at 08:45:03AM +0200, Theo Buehler wrote:
> On Wed, Aug 08, 2018 at 05:53:56PM +0200, Theo Buehler wrote:
> > On Wed, Aug 08, 2018 at 02:45:29PM +0100, Ricardo Mestre wrote:
> > > Hi,
> > >
> > > When openssl(1) passwd is invoked without passing in the `password' as
> > > argument
> > > , meaning interactively, and if the password is 10 or more characters it
> > > will
> > > show the following memory corruption error, and using -crypt which is the
> > > default:
> > >
> > > openssl(43025) in free(): chunck canary corrupted 0x13a8dc4a1bb0 0xa@0xa
> > >
> > > pw_len is set to 8, then passwd_malloc_size is set to pw_len + 2 in order
> > > to be
> > > able to warn the user that the password will be truncated then it calls
> > > EVP_read_pw_string(3) which allocates the space size of the input buffer,
> > > in
> > > this case password_malloc_size plus 1 for the NUL-termination character
> > > through
> > > strlcpy(3).
> > >
> > > When we finally call free(password_malloc) the sizes will differ and the
> > > memory
> > > will be corrupted, in order to solve this when we allocate memory for the
> > > input
> > > buffer we need to add plus 1 for the NUL-termination character.
> > >
> > > Comments? OK?
> >
> > Nice find. I think this is an off-by-one in EVP_read_pw_string_min()'s
> > usage of UI_add_input_string() (and UI_add_verify_string()). The man
> > page explicitly says that NUL is *not* counted in minlen and maxlen.
> >
> > UI_add_input_string() and UI_add_verify_string() add a prompt to ui, as
> > well as flags and a result buffer and the desired minimum and maximum
> > sizes of the result, not counting the final NUL character.
> >
> > Notice that EVP_read_pw_string(buf, sizeof buf, ...) is a common enough
> > idiom. All these are subject to a similar buffer overrun. On the other
> > hand, everywhere else our tree has the correct usage of
> >
> > UI_add_input_string(ui, prompt, flags, buf, 0, sizeof(buf) - 1),
> >
> > so I believe we should fix this in EVP_read_pw_string_min() as follows:
>
> I got some oks on this, but here's a diff that I like better since it's
> more defensive.
>
> First, ints min and len only make sense if they're non-negative, so
> let's check for that. Second, let's check for the maxlen < minlen
> situation as the UI_* family of functions doesn't seem to do that.
> Third, it seems easier to cap len at BUFSIZ instead of repeating the use
> of the ternary operator.
>
> I forgot to mention that this diff also aligns EVP_read_pw_string() with
> the documented behavior whereas currently buf would need to be of size
> length+1 to avoid the buffer overrun.
Right after sending I noticed that I had the checks in the wrong order.
Sorry about that.
Index: evp/evp_key.c
===
RCS file: /var/cvs/src/lib/libcrypto/evp/evp_key.c,v
retrieving revision 1.24
diff -u -p -r1.24 evp_key.c
--- evp/evp_key.c 29 Jan 2017 17:49:23 - 1.24
+++ evp/evp_key.c 9 Aug 2018 07:08:20 -
@@ -101,17 +101,20 @@ EVP_read_pw_string_min(char *buf, int mi
char buff[BUFSIZ];
UI *ui;
+ if (len > BUFSIZ)
+ len = BUFSIZ;
+ if (min < 0 || len - 1 < min)
+ return -1;
if ((prompt == NULL) && (prompt_string[0] != '\0'))
prompt = prompt_string;
ui = UI_new();
if (ui == NULL)
return -1;
- if (UI_add_input_string(ui, prompt, 0, buf, min,
- (len >= BUFSIZ) ? BUFSIZ - 1 : len) < 0)
+ if (UI_add_input_string(ui, prompt, 0, buf, min, len - 1) < 0)
return -1;
if (verify) {
- if (UI_add_verify_string(ui, prompt, 0, buff, min,
- (len >= BUFSIZ) ? BUFSIZ - 1 : len, buf) < 0)
+ if (UI_add_verify_string(ui, prompt, 0, buff, min, len - 1, buf)
+ < 0)
return -1;
}
ret = UI_process(ui);
