cleanup defunct prototype in snmpe.c
It looks like some code was shuffled around in revision 1.34 in which snmpe_application was renamed to smi_application and this prototype was missed. ok? Index: snmpe.c === RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v retrieving revision 1.52 diff -u -p -r1.52 snmpe.c --- snmpe.c 15 Apr 2018 11:57:29 - 1.52 +++ snmpe.c 23 Jul 2018 19:32:40 - @@ -45,8 +45,6 @@ intsnmpe_parse(struct snmp_message *); voidsnmpe_tryparse(int, struct snmp_message *); int snmpe_parsevarbinds(struct snmp_message *); voidsnmpe_response(struct snmp_message *); -unsigned long -snmpe_application(struct ber_element *); voidsnmpe_sig_handler(int sig, short, void *); int snmpe_dispatch_parent(int, struct privsep_proc *, struct imsg *); int snmpe_bind(struct address *);
mention some missing macros in libevent man pages
Add a few macros in use but currently missing from the man page. ok? Index: event.3 === RCS file: /cvs/src/lib/libevent/event.3,v retrieving revision 1.53 diff -u -p -r1.53 event.3 --- event.3 29 Jun 2017 01:25:59 - 1.53 +++ event.3 25 Jul 2018 20:00:44 - @@ -67,7 +67,9 @@ .Nm bufferevent_read , .Nm bufferevent_enable , .Nm bufferevent_disable , -.Nm bufferevent_settimeout +.Nm bufferevent_settimeout , +.Nm EVBUFFER_INPUT , +.Nm EVBUFFER_OUTPUT .Nd execute a function when a specific event occurs .Sh SYNOPSIS .In sys/time.h @@ -154,6 +156,10 @@ .Fn "bufferevent_disable" "struct bufferevent *bufev" "short event" .Ft void .Fn "bufferevent_settimeout" "struct bufferevent *bufev" "int timeout_read" "int timeout_write" +.Ft "struct evbuffer *" +.Fn "EVBUFFER_INPUT" "struct bufferevent *bufev" +.Ft "struct evbuffer *" +.Fn "EVBUFFER_OUTPUT" "struct bufferevent *bufev" .Sh DESCRIPTION The .Nm event @@ -507,6 +513,17 @@ If multiple bases are in use, .Fn bufferevent_base_set must be called before enabling the bufferevent for the first time. +.Pp +The +.Fn EVBUFFER_INPUT +and +.Fn EVBUFFER_OUTPUT +macros return a pointer to evbuffer +.Fa input +and +.Fa output +respectively for the specified bufferevent +.Fa bufev . .Sh ADDITIONAL NOTES It is possible to disable support for .Va kqueue , poll Index: evbuffer_new.3 === RCS file: /cvs/src/lib/libevent/evbuffer_new.3,v retrieving revision 1.13 diff -u -p -r1.13 evbuffer_new.3 --- evbuffer_new.3 1 Aug 2017 14:57:03 - 1.13 +++ evbuffer_new.3 25 Jul 2018 20:00:48 - @@ -33,7 +33,8 @@ .Nm evbuffer_find , .Nm evbuffer_readline , .Nm evbuffer_readln , -.Nm EVBUFFER_LENGTH +.Nm EVBUFFER_LENGTH , +.Nm EVBUFFER_DATA .Nd libevent utility API for buffered input/output .Sh SYNOPSIS .In event.h @@ -77,6 +78,8 @@ .Fc .Ft size_t .Fn "EVBUFFER_LENGTH" "const struct evbuffer *buf" +.Ft "u_char *" +.Fn "EVBUFFER_DATA" "const struct evbuffer *buf" .Sh DESCRIPTION The evbuffer API provides an implementation of buffering for use with libevent. @@ -259,6 +262,11 @@ on failure. .Pp .Fn EVBUFFER_LENGTH returns the number of bytes available in the evbuffer. +.Pp +.Fn EVBUFFER_DATA +returns a pointer to the evbuffer +.Fa buf +on success. .Sh SEE ALSO .Xr errno 2 , .Xr event 3 ,
relocate some public ber functions
Some public ber functions sneaked in below the internal functions comment. Move them up so the comment regains its former truthiness. Ok? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.14 diff -u -p -r1.14 ber.c --- usr.bin/ldap/ber.c 13 Jul 2018 08:50:38 - 1.14 +++ usr.bin/ldap/ber.c 30 Jul 2018 20:20:40 - @@ -430,6 +430,32 @@ ber_string2oid(const char *oidstr, struc return (0); } +int +ber_oid_cmp(struct ber_oid *a, struct ber_oid *b) +{ + size_t i; + for (i = 0; i < BER_MAX_OID_LEN; i++) { + if (a->bo_id[i] != 0) { + if (a->bo_id[i] == b->bo_id[i]) + continue; + else if (a->bo_id[i] < b->bo_id[i]) { + /* b is a successor of a */ + return (1); + } else { + /* b is a predecessor of a */ + return (-1); + } + } else if (b->bo_id[i] != 0) { + /* b is larger, but a child of a */ + return (2); + } else + break; + } + + /* b and a are identical */ + return (0); +} + struct ber_element * ber_add_oid(struct ber_element *prev, struct ber_oid *o) { @@ -756,6 +782,15 @@ ber_scanf_elements(struct ber_element *b } +ssize_t +ber_get_writebuf(struct ber *b, void **buf) +{ + if (b->br_wbuf == NULL) + return -1; + *buf = b->br_wbuf; + return (b->br_wend - b->br_wbuf); +} + /* * write ber elements to the write buffer * @@ -795,6 +830,13 @@ ber_write_elements(struct ber *ber, stru return (len); } +void +ber_set_readbuf(struct ber *b, void *buf, size_t len) +{ + b->br_rbuf = b->br_rptr = buf; + b->br_rend = (u_int8_t *)buf + len; +} + /* * read ber elements from the read buffer * @@ -896,6 +938,26 @@ ber_calc_len(struct ber_element *root) return (root->be_len + size); } +void +ber_set_application(struct ber *b, unsigned long (*cb)(struct ber_element *)) +{ + b->br_application = cb; +} + +void +ber_set_writecallback(struct ber_element *elm, void (*cb)(void *, size_t), +void *arg) +{ + elm->be_cb = cb; + elm->be_cbarg = arg; +} + +void +ber_free(struct ber *b) +{ + free(b->br_wbuf); +} + /* * internal functions */ @@ -1204,42 +1266,6 @@ ber_read_element(struct ber *ber, struct return totlen; } -void -ber_set_readbuf(struct ber *b, void *buf, size_t len) -{ - b->br_rbuf = b->br_rptr = buf; - b->br_rend = (u_int8_t *)buf + len; -} - -ssize_t -ber_get_writebuf(struct ber *b, void **buf) -{ - if (b->br_wbuf == NULL) - return -1; - *buf = b->br_wbuf; - return (b->br_wend - b->br_wbuf); -} - -void -ber_set_application(struct ber *b, unsigned long (*cb)(struct ber_element *)) -{ - b->br_application = cb; -} - -void -ber_set_writecallback(struct ber_element *elm, void (*cb)(void *, size_t), -void *arg) -{ - elm->be_cb = cb; - elm->be_cbarg = arg; -} - -void -ber_free(struct ber *b) -{ - free(b->br_wbuf); -} - static ssize_t ber_getc(struct ber *b, u_char *c) { @@ -1265,30 +1291,4 @@ ber_read(struct ber *ber, void *buf, siz ber->br_offs += len; return len; -} - -int -ber_oid_cmp(struct ber_oid *a, struct ber_oid *b) -{ - size_t i; - for (i = 0; i < BER_MAX_OID_LEN; i++) { - if (a->bo_id[i] != 0) { - if (a->bo_id[i] == b->bo_id[i]) - continue; - else if (a->bo_id[i] < b->bo_id[i]) { - /* b is a successor of a */ - return (1); - } else { - /* b is a predecessor of a */ - return (-1); - } - } else if (b->bo_id[i] != 0) { - /* b is larger, but a child of a */ - return (2); - } else - break; - } - - /* b and a are identical */ - return (0); } Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.24 diff -u -p -r1.24 ber.c --- usr.sbin/ldapd/ber.c13 Jul 2018 08:50:38 - 1.24 +++ usr.sbin/ldapd/ber.c30 Jul 2018 20:21:58 - @@ -430,6 +430,32 @@ ber_string2oid(const char *oidstr, struc return (0); } +int +ber_oid_cmp(struct ber_oid *a, struct ber_oid *b) +{ + size_t i; + for (i = 0; i < BER_MAX_OID_LEN; i++) { + if (a->bo_id[i] != 0) { + if (a->bo_id[i] == b->b
tweaks to namei.9
A little less wordy when introducing the namieidata structure. Ok? Index: namei.9 === RCS file: /cvs/src/share/man/man9/namei.9,v retrieving revision 1.18 diff -u -p -r1.18 namei.9 --- namei.9 23 Nov 2015 17:53:57 - 1.18 +++ namei.9 2 Aug 2018 13:51:43 - @@ -67,10 +67,9 @@ for name-to-inode conversion, in the day .Xr vfs 9 interface was implemented. .Pp -The arguments passed to the functions are encapsulated in the +Arguments passed to these functions are encapsulated in the following .Em nameidata -structure. -It has the following structure: +structure: .Bd -literal struct nameidata { /*
Re: tweaks to namei.9
On Thu, Aug 02, 2018 at 03:15:14PM +0100, Jason McIntyre wrote: > On Thu, Aug 02, 2018 at 01:58:38PM +0000, Rob Pierce wrote: > > A little less wordy when introducing the namieidata structure. > > > > Ok? > > > > Index: namei.9 > > === > > RCS file: /cvs/src/share/man/man9/namei.9,v > > retrieving revision 1.18 > > diff -u -p -r1.18 namei.9 > > --- namei.9 23 Nov 2015 17:53:57 - 1.18 > > +++ namei.9 2 Aug 2018 13:51:43 - > > @@ -67,10 +67,9 @@ for name-to-inode conversion, in the day > > .Xr vfs 9 > > interface was implemented. > > .Pp > > -The arguments passed to the functions are encapsulated in the > > +Arguments passed to these functions are encapsulated in the following > > .Em nameidata > > -structure. > > -It has the following structure: > > +structure: > > .Bd -literal > > struct nameidata { > > /* > > > > hi. > > i'm not sure it's a big win - it's just another way of saying the same > thing. but now it can be interpreted to mean that there are more than one > type of namei structure. > > the use of "structure" twice isn;t ideal though, i agree. > > jmc I agree, this is not a big win, but when I read four instances of "the" and two instances of "structure" in two sentence where one would do, I start to lose focus. I don't see how this could be misinterpreted, but maybe I am missing something. Rob
Re: please test: unveil for ifconfig
On Thu, Aug 02, 2018 at 07:04:31PM +0200, Florian Obser wrote: > I have been told that this is going to fall into snaps soon. If you > are doing weird (or normal) things with ifconfig, please test. I am running a kernel with some unveil error condition debugging, and with that I proceeded to test the ifstated regression test (which exercises some basic ifconfig operations). Everything worked as expected, and no strange unveil kernel messages in my logs. Rob > In particular if you use rulefile. > > Thanks! > > diff --git ifconfig.c ifconfig.c > index 9bfb1751aab..873aed5bcc7 100644 > --- ifconfig.c > +++ ifconfig.c > @@ -676,10 +676,13 @@ main(int argc, char *argv[]) > int create = 0; > int Cflag = 0; > int gflag = 0; > + int found_rulefile = 0; > int i; > > /* If no args at all, print all interfaces. */ > if (argc < 2) { > + if (unveil(NULL, NULL) == -1) > + err(1, "unveil"); > aflag = 1; > printif(NULL, 0); > return (0); > @@ -721,6 +724,18 @@ main(int argc, char *argv[]) > } else if (strlcpy(name, *argv, sizeof(name)) >= IFNAMSIZ) > errx(1, "interface name '%s' too long", *argv); > argc--, argv++; > + > + for (i = 0; i < argc; i++) { > + if (strcmp(argv[0], "rulefile") == 0) { > + found_rulefile = 1; > + break; > + } > + } > + > + if (!found_rulefile) > + if (unveil(NULL, NULL) == -1) > + err(1, "unveil"); > + > if (argc > 0) { > for (afp = rafp = afs; rafp->af_name; rafp++) > if (strcmp(rafp->af_name, *argv) == 0) { > > > -- > I'm not entirely sure you are real. >
Re: please test: unveil for ifconfig
- Original Message - > From: "Bryan Steele" > To: "tech" > Sent: Thursday, August 2, 2018 1:24:48 PM > Subject: Re: please test: unveil for ifconfig > On Thu, Aug 02, 2018 at 07:04:31PM +0200, Florian Obser wrote: > > I have been told that this is going to fall into snaps soon. If you > > are doing weird (or normal) things with ifconfig, please test. > > In particular if you use rulefile. > > Thanks! > > diff --git ifconfig.c ifconfig.c > > index 9bfb1751aab..873aed5bcc7 100644 > > --- ifconfig.c > > +++ ifconfig.c > > @@ -676,10 +676,13 @@ main(int argc, char *argv[]) > > int create = 0; > > int Cflag = 0; > > int gflag = 0; > > + int found_rulefile = 0; > > int i; > > /* If no args at all, print all interfaces. */ > > if (argc < 2) { > > + if (unveil(NULL, NULL) == -1) > > + err(1, "unveil"); > > aflag = 1; > > printif(NULL, 0); > > return (0); > > @@ -721,6 +724,18 @@ main(int argc, char *argv[]) > > } else if (strlcpy(name, *argv, sizeof(name)) >= IFNAMSIZ) > > errx(1, "interface name '%s' too long", *argv); > > argc--, argv++; > > + > > + for (i = 0; i < argc; i++) { > > + if (strcmp(argv[0], "rulefile") == 0) { > > + found_rulefile = 1; > > + break; > > + } > > + } > > + > > + if (!found_rulefile) > > + if (unveil(NULL, NULL) == -1) > > + err(1, "unveil"); > > + > > if (argc > 0) { > > for (afp = rafp = afs; rafp->af_name; rafp++) > > if (strcmp(rafp->af_name, *argv) == 0) { > Are these the only two unveil(2) calls in ifconfig(8)? If not mistaken, > unless you unveil something before the call to disable unveil, it never > actually restricts access to the filesystem, instead you need: > if (unveil("/", "") == -1) > err(1, "unveil"); > if (unveil(NULL, NULL) == -1) > err(1, "unveil"); > -Bryan. Ah, yes. unveil(NULL, NULL) does not activate unveil. A recent man page update captures this a little better. I will test again. with your addition. Rob
Re: please test: unveil for ifconfig
- Original Message - > From: "Rob Pierce" > To: "Bryan Steele" > Cc: "tech" > Sent: Thursday, August 2, 2018 1:30:15 PM > Subject: Re: please test: unveil for ifconfig > - Original Message - > > From: "Bryan Steele" > > To: "tech" > > Sent: Thursday, August 2, 2018 1:24:48 PM > > Subject: Re: please test: unveil for ifconfig > > On Thu, Aug 02, 2018 at 07:04:31PM +0200, Florian Obser wrote: > > > I have been told that this is going to fall into snaps soon. If you > > > are doing weird (or normal) things with ifconfig, please test. > > > In particular if you use rulefile. > > > Thanks! > > > diff --git ifconfig.c ifconfig.c > > > index 9bfb1751aab..873aed5bcc7 100644 > > > --- ifconfig.c > > > +++ ifconfig.c > > > @@ -676,10 +676,13 @@ main(int argc, char *argv[]) > > > int create = 0; > > > int Cflag = 0; > > > int gflag = 0; > > > + int found_rulefile = 0; > > > int i; > > > /* If no args at all, print all interfaces. */ > > > if (argc < 2) { > > > + if (unveil(NULL, NULL) == -1) > > > + err(1, "unveil"); > > > aflag = 1; > > > printif(NULL, 0); > > > return (0); > > > @@ -721,6 +724,18 @@ main(int argc, char *argv[]) > > > } else if (strlcpy(name, *argv, sizeof(name)) >= IFNAMSIZ) > > > errx(1, "interface name '%s' too long", *argv); > > > argc--, argv++; > > > + > > > + for (i = 0; i < argc; i++) { > > > + if (strcmp(argv[0], "rulefile") == 0) { > > > + found_rulefile = 1; > > > + break; > > > + } > > > + } > > > + > > > + if (!found_rulefile) > > > + if (unveil(NULL, NULL) == -1) > > > + err(1, "unveil"); > > > + > > > if (argc > 0) { > > > for (afp = rafp = afs; rafp->af_name; rafp++) > > > if (strcmp(rafp->af_name, *argv) == 0) { > > Are these the only two unveil(2) calls in ifconfig(8)? If not mistaken, > > unless you unveil something before the call to disable unveil, it never > > actually restricts access to the filesystem, instead you need: > > if (unveil("/", "") == -1) > > err(1, "unveil"); > > if (unveil(NULL, NULL) == -1) > > err(1, "unveil"); > > -Bryan. > Ah, yes. unveil(NULL, NULL) does not activate unveil. A recent man page update > captures this a little better. > I will test again. with your addition. > Rob ifstated regress passes with modified unveil calls. They are pretty basic use cases though. Rob
avoid overflow in snmp message id
Prevent server side (snmpd) overflow for message id in the snmp header. ok? Index: snmpclient.c === RCS file: /cvs/src/usr.sbin/snmpctl/snmpclient.c,v retrieving revision 1.16 diff -u -p -r1.16 snmpclient.c --- snmpclient.c8 Aug 2018 18:50:38 - 1.16 +++ snmpclient.c10 Aug 2018 14:21:23 - @@ -407,7 +407,7 @@ snmpc_sendreq(struct snmpc *sc, unsigned erroridx = SNMPC_MAXREPETITIONS; /* SNMP header */ - sc->sc_msgid = arc4random(); + sc->sc_msgid = arc4random() & 0x7fff; if ((root = ber_add_sequence(NULL)) == NULL) return (-1); if ((b = ber_printf_elements(root, "ds{tddd{{O0}}",
change ber_write_elements to return ssize_t
In aldap.c, tls_write(2) and write(2) also return ssize_t, so both error and wrote have been changed accordingly. ok? Index: usr.bin/ldap/aldap.c === RCS file: /cvs/src/usr.bin/ldap/aldap.c,v retrieving revision 1.4 diff -u -p -r1.4 aldap.c --- usr.bin/ldap/aldap.c31 Jul 2018 11:37:18 - 1.4 +++ usr.bin/ldap/aldap.c12 Aug 2018 01:53:10 - @@ -126,10 +126,10 @@ aldap_tls(struct aldap *ldap, struct tls int aldap_send(struct aldap *ldap, struct ber_element *root) { - int error, wrote; void *ptr; char *data; size_t len, done; + ssize_t error, wrote; len = ber_calc_len(root); error = ber_write_elements(&ldap->ber, root); @@ -311,7 +311,7 @@ int aldap_create_page_control(struct ber_element *elm, int size, struct aldap_page_control *page) { - int len; + ssize_t len; struct ber c; struct ber_element *ber = NULL; Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.18 diff -u -p -r1.18 ber.c --- usr.bin/ldap/ber.c 3 Aug 2018 01:51:28 - 1.18 +++ usr.bin/ldap/ber.c 12 Aug 2018 01:53:10 - @@ -802,7 +802,7 @@ ber_get_writebuf(struct ber *b, void **b * >=0 number of bytes written * -1 on failure and sets errno */ -int +ssize_t ber_write_elements(struct ber *ber, struct ber_element *root) { size_t len; Index: usr.bin/ldap/ber.h === RCS file: /cvs/src/usr.bin/ldap/ber.h,v retrieving revision 1.6 diff -u -p -r1.6 ber.h --- usr.bin/ldap/ber.h 3 Aug 2018 01:51:28 - 1.6 +++ usr.bin/ldap/ber.h 12 Aug 2018 01:53:10 - @@ -124,7 +124,7 @@ int ber_string2oid(const char *, stru struct ber_element *ber_printf_elements(struct ber_element *, char *, ...); int ber_scanf_elements(struct ber_element *, char *, ...); ssize_t ber_get_writebuf(struct ber *, void **); -int ber_write_elements(struct ber *, struct ber_element *); +ssize_t ber_write_elements(struct ber *, struct ber_element *); voidber_set_readbuf(struct ber *, void *, size_t); struct ber_element *ber_read_elements(struct ber *, struct ber_element *); off_t ber_getpos(struct ber_element *); cvs server: Diffing usr.sbin cvs server: Diffing usr.sbin/ldapd Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.28 diff -u -p -r1.28 ber.c --- usr.sbin/ldapd/ber.c3 Aug 2018 01:51:28 - 1.28 +++ usr.sbin/ldapd/ber.c12 Aug 2018 01:53:10 - @@ -802,7 +802,7 @@ ber_get_writebuf(struct ber *b, void **b * >=0 number of bytes written * -1 on failure and sets errno */ -int +ssize_t ber_write_elements(struct ber *ber, struct ber_element *root) { size_t len; Index: usr.sbin/ldapd/ber.h === RCS file: /cvs/src/usr.sbin/ldapd/ber.h,v retrieving revision 1.7 diff -u -p -r1.7 ber.h --- usr.sbin/ldapd/ber.h3 Aug 2018 01:51:28 - 1.7 +++ usr.sbin/ldapd/ber.h12 Aug 2018 01:53:10 - @@ -124,7 +124,7 @@ int ber_string2oid(const char *, stru struct ber_element *ber_printf_elements(struct ber_element *, char *, ...); int ber_scanf_elements(struct ber_element *, char *, ...); ssize_t ber_get_writebuf(struct ber *, void **); -int ber_write_elements(struct ber *, struct ber_element *); +ssize_t ber_write_elements(struct ber *, struct ber_element *); voidber_set_readbuf(struct ber *, void *, size_t); struct ber_element *ber_read_elements(struct ber *, struct ber_element *); off_t ber_getpos(struct ber_element *); Index: usr.sbin/ldapd/ldape.c === RCS file: /cvs/src/usr.sbin/ldapd/ldape.c,v retrieving revision 1.29 diff -u -p -r1.29 ldape.c --- usr.sbin/ldapd/ldape.c 31 Jul 2018 11:01:00 - 1.29 +++ usr.sbin/ldapd/ldape.c 12 Aug 2018 01:53:10 - @@ -72,7 +72,7 @@ void send_ldap_extended_response(struct conn *conn, int msgid, unsigned int type, long long result_code, const char *extended_oid) { - int rc; + ssize_t rc; struct ber_element *root, *elm; void*buf; @@ -117,7 +117,7 @@ ldap_refer(struct request *req, const ch struct referral *ref; long longresult_code = LDAP_REFERRAL; unsigned int type;
remove some end-of-content code from ber api
As per X.690, "the end-of-contents octets shall be present if the length is encoded as specified in 8.1.3.6, otherwise they shall not be present", i.e. only used with indefinite length encoding. Since we do not support indefinite length encoding, I thought it may make sense to remove some corresponding code from the ber api. The end-of-content type under the universal class has a special meaning; if applications wish to make use of a similar, non-universal content value they are free to do so without assistance from the ber api. It would appear that ldap still wants to use the "eoc" functions, but I believe simply checking for a ber element length of zero should suffice in these cases (i.e. aldap.c). I think the RFC 2411 comment referenced in usr.sbin/ldapd/modify.c below should be referring to RFC 4511, but I will send a separate diff proposal for that. The ldapd and snmpd regression tests pass for me with these changes. I am only looking for feedback and testing at this time. I may be wrong with this approach, so feel free to correct me if you feel this is a misguided change. All such (DER like) restrictions enforced by the ber api will be clarified in an upcoming ber.3 diff. Regards, Rob Index: usr.bin/ldap/aldap.c === RCS file: /cvs/src/usr.bin/ldap/aldap.c,v retrieving revision 1.4 diff -u -p -r1.4 aldap.c --- usr.bin/ldap/aldap.c31 Jul 2018 11:37:18 - 1.4 +++ usr.bin/ldap/aldap.c12 Aug 2018 17:39:52 - @@ -568,7 +568,7 @@ aldap_count_attrs(struct aldap_message * return (-1); for (i = 0, a = msg->body.search.attrs; - a != NULL && ber_get_eoc(a) != 0; + a != NULL && a->be_len != 0; i++, a = a->be_next) ; @@ -616,7 +616,7 @@ aldap_next_attr(struct aldap_message *ms LDAP_DEBUG("attr", msg->body.search.iter); - if (ber_get_eoc(msg->body.search.iter) == 0) + if (msg->body.search.iter->be_len == 0) goto notfound; if (ber_scanf_elements(msg->body.search.iter, "{s(e)}e", &key, &a, &b) @@ -654,7 +654,7 @@ aldap_match_attr(struct aldap_message *m for (a = msg->body.search.attrs;;) { if (a == NULL) goto notfound; - if (ber_get_eoc(a) == 0) + if (a->be_len == 0) goto notfound; if (ber_scanf_elements(a, "{s(e", &descr, &b) != 0) goto fail; Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.18 diff -u -p -r1.18 ber.c --- usr.bin/ldap/ber.c 3 Aug 2018 01:51:28 - 1.18 +++ usr.bin/ldap/ber.c 12 Aug 2018 17:39:52 - @@ -359,28 +359,6 @@ ber_get_null(struct ber_element *elm) return 0; } -struct ber_element * -ber_add_eoc(struct ber_element *prev) -{ - struct ber_element *elm; - - if ((elm = ber_get_element(BER_TYPE_EOC)) == NULL) - return NULL; - - ber_link_elements(prev, elm); - - return elm; -} - -int -ber_get_eoc(struct ber_element *elm) -{ - if (elm->be_encoding != BER_TYPE_EOC) - return -1; - - return 0; -} - size_t ber_oid2ber(struct ber_oid *o, u_int8_t *buf, size_t len) { @@ -629,11 +607,6 @@ ber_printf_elements(struct ber_element * case ')': ber = sub; break; - case '.': - if ((e = ber_add_eoc(ber)) == NULL) - goto fail; - ber = e; - break; default: break; } @@ -737,11 +710,6 @@ ber_scanf_elements(struct ber_element *b goto fail; ret++; break; - case '.': - if (ber->be_encoding != BER_TYPE_EOC) - goto fail; - ret++; - break; case 'p': pos = va_arg(ap, off_t *); *pos = ber_getpos(ber); @@ -990,7 +958,10 @@ ber_dump_element(struct ber *ber, struct ber_write(ber, root->be_val, root->be_len); break; case BER_TYPE_NULL: /* no payload */ + break; case BER_TYPE_EOC: + if (root->be_class == BER_CLASS_UNIVERSAL) + return -1; break; case BER_TYPE_SEQUENCE: case BER_TYPE_SET: @@ -1202,6 +1173,8 @@ ber_read_element(struct ber *ber, struct switch (elm->be_encoding) { case BER_TYPE_EOC: /* End-Of-Content */ + if (class == BER_CLASS_UNIVERSAL) + return -1; break; case BER_TYPE_BOOLEAN:
Re: Remove unused variable in usr.bin/openssl/apps.c
On Thu, Aug 16, 2018 at 06:14:06PM +0800, Nan Xiao wrote: > Hi tech@, > > The `free_out' variable seems redundant, so this patch removes it: > > Index: apps.c > === > RCS file: /cvs/src/usr.bin/openssl/apps.c,v > retrieving revision 1.47 > diff -u -p -r1.47 apps.c > --- apps.c7 Feb 2018 08:57:25 - 1.47 > +++ apps.c16 Aug 2018 09:18:43 - > @@ -2050,11 +2050,9 @@ policies_print(BIO *out, X509_STORE_CTX > { > X509_POLICY_TREE *tree; > int explicit_policy; > - int free_out = 0; > > if (out == NULL) { > out = BIO_new_fp(stderr, BIO_NOCLOSE); > - free_out = 1; > } > tree = X509_STORE_CTX_get0_policy_tree(ctx); > explicit_policy = X509_STORE_CTX_get_explicit_policy(ctx); > > -- > Best Regards > Nan Xiao Committed. Thanks! Rob
Re: xidle: launching program on timeout without active-area
On Tue, Sep 04, 2018 at 01:54:10PM +0200, Claudio Jeker wrote: > On Mon, Sep 03, 2018 at 03:49:46PM +0200, Sebastien Marie wrote: > > ping > > I like it, OK claudio@ but I'm not really a X person. I also like it and it works well for me. ok rob@ with the same caveat. Rob > > On Tue, Aug 14, 2018 at 06:15:08AM +0200, Sebastien Marie wrote: > > > ping > > > > > > On Wed, Jul 25, 2018 at 02:13:49PM +0200, Sebastien Marie wrote: > > > > On Wed, Jul 25, 2018 at 12:55:48PM +0200, Claudio Jeker wrote: > > > > > On Wed, Jul 25, 2018 at 12:27:29PM +0200, Sebastien Marie wrote: > > > > > > On Mon, Jul 16, 2018 at 11:37:41AM +0200, Sebastien Marie wrote: > > > > > > > > > > > > > xidle(1) seems great for such purpose. But I didn't found a way > > > > > > > to just > > > > > > > use timeout and not also an active area (a corner where the > > > > > > > program is > > > > > > > launched if pointer stays inside few seconds). > > > > > > > > > > > > > > The following diff tries to implement a way to disable the active > > > > > > > area > > > > > > > without being too intrusive. > > > > > > > > > > > > > > For that, I used the `delay' parameter ("Specify the number of > > > > > > > seconds > > > > > > > the pointer has to be in the given position before running the > > > > > > > program."), to allow value -1, and make it to discard the event. > > > > > > > > > > > > > > Does it make sens ? Or any proposition to more straighfull > > > > > > > approch ? > > > > > > > > > > > > > > > > > I would love to be able to use xidle without active area but I have > > > > > to say > > > > > that your approach with -1 as delay seems a bit like a hack. Wouldn't > > > > > it > > > > > be better to add a -no option to disable the corners all together? > > > > > Maybe > > > > > even make that the default instead of -nw? > > > > > > > > > > > > > the "-delay -1" approch was taken to avoid too instrusive change that > > > > would clash with upstream (but are we upstream ? I didn't found xidle > > > > under > > > > www.x.org). Anyway, the approch with -no option seems to not be too > > > > intrusive neither and it is better for user point of vue. > > > > > > > > So below a new diff with -no option. > > > > > > > > When used, the -no flag that sets `position' variable to `none'. The > > > > active area window is still created (its avoid to manage a new case > > > > where `xi->win' could be NULL), but the window isn't mapper and no > > > > event asked for EnterWindow. > > > > > > > > Sending USR1 still work as intented. > > > > > > > > I didn't change the default value for the position, but it could be > > > > easily done (one line change in xidle.c and xidle.1). > > > > > > > > -- > > > > Sebastien Marie > > > > > > > > Index: xidle.1 > > > > === > > > > RCS file: /cvs/xenocara/app/xidle/xidle.1,v > > > > retrieving revision 1.4 > > > > diff -u -p -r1.4 xidle.1 > > > > --- xidle.1 9 Nov 2017 19:13:03 - 1.4 > > > > +++ xidle.1 25 Jul 2018 11:54:13 - > > > > @@ -35,7 +35,7 @@ > > > > .Op Fl area Ar pixels > > > > .Op Fl delay Ar secs > > > > .Op Fl display Ar display > > > > -.Op Fl nw | ne | sw | se > > > > +.Op Fl no | nw | ne | sw | se > > > > .Op Fl program Ar path > > > > .Op Fl timeout Ar secs > > > > .Ek > > > > @@ -66,8 +66,8 @@ The default is 2 seconds. > > > > .It Fl display Ar display > > > > This argument allows you to specify the server to connect to; see > > > > .Xr X 7 . > > > > -.It Fl nw | ne | sw | se > > > > -Set the position to one of northwest, northeast, southwest, or > > > > southeast, > > > > +.It Fl no | nw | ne | sw | se > > > > +Set the position to one of none, northwest, northeast, southwest, or > > > > southeast, > > > > respectively. > > > > If no position is specified, > > > > the default is northwest. > > > > @@ -100,7 +100,9 @@ Specify the number of seconds to wait be > > > > .Fl delay > > > > option. > > > > .It Sy position No (class Sy Position ) > > > > -Set the position to one of: "nw", "ne", "sw", or "se"; see > > > > descriptions of the > > > > +Set the position to one of: "no", "nw", "ne", "sw", or "se"; see > > > > descriptions > > > > +of the > > > > +.Fl no , > > > > .Fl nw , > > > > .Fl ne , > > > > .Fl sw , > > > > Index: xidle.c > > > > === > > > > RCS file: /cvs/xenocara/app/xidle/xidle.c,v > > > > retrieving revision 1.5 > > > > diff -u -p -r1.5 xidle.c > > > > --- xidle.c 20 Aug 2017 16:43:25 - 1.5 > > > > +++ xidle.c 25 Jul 2018 11:59:40 - > > > > @@ -53,7 +53,8 @@ enum { > > > > north = 0x01, > > > > south = 0x02, > > > > east = 0x04, > > > > - west = 0x08 > > > > + west = 0x08, > > > > + none = 0x10, > > > > }; > > > > > > > > enum { XIDLE_LOCK = 1, XIDLE_DIE = 2 }; > > > > @@ -84,6 +85,7 @@ static XrmOptionDescRec opts[] = { >
snmpd agentx.c cleanup
This reduces the diff with relayd/agentx.c. Ok? Index: agentx.c === RCS file: /cvs/src/usr.sbin/snmpd/agentx.c,v retrieving revision 1.11 diff -u -p -r1.11 agentx.c --- agentx.c5 Jan 2018 08:13:32 - 1.11 +++ agentx.c12 Feb 2018 19:59:57 - @@ -18,12 +18,8 @@ #include #include #include -#include #include -#include - -#include #include #include #include @@ -341,7 +337,7 @@ snmp_agentx_recv(struct agentx_handle *h if (h->r == NULL) { if ((h->r = snmp_agentx_pdu_alloc()) == NULL) return (NULL); - h->r->datalen = 0; /* XXX -- force this for receive buffers */ + h->r->datalen = 0; /* XXX force this for receive buffers */ } pdu = h->r; @@ -962,7 +958,8 @@ snmp_agentx_do_read_oid(struct agentx_pd } int -snmp_agentx_read_searchrange(struct agentx_pdu *pdu, struct agentx_search_range *sr) +snmp_agentx_read_searchrange(struct agentx_pdu *pdu, +struct agentx_search_range *sr) { if (snmp_agentx_do_read_oid(pdu, &sr->start, &sr->include) == -1 || snmp_agentx_read_oid(pdu, &sr->end) == -1) @@ -1037,7 +1034,7 @@ snmp_oid2string(struct snmp_oid *o, char bzero(buf, len); for (i = 0; i < o->o_n; i++) { - snprintf(str, sizeof(str), "%d", o->o_id[i]); + snprintf(str, sizeof(str), "%u", o->o_id[i]); strlcat(buf, str, len); if (i < (o->o_n - 1)) strlcat(buf, ".", len); @@ -1124,7 +1121,7 @@ snmp_agentx_dump_hdr(struct agentx_hdr * return; } - fprintf(stderr, + fprintf(stderr, "agentx: version %d type %s flags %d reserved %d" " sessionid %d transactid %d packetid %d length %d", hdr->version, snmp_agentx_type2name(hdr->type), hdr->flags,
Re: snmpd agentx.c cleanup
On Mon, Feb 12, 2018 at 03:03:07PM -0500, Rob Pierce wrote: > This reduces the diff with relayd/agentx.c. A little bit closer now. ok? Index: agentx.c === RCS file: /cvs/src/usr.sbin/snmpd/agentx.c,v retrieving revision 1.11 diff -u -p -r1.11 agentx.c --- agentx.c5 Jan 2018 08:13:32 - 1.11 +++ agentx.c12 Feb 2018 22:08:34 - @@ -18,12 +18,8 @@ #include #include #include -#include #include -#include - -#include #include #include #include @@ -217,9 +213,14 @@ snmp_agentx_response(struct agentx_handl { struct agentx_response_data resp; - if (snmp_agentx_read_response(pdu, &resp) == -1) + if (snmp_agentx_read_raw(pdu, &resp, sizeof(resp)) == -1) return (-1); + if (!snmp_agentx_byteorder_native(pdu->hdr)) { + resp.error = snmp_agentx_int16_byteswap(resp.error); + resp.index = snmp_agentx_int16_byteswap(resp.index); + } + h->error = resp.error; if (resp.error != AGENTX_ERR_NONE) return (-1); @@ -227,20 +228,6 @@ snmp_agentx_response(struct agentx_handl return (0); } -int -snmp_agentx_read_response(struct agentx_pdu *pdu, struct agentx_response_data *resp) -{ - if (snmp_agentx_read_raw(pdu, resp, sizeof(*resp)) == -1) - return (-1); - - if (!snmp_agentx_byteorder_native(pdu->hdr)) { - resp->error = snmp_agentx_int16_byteswap(resp->error); - resp->index = snmp_agentx_int16_byteswap(resp->index); - } - - return (0); -} - /* * Read the response PDU for an open operation. */ @@ -341,7 +328,7 @@ snmp_agentx_recv(struct agentx_handle *h if (h->r == NULL) { if ((h->r = snmp_agentx_pdu_alloc()) == NULL) return (NULL); - h->r->datalen = 0; /* XXX -- force this for receive buffers */ + h->r->datalen = 0; /* XXX force this for receive buffers */ } pdu = h->r; @@ -962,7 +949,8 @@ snmp_agentx_do_read_oid(struct agentx_pd } int -snmp_agentx_read_searchrange(struct agentx_pdu *pdu, struct agentx_search_range *sr) +snmp_agentx_read_searchrange(struct agentx_pdu *pdu, +struct agentx_search_range *sr) { if (snmp_agentx_do_read_oid(pdu, &sr->start, &sr->include) == -1 || snmp_agentx_read_oid(pdu, &sr->end) == -1) @@ -1037,7 +1025,7 @@ snmp_oid2string(struct snmp_oid *o, char bzero(buf, len); for (i = 0; i < o->o_n; i++) { - snprintf(str, sizeof(str), "%d", o->o_id[i]); + snprintf(str, sizeof(str), "%u", o->o_id[i]); strlcat(buf, str, len); if (i < (o->o_n - 1)) strlcat(buf, ".", len); @@ -1124,7 +1112,7 @@ snmp_agentx_dump_hdr(struct agentx_hdr * return; } - fprintf(stderr, + fprintf(stderr, "agentx: version %d type %s flags %d reserved %d" " sessionid %d transactid %d packetid %d length %d", hdr->version, snmp_agentx_type2name(hdr->type), hdr->flags, Index: control.c === RCS file: /cvs/src/usr.sbin/snmpd/control.c,v retrieving revision 1.42 diff -u -p -r1.42 control.c --- control.c 21 Apr 2017 13:50:23 - 1.42 +++ control.c 12 Feb 2018 22:08:34 - @@ -537,7 +537,7 @@ control_dispatch_agentx(int fd, short ev struct agentx_varbind_hdrvbhdr; struct ber_element **elm, **iter; - if (snmp_agentx_read_response(pdu, &resp) == -1) { + if (snmp_agentx_read_raw(pdu, &resp, sizeof(resp)) == -1) { msg->sm_error = SNMP_ERROR_GENERR; goto dispatch; } Index: snmp.h === RCS file: /cvs/src/usr.sbin/snmpd/snmp.h,v retrieving revision 1.14 diff -u -p -r1.14 snmp.h --- snmp.h 11 Jun 2015 18:49:09 - 1.14 +++ snmp.h 12 Feb 2018 22:08:34 - @@ -369,7 +369,6 @@ struct agentx_handle * struct agentx_handle * snmp_agentx_fdopen(int, char *, struct snmp_oid *); intsnmp_agentx_response(struct agentx_handle *, struct agentx_pdu *); -intsnmp_agentx_read_response(struct agentx_pdu *, struct agentx_response_data *); intsnmp_agentx_open_response(struct agentx_handle *, struct agentx_pdu *); struct agentx_pdu * snmp_agentx_open_pdu(struct agentx_handle *, char *descr,
Re: update ifstated parser
On Mon, Feb 26, 2018 at 05:10:43PM -0600, Michael Graves wrote: > Hello > > I use ifstated(8) to track the state of the the external interface that is > configured via dhcp and based upon the state, (re)configure a VXLAN > interface. > The ifstated.conf currently looks like > > === > exif="em0" > vxif="vxlan0" > > init-state state_down > > state state_up { > init { > run "ifconfig vlxna0 up" > } > if ( "ifconfig em0 | grep -q inet" every 60 ) > run "sleep 30 && ifconfig vxlan0 tunnel `ifconfig em0 | \ > sed -nre 's/.*inet ([^ ]+).*/\1/p'` \ > `dig +short name-of-remote-device`" > if $exif.link.down > set-state state_down > } > > state state_down { > init { > run "ifconfig vxlan0 down" > } > if $exif.link.up > set-state state_up > } > === > > The problem I ran into is that when I tried to substitute the vxlan0 and em0 > entries with exif and vxif in the 'run' statements no macro expansion > occurred. > This patch allows macro expansion within the 'run' and 'if' statements. > > I appreciate any feedback. > Regards > Index: parse.y > === > RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v > retrieving revision 1.47 > diff -u -p -r1.47 parse.y > --- parse.y 21 Aug 2017 17:38:55 - 1.47 > +++ parse.y 26 Feb 2018 22:47:11 - > @@ -509,9 +509,10 @@ int > yylex(void) > { > u_char buf[8096]; > - u_char *p, *val; > + u_char *p, *p1, *val; > int quotec, next, c; > int token; > + size_t x; > > top: > p = buf; > @@ -575,6 +576,35 @@ top: > } else if (c == '\0') { > yyerror("syntax error"); > return (findeol()); > + } else if (c == '$') { > + p1 = p; > + while (1) { > + if ((c = lgetc(0)) == EOF) > + return (0); > + if (p1 + 1 >= buf + sizeof(buf) - 1) { > + yyerror("string too long"); > + return (findeol()); > + } > + if (isalnum(c) || c == '_') { > + *p1++ = c; > + continue; > + } > + *p1 = '\0'; > + lungetc(c); > + break; > + } > + val = symget(p); > + if (val == NULL) { > + yyerror("macro '%s' not defined", buf); > + return (findeol()); > + } > + x = strlcpy(p,val,(buf-p)); > + if (x >= (buf-p)) { > + yyerror("string too long"); > + return (findeol()); > + } > + p += x; > + continue; > } > if (p + 1 >= buf + sizeof(buf) - 1) { > yyerror("string too long"); Hey Michael, Thank you for your email. I have been playing with your diff and will send you some comments shortly. This might be worth future consideration. For now, I think we should update the man page to clearly state that macro expansion does not take place inside quotes as is currently done in the other man pages. Ok? Index: ifstated.conf.5 === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.conf.5,v retrieving revision 1.11 diff -u -p -r1.11 ifstated.conf.5 --- ifstated.conf.5 3 Mar 2018 02:57:17 - 1.11 +++ ifstated.conf.5 5 Mar 2018 18:39:06 - @@ -55,6 +55,8 @@ instead of using the first state defined Macros can be defined that will later be expanded in context. Macro names must start with a letter, digit, or underscore, and may contain any of those characters. +Macros are not expanded inside quotes. +.Pp Macro names may not be reserved words like, for example, .Ar state or
diff for snmpd agentx.c
This brings snmpd agentx.c closer to relayd agentx.c. In the remaining delta, I am not sure if the pdu context code should be removed from the snmpd version or added to the relayd version. Anyway, this is one step closer. Ok? Rob Index: agentx.c === RCS file: /cvs/src/usr.sbin/snmpd/agentx.c,v retrieving revision 1.12 diff -u -p -r1.12 agentx.c --- agentx.c14 Feb 2018 12:43:07 - 1.12 +++ agentx.c10 Jun 2018 16:59:01 - @@ -18,12 +18,8 @@ #include #include #include -#include #include -#include - -#include #include #include #include @@ -217,9 +213,14 @@ snmp_agentx_response(struct agentx_handl { struct agentx_response_data resp; - if (snmp_agentx_read_response(pdu, &resp) == -1) + if (snmp_agentx_read_raw(pdu, &resp, sizeof(resp)) == -1) return (-1); + if (!snmp_agentx_byteorder_native(pdu->hdr)) { + resp.error = snmp_agentx_int16_byteswap(resp.error); + resp.index = snmp_agentx_int16_byteswap(resp.index); + } + h->error = resp.error; if (resp.error != AGENTX_ERR_NONE) return (-1); @@ -227,20 +228,6 @@ snmp_agentx_response(struct agentx_handl return (0); } -int -snmp_agentx_read_response(struct agentx_pdu *pdu, struct agentx_response_data *resp) -{ - if (snmp_agentx_read_raw(pdu, resp, sizeof(*resp)) == -1) - return (-1); - - if (!snmp_agentx_byteorder_native(pdu->hdr)) { - resp->error = snmp_agentx_int16_byteswap(resp->error); - resp->index = snmp_agentx_int16_byteswap(resp->index); - } - - return (0); -} - /* * Read the response PDU for an open operation. */ @@ -341,7 +328,7 @@ snmp_agentx_recv(struct agentx_handle *h if (h->r == NULL) { if ((h->r = snmp_agentx_pdu_alloc()) == NULL) return (NULL); - h->r->datalen = 0; /* XXX -- force this for receive buffers */ + h->r->datalen = 0; /* XXX force this for receive buffers */ } pdu = h->r; @@ -1038,7 +1025,7 @@ snmp_oid2string(struct snmp_oid *o, char bzero(buf, len); for (i = 0; i < o->o_n; i++) { - snprintf(str, sizeof(str), "%d", o->o_id[i]); + snprintf(str, sizeof(str), "%u", o->o_id[i]); strlcat(buf, str, len); if (i < (o->o_n - 1)) strlcat(buf, ".", len); Index: control.c === RCS file: /cvs/src/usr.sbin/snmpd/control.c,v retrieving revision 1.42 diff -u -p -r1.42 control.c --- control.c 21 Apr 2017 13:50:23 - 1.42 +++ control.c 10 Jun 2018 16:59:01 - @@ -537,7 +537,7 @@ control_dispatch_agentx(int fd, short ev struct agentx_varbind_hdrvbhdr; struct ber_element **elm, **iter; - if (snmp_agentx_read_response(pdu, &resp) == -1) { + if (snmp_agentx_read_raw(pdu, &resp, sizeof(resp)) == -1) { msg->sm_error = SNMP_ERROR_GENERR; goto dispatch; } Index: snmp.h === RCS file: /cvs/src/usr.sbin/snmpd/snmp.h,v retrieving revision 1.14 diff -u -p -r1.14 snmp.h --- snmp.h 11 Jun 2015 18:49:09 - 1.14 +++ snmp.h 10 Jun 2018 16:59:01 - @@ -369,7 +369,6 @@ struct agentx_handle * struct agentx_handle * snmp_agentx_fdopen(int, char *, struct snmp_oid *); intsnmp_agentx_response(struct agentx_handle *, struct agentx_pdu *); -intsnmp_agentx_read_response(struct agentx_pdu *, struct agentx_response_data *); intsnmp_agentx_open_response(struct agentx_handle *, struct agentx_pdu *); struct agentx_pdu * snmp_agentx_open_pdu(struct agentx_handle *, char *descr,
delete unused function in column.c
I stumbled across this while studying calloc() calls in the tree. ok? Index: column.c === RCS file: /cvs/src/usr.bin/column/column.c,v retrieving revision 1.25 diff -u -p -r1.25 column.c --- column.c4 Sep 2016 20:33:36 - 1.25 +++ column.c22 Jun 2018 02:34:24 - @@ -46,7 +46,6 @@ void c_columnate(void); void *ereallocarray(void *, size_t, size_t); -void *ecalloc(size_t, size_t); void input(FILE *); void maketbl(void); void print(void); @@ -339,16 +338,6 @@ void * ereallocarray(void *ptr, size_t nmemb, size_t size) { if ((ptr = reallocarray(ptr, nmemb, size)) == NULL) - err(1, NULL); - return ptr; -} - -void * -ecalloc(size_t nmemb, size_t size) -{ - void *ptr; - - if ((ptr = calloc(nmemb, size)) == NULL) err(1, NULL); return ptr; }
use __func__ in iked util.c log_debug
ok? Index: util.c === RCS file: /cvs/src/sbin/iked/util.c,v retrieving revision 1.35 diff -u -p -r1.35 util.c --- util.c 13 Dec 2017 08:27:06 - 1.35 +++ util.c 22 Jun 2018 12:52:09 - @@ -703,7 +703,7 @@ expand_string(char *label, size_t len, c char *p, *q; if ((tmp = calloc(1, len)) == NULL) { - log_debug("expand_string: calloc"); + log_debug("%s: calloc", __func__); return (-1); } p = q = label; @@ -711,7 +711,7 @@ expand_string(char *label, size_t len, c *q = '\0'; if ((strlcat(tmp, p, len) >= len) || (strlcat(tmp, repl, len) >= len)) { - log_debug("expand_string: string too long"); + log_debug("%s: string too long", __func__); free(tmp); return (-1); } @@ -719,7 +719,7 @@ expand_string(char *label, size_t len, c p = q; } if (strlcat(tmp, p, len) >= len) { - log_debug("expand_string: string too long"); + log_debug("%s: string too long", __func__); free(tmp); return (-1); }
ber.c fix for length calculations
It looks like a BER problem found while testing the new ldap client (with an empty password) was already addressed in snmpd back in 2010 by martinh. In LDAP under a CONTEXT class, 0 corresponds to LDAP_AUTH_SIMPLE. This is currently interpreted as an EOC (end-of-content) and causes a miscalculation of the length which results in unexpected behaviour. Reyk pointed me in the right direction, and after much messing around I came up with a solution which was close to the following snmpd commit. >From the src/usr.sbin/snmpd/ber.c commit log: revision 1.23 date: 2010/09/20 08:30:13; author: martinh; state: Exp; lines: +3 -2; Allow output of null values with a context class. This is used in SNMPv2 to return an error exception value for a varbind result ("noSuchObject[0] IMPLICIT NULL" in rfc1905). However, BER_TYPE_EOC only applies to the UNIVERSAL class, so I think an explicit check against BER_CLASS_UNIVERSAL is a better solution and works across both ldap and snmp. The following diff applies this change to ldap, ldapd, and ypldap, and brings snmpd in line with this approach. Thoughts? Ok? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.1.1.1 diff -u -p -r1.1.1.1 ber.c --- usr.bin/ldap/ber.c 13 Jun 2018 15:45:57 - 1.1.1.1 +++ usr.bin/ldap/ber.c 24 Jun 2018 21:37:39 - @@ -861,7 +861,8 @@ ber_calc_len(struct ber_element *root) size += ber_calc_len(root->be_next); /* This is an empty element, do not use a minimal size */ - if (root->be_type == BER_TYPE_EOC && root->be_len == 0) + if (root->be_class == BER_CLASS_UNIVERSAL && + root->be_type == BER_TYPE_EOC && root->be_len == 0) return (0); return (root->be_len + size); Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.13 diff -u -p -r1.13 ber.c --- usr.sbin/ldapd/ber.c8 Feb 2018 18:02:06 - 1.13 +++ usr.sbin/ldapd/ber.c24 Jun 2018 21:37:39 - @@ -874,7 +874,8 @@ ber_calc_len(struct ber_element *root) size += ber_calc_len(root->be_next); /* This is an empty element, do not use a minimal size */ - if (root->be_type == BER_TYPE_EOC && root->be_len == 0) + if (root->be_class == BER_CLASS_UNIVERSAL && + root->be_type == BER_TYPE_EOC && root->be_len == 0) return (0); return (root->be_len + size); Index: usr.sbin/ypldap/ber.c === RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v retrieving revision 1.13 diff -u -p -r1.13 ber.c --- usr.sbin/ypldap/ber.c 8 Feb 2018 18:02:06 - 1.13 +++ usr.sbin/ypldap/ber.c 24 Jun 2018 21:37:39 - @@ -861,7 +861,8 @@ ber_calc_len(struct ber_element *root) size += ber_calc_len(root->be_next); /* This is an empty element, do not use a minimal size */ - if (root->be_type == BER_TYPE_EOC && root->be_len == 0) + if (root->be_class == BER_CLASS_UNIVERSAL && + root->be_type == BER_TYPE_EOC && root->be_len == 0) return (0); return (root->be_len + size); Index: usr.sbin/snmpd/ber.c === RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v retrieving revision 1.32 diff -u -p -r1.32 ber.c --- usr.sbin/snmpd/ber.c8 Feb 2018 18:02:06 - 1.32 +++ usr.sbin/snmpd/ber.c24 Jun 2018 21:37:39 - @@ -880,7 +880,7 @@ ber_calc_len(struct ber_element *root) size += ber_calc_len(root->be_next); /* This is an empty element, do not use a minimal size */ - if (root->be_class != BER_CLASS_CONTEXT && + if (root->be_class == BER_CLASS_UNIVERSAL && root->be_type == BER_TYPE_EOC && root->be_len == 0) return (0);
sync calloc call in ber.c
This ber.c change has been in ldapd since rev 1.1 and was applied to snmpd back in 2012. The following diff applies the change to the ldap client and ypldap. Ok? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.2 diff -u -p -r1.2 ber.c --- usr.bin/ldap/ber.c 27 Jun 2018 13:22:17 - 1.2 +++ usr.bin/ldap/ber.c 27 Jun 2018 13:55:51 - @@ -269,7 +269,7 @@ ber_add_nstring(struct ber_element *prev struct ber_element *elm; char *string; - if ((string = calloc(1, len)) == NULL) + if ((string = calloc(1, len + 1)) == NULL) return NULL; if ((elm = ber_get_element(BER_TYPE_OCTETSTRING)) == NULL) { free(string); Index: usr.sbin/ypldap/ber.c === RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v retrieving revision 1.14 diff -u -p -r1.14 ber.c --- usr.sbin/ypldap/ber.c 27 Jun 2018 13:22:17 - 1.14 +++ usr.sbin/ypldap/ber.c 27 Jun 2018 13:55:51 - @@ -269,7 +269,7 @@ ber_add_nstring(struct ber_element *prev struct ber_element *elm; char *string; - if ((string = calloc(1, len)) == NULL) + if ((string = calloc(1, len + 1)) == NULL) return NULL; if ((elm = ber_get_element(BER_TYPE_OCTETSTRING)) == NULL) { free(string);
next step in synchronizing ber.c and ber.h
The following diff makes ber.c and ber.h identical across ldap, ldapd, and ypldap, and slightly reduces the diff with snmpd. It covers the evolution of a few scattered enhancements, including: - sync proscription of indefinite length BER encoding - sync consistent presence of ber_free_element() - sync correct parsing of '}' and ')' in ber_scanf_elements() (i.e. a "break" instead of a "continue") - sync use of ber_readbuf in ber_getc() Ok? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.4 diff -u -p -r1.4 ber.c --- usr.bin/ldap/ber.c 27 Jun 2018 20:38:10 - 1.4 +++ usr.bin/ldap/ber.c 27 Jun 2018 22:15:55 - @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b goto fail; ber = parent[level--]; ret++; - continue; + break; default: goto fail; } @@ -822,6 +822,19 @@ ber_read_elements(struct ber *ber, struc } void +ber_free_element(struct ber_element *root) +{ + if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE || + root->be_encoding == BER_TYPE_SET)) + ber_free_elements(root->be_sub); + if (root->be_free && (root->be_encoding == BER_TYPE_OCTETSTRING || + root->be_encoding == BER_TYPE_BITSTRING || + root->be_encoding == BER_TYPE_OBJECT)) + free(root->be_val); + free(root); +} + +void ber_free_elements(struct ber_element *root) { if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE || @@ -1030,6 +1043,12 @@ get_len(struct ber *b, ssize_t *len) return 1; } + if (u == 0x80) { + /* Indefinite length not supported. */ + errno = EINVAL; + return -1; + } + n = u & ~BER_TAG_MORE; if (sizeof(ssize_t) < n) { errno = ERANGE; @@ -1046,12 +1065,6 @@ get_len(struct ber *b, ssize_t *len) if (s < 0) { /* overflow */ errno = ERANGE; - return -1; - } - - if (s == 0) { - /* invalid encoding */ - errno = EINVAL; return -1; } Index: usr.bin/ldap/ber.h === RCS file: /cvs/src/usr.bin/ldap/ber.h,v retrieving revision 1.1.1.1 diff -u -p -r1.1.1.1 ber.h --- usr.bin/ldap/ber.h 13 Jun 2018 15:45:57 - 1.1.1.1 +++ usr.bin/ldap/ber.h 27 Jun 2018 22:15:55 - @@ -119,6 +119,7 @@ ssize_t ber_get_writebuf(struct ber * int ber_write_elements(struct ber *, struct ber_element *); voidber_set_readbuf(struct ber *, void *, size_t); struct ber_element *ber_read_elements(struct ber *, struct ber_element *); +voidber_free_element(struct ber_element *); voidber_free_elements(struct ber_element *); size_t ber_calc_len(struct ber_element *); voidber_set_application(struct ber *, Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.14 diff -u -p -r1.14 ber.c --- usr.sbin/ldapd/ber.c27 Jun 2018 13:22:17 - 1.14 +++ usr.sbin/ldapd/ber.c27 Jun 2018 22:15:55 - @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b goto fail; ber = parent[level--]; ret++; - continue; + break; default: goto fail; } Index: usr.sbin/ypldap/ber.c === RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v retrieving revision 1.16 diff -u -p -r1.16 ber.c --- usr.sbin/ypldap/ber.c 27 Jun 2018 20:38:10 - 1.16 +++ usr.sbin/ypldap/ber.c 27 Jun 2018 22:15:56 - @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b goto fail; ber = parent[level--]; ret++; - continue; + break; default: goto fail; } @@ -822,6 +822,19 @@ ber_read_elements(struct ber *ber, struc } void +ber_free_element(struct ber_element *root) +{ + if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE || + root->be_encoding == BER_TYPE_SET)) + ber_free_elements(root->be_sub); + if (root->be_free && (root->be_encoding == BER_TYPE_OCTETSTRING || + root->be_encoding == BER_TYPE_BITSTRING || + root->be_encoding == BER_TYPE_
synchronize ber.c and ber.h across four consumers
This diff is a final synchronization of ber.c and ber.h. It basically takes (2012) ber additions to snmpd and adds them back to ldap, ldapd, and ypldap instances. See usr.sbin/snmpd/ber.c revision 1.24 commit log for a summary of those changes (e.g. SNMPv2 traps, User-based Security Model, callback for the USM HMAC calculations). The ber code in this diff is not used by ldap, ldapd, or ypldap, but these changes will make maintenance easier going forward. The current (six) consumers of this ber implementation are as follows: - usr.bin/ldap - usr.sbin/ldapctl - usr.sbin/ldapd - usr.sbin/snmpctl - usr.sbin/snmpd - usr.sbin/ypldap Ok? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.6 diff -u -p -r1.6 ber.c --- usr.bin/ldap/ber.c 29 Jun 2018 18:28:41 - 1.6 +++ usr.bin/ldap/ber.c 29 Jun 2018 19:41:59 - @@ -631,10 +631,11 @@ ber_scanf_elements(struct ber_element *b va_list ap; int *d, level = -1; unsigned long *t; - long long *i; + long long *i, l; void**ptr; size_t *len, ret = 0, n = strlen(fmt); char**s; + off_t *pos; struct ber_oid *o; struct ber_element *parent[_MAX_SEQ], **e; @@ -656,6 +657,13 @@ ber_scanf_elements(struct ber_element *b goto fail; ret++; break; + case 'd': + d = va_arg(ap, int *); + if (ber_get_integer(ber, &l) == -1) + goto fail; + *d = l; + ret++; + break; case 'e': e = va_arg(ap, struct ber_element **); *e = ber; @@ -712,6 +720,11 @@ ber_scanf_elements(struct ber_element *b goto fail; ret++; break; + case 'p': + pos = va_arg(ap, off_t *); + *pos = ber_getpos(ber); + ret++; + continue; case '{': case '(': if (ber->be_encoding != BER_TYPE_SEQUENCE && @@ -821,6 +834,12 @@ ber_read_elements(struct ber *ber, struc return root; } +off_t +ber_getpos(struct ber_element *elm) +{ + return elm->be_offs; +} + void ber_free_element(struct ber_element *root) { @@ -893,6 +912,8 @@ ber_dump_element(struct ber *ber, struct uint8_t u; ber_dump_header(ber, root); + if (root->be_cb) + root->be_cb(root->be_cbarg, ber->br_wptr - ber->br_wbuf); switch (root->be_encoding) { case BER_TYPE_BOOLEAN: @@ -1101,6 +1122,7 @@ ber_read_element(struct ber *ber, struct elm->be_type = type; elm->be_len = len; + elm->be_offs = ber->br_offs;/* element position within stream */ elm->be_class = class; if (elm->be_encoding == 0) { @@ -1232,6 +1254,15 @@ ber_set_application(struct ber *b, unsig } void +ber_set_writecallback(struct ber_element *elm, void (*cb)(void *, size_t), +void *arg) +{ + elm->be_cb = cb; + elm->be_cbarg = arg; +} + + +void ber_free(struct ber *b) { free(b->br_wbuf); @@ -1258,5 +1289,33 @@ ber_read(struct ber *ber, void *buf, siz b += r; remain -= r; } - return (b - (u_char *)buf); + r = b - (u_char *)buf; + ber->br_offs += r; + return r; +} + +int +ber_oid_cmp(struct ber_oid *a, struct ber_oid *b) +{ + size_t i; + for (i = 0; i < BER_MAX_OID_LEN; i++) { + if (a->bo_id[i] != 0) { + if (a->bo_id[i] == b->bo_id[i]) + continue; + else if (a->bo_id[i] < b->bo_id[i]) { + /* b is a successor of a */ + return (1); + } else { + /* b is a predecessor of a */ + return (-1); + } + } else if (b->bo_id[i] != 0) { + /* b is larger, but a child of a */ + return (2); + } else + break; + } + + /* b and a are identical */ + return (0); } Index: usr.bin/ldap/ber.h === RCS file: /cvs/src/usr.bin/ldap/ber.h,v retrieving revision 1.3 diff -u -p -r1.3 ber.h --- usr.bin/ldap/ber.h 29 Jun 2018 18:28:41 - 1.3 +++ usr.bin/ldap/ber.h 29 Jun 2018 19:41:59 - @@
call ber_read() from ber_getc() in ldap, ldapd, and ypldap
I recently committed a piece of BER code synchronizing in the wrong direction (i.e. from the ldap instances to the snmpd instance). sthen@ noticed a break in SNMPv3 authentication and reverted that part of the change. Thanks Stuart! I just fixed some typos in the snmpd regression test which prevented me from noticing the problem. I am also the author of those typos... arg. I believe the diff below synchronizes this piece of BER code in the right direction, from the snmpd instance to the ldap instances. I have done some testing against ldap and ldapd, but not ypldap. If anyone out there could perform further testing on and/or review the change to make sure it doesn't break anything that would be much appreciated. Thanks! Rob Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.6 diff -u -p -r1.6 ber.c --- usr.bin/ldap/ber.c 29 Jun 2018 18:28:41 - 1.6 +++ usr.bin/ldap/ber.c 30 Jun 2018 17:50:06 - @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) static ssize_t ber_getc(struct ber *b, u_char *c) { - return ber_readbuf(b, c, 1); + return ber_read(b, c, 1); } static ssize_t Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.16 diff -u -p -r1.16 ber.c --- usr.sbin/ldapd/ber.c29 Jun 2018 18:28:42 - 1.16 +++ usr.sbin/ldapd/ber.c30 Jun 2018 17:50:06 - @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) static ssize_t ber_getc(struct ber *b, u_char *c) { - return ber_readbuf(b, c, 1); + return ber_read(b, c, 1); } static ssize_t Index: usr.sbin/ypldap/ber.c === RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v retrieving revision 1.18 diff -u -p -r1.18 ber.c --- usr.sbin/ypldap/ber.c 29 Jun 2018 18:28:42 - 1.18 +++ usr.sbin/ypldap/ber.c 30 Jun 2018 17:50:06 - @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) static ssize_t ber_getc(struct ber *b, u_char *c) { - return ber_readbuf(b, c, 1); + return ber_read(b, c, 1); } static ssize_t
Re: [PATCH] fix typo in if_aue.c
Ok rob@ > From: "Kevin Lo" > To: "tech" > Sent: Monday, July 2, 2018 10:23:39 AM > Subject: [PATCH] fix typo in if_aue.c > Hi, > I've just noticed a little typo in the if_aue.c (s/read/write). > The diff is below. > Index: sys/dev/usb/if_aue.c > === > RCS file: /cvs/src/sys/dev/usb/if_aue.c,v > retrieving revision 1.106 > diff -u -p -u -p -r1.106 if_aue.c > --- sys/dev/usb/if_aue.c 22 Jan 2017 10:17:39 - 1.106 > +++ sys/dev/usb/if_aue.c 2 Jul 2018 14:19:57 - > @@ -509,7 +509,7 @@ aue_miibus_writereg(struct device *dev, > } > if (i == AUE_TIMEOUT) { > - printf("%s: MII read timed out\n", > + printf("%s: MII write timed out\n", > sc->aue_dev.dv_xname); > } > aue_unlock_mii(sc);
Re: [PATCH] mos: nuke unused variable
Ok rob@ > From: "Kevin Lo" > To: "tech" > Sent: Monday, July 2, 2018 10:22:58 AM > Subject: [PATCH] mos: nuke unused variable > Ok ? > Index: sys/dev/usb/if_mos.c > === > RCS file: /cvs/src/sys/dev/usb/if_mos.c,v > retrieving revision 1.38 > diff -u -p -u -p -r1.38 if_mos.c > --- sys/dev/usb/if_mos.c 22 Jan 2017 10:17:39 - 1.38 > +++ sys/dev/usb/if_mos.c 2 Jul 2018 14:09:39 - > @@ -376,15 +376,12 @@ int > mos_miibus_readreg(struct device *dev, int phy, int reg) > { > struct mos_softc *sc = (void *)dev; > - uWord val; > int i,res; > if (usbd_is_dying(sc->mos_udev)) { > DPRINTF(("mos: dying\n")); > return (0); > } > - > - USETW(val, 0); > mos_lock_mii(sc);
Re: call ber_read() from ber_getc() in ldap, ldapd, and ypldap
On Sat, Jun 30, 2018 at 02:04:16PM -0400, Rob Pierce wrote: > I recently committed a piece of BER code synchronizing in the wrong direction > (i.e. from the ldap instances to the snmpd instance). sthen@ noticed a break > in SNMPv3 authentication and reverted that part of the change. Thanks Stuart! > > I just fixed some typos in the snmpd regression test which prevented me from > noticing the problem. I am also the author of those typos... arg. > > I believe the diff below synchronizes this piece of BER code in the right > direction, from the snmpd instance to the ldap instances. I have done some > testing against ldap and ldapd, but not ypldap. If anyone out there could > perform further testing on and/or review the change to make sure it doesn't > break anything that would be much appreciated. > > Thanks! > > Rob > > Index: usr.bin/ldap/ber.c > === > RCS file: /cvs/src/usr.bin/ldap/ber.c,v > retrieving revision 1.6 > diff -u -p -r1.6 ber.c > --- usr.bin/ldap/ber.c29 Jun 2018 18:28:41 - 1.6 > +++ usr.bin/ldap/ber.c30 Jun 2018 17:50:06 - > @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) > static ssize_t > ber_getc(struct ber *b, u_char *c) > { > - return ber_readbuf(b, c, 1); > + return ber_read(b, c, 1); > } > > static ssize_t > > Index: usr.sbin/ldapd/ber.c > === > RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v > retrieving revision 1.16 > diff -u -p -r1.16 ber.c > --- usr.sbin/ldapd/ber.c 29 Jun 2018 18:28:42 - 1.16 > +++ usr.sbin/ldapd/ber.c 30 Jun 2018 17:50:06 - > @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) > static ssize_t > ber_getc(struct ber *b, u_char *c) > { > - return ber_readbuf(b, c, 1); > + return ber_read(b, c, 1); > } > > static ssize_t > > Index: usr.sbin/ypldap/ber.c > === > RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v > retrieving revision 1.18 > diff -u -p -r1.18 ber.c > --- usr.sbin/ypldap/ber.c 29 Jun 2018 18:28:42 - 1.18 > +++ usr.sbin/ypldap/ber.c 30 Jun 2018 17:50:06 - > @@ -1240,7 +1240,7 @@ ber_free(struct ber *b) > static ssize_t > ber_getc(struct ber *b, u_char *c) > { > - return ber_readbuf(b, c, 1); > + return ber_read(b, c, 1); > } > > static ssize_t After further review and testing I am now looking for ok's on this. Ok?
Re: call ber_read() from ber_getc() in ldap, ldapd, and ypldap
On Tue, Jul 03, 2018 at 09:25:06PM +0100, Stuart Henderson wrote: > On 2018/07/03 22:17, Claudio Jeker wrote: > > I have a hard time to understand why this is needed in snmpd. > > For single char reads ber_readbuf(b, c, 1) and ber_read(b, c, 1) should do > > exaclty the same. At least in the old code. I see that snmpd added br_offs > > in a way that causes this breakage. The update of br_offs is handled in > > the wrong place in my opinion. > > > > I would prefer something like the appended diff. > > This also removes the r == 0 check in ber_read since ber_readbuf cannot > > return 0. Can someone check against the snmpd usecase that failed? > > Yes this fixes it, thanks. The use case that fails is "configure SNMPv3, > try to use it". > e.g. > > user "username" authkey "authkey" enc aes enckey "privkey" > > $ snmpwalk -v3 -l authPriv -a SHA -A authkey -u username -x AES -X privkey > hostname > > > -- > > :wq Claudio > > > > Index: ber.c > > === > > RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v > > retrieving revision 1.37 > > diff -u -p -r1.37 ber.c > > --- ber.c 1 Jul 2018 20:03:48 - 1.37 > > +++ ber.c 3 Jul 2018 20:16:13 - > > @@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct > > static ssize_t > > ber_readbuf(struct ber *b, void *buf, size_t nbytes) > > { > > - size_t sz; > > - size_t len; > > + size_t sz, len; > > > > if (b->br_rbuf == NULL) > > return -1; > > @@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si > > > > bcopy(b->br_rptr, buf, len); > > b->br_rptr += len; > > + b->br_offs += len; > > > > return (len); > > } > > @@ -1271,7 +1271,7 @@ ber_free(struct ber *b) > > static ssize_t > > ber_getc(struct ber *b, u_char *c) > > { > > - return ber_read(b, c, 1); > > + return ber_readbuf(b, c, 1); > > } > > > > static ssize_t > > @@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz > > r = ber_readbuf(ber, b, remain); > > if (r == -1) > > return -1; > > - if (r == 0) > > - return (b - (u_char *)buf); > > b += r; > > remain -= r; > > } > > - r = b - (u_char *)buf; > > - ber->br_offs += r; > > - return r; > > + return (b - (u_char *)buf); > > } > > > > int > > Your diff also passes the snmpd regression tests here, including that specific use case. LDAP also appears happy with this change. How about this larger diff (based on yours) for all ber.c instances? Index: usr.bin/ldap/ber.c === RCS file: /cvs/src/usr.bin/ldap/ber.c,v retrieving revision 1.8 diff -u -p -r1.8 ber.c --- usr.bin/ldap/ber.c 3 Jul 2018 18:49:10 - 1.8 +++ usr.bin/ldap/ber.c 3 Jul 2018 21:33:26 - @@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes) { - size_t sz; - size_t len; + size_t sz, len; if (b->br_rbuf == NULL) return -1; @@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si bcopy(b->br_rptr, buf, len); b->br_rptr += len; + b->br_offs += len; return (len); } @@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz r = ber_readbuf(ber, b, remain); if (r == -1) return -1; - if (r == 0) - return (b - (u_char *)buf); b += r; remain -= r; } - r = b - (u_char *)buf; - ber->br_offs += r; - return r; + return (b - (u_char *)buf); } int Index: usr.sbin/ldapd/ber.c === RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v retrieving revision 1.18 diff -u -p -r1.18 ber.c --- usr.sbin/ldapd/ber.c3 Jul 2018 18:49:10 - 1.18 +++ usr.sbin/ldapd/ber.c3 Jul 2018 21:33:31 - @@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes) { - size_t sz; - size_t len; + size_t sz, len; if (b->br_rbuf == NULL) return -1; @@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si bcopy(b->br_rptr, buf, len); b->br_rptr += len; + b->br_offs += len; return (len); } @@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz r = ber_readbuf(ber, b, remain); if (r == -1) return -1; - if (r == 0) - return (b - (u_char *)buf); b += r; remain -= r; } - r = b - (u_char *)buf; - ber->br_offs += r; - return r; + return (b - (u_char *)buf); } int Index:
avoid vfprintf NULL errors in ldape.c log_debug()
Running the current ldapd regression tests result in the following (repeated) errors in my /var/log/messages: ... ldapd: vfprintf %s NULL in "current bind dn = %s " This is because regress/usr.sbin/ldapd/run-tests.pl is performing unnecessary unbinds in END { }. Though the regression test should probably be fixed, the following diff ensures that log_debug is not called with a NULL argument. Does this make sense? Index: ldape.c === RCS file: /cvs/src/usr.sbin/ldapd/ldape.c,v retrieving revision 1.27 diff -u -p -r1.27 ldape.c --- ldape.c 15 May 2018 11:19:21 - 1.27 +++ ldape.c 3 Jul 2018 23:32:27 - @@ -229,7 +229,8 @@ ldap_abandon(struct request *req) int ldap_unbind(struct request *req) { - log_debug("current bind dn = %s", req->conn->binddn); + log_debug("current bind dn = %s", + req->conn->binddn == NULL ? "" : req->conn->binddn); conn_disconnect(req->conn); request_free(req); return -1; /* don't send any response */
Re: ber.c: simplify ber_read()
On Sun, Jul 08, 2018 at 06:38:16PM +0200, Jeremie Courreges-Anglas wrote: > > When looking at the recent changes in this file I noticed that the > presence of both ber_read() and ber_readbuf() was due to an incomplete > cleanup from my part. > > revision 1.32 > date: 2018/02/08 18:02:06; author: jca; state: Exp; lines: +6 -22; > commitid: YNQMco5lCS8YEifQ; > Kill ber.c support for direct fd read/writes > > This mechanism is already unused and annotated with lots of XXX's, no > need to keep it around. ok claudio@ > > I think the recent changes would have been easier if the code had been > further simplified. Here's a shot at it: > - stop looping in ber_read(), there is no fd read to handle any more so > calling ber_readbuf() once is enough > - amend the condition which decides whether to return -1/ECANCELED: > - it makes little sense to pass a buffer length size of zero to > ber_read(), but then we should probably return 0 and not -1/ECANCELED > - I don't think we should perform partial reads: either the caller > function has the data it asked for, or nothing. Allowing partial > reads means that we're putting the burden of handling an incomplete > buffer on the caller. > - inline what remains of ber_readbuf() in ber_read() > > regress tests for ldapd and snmpd pass, it would be cool to have test > reports from people who actually use those programs (ldap, ldapd, snmpd > and ypldap). > > Looking for reviews and oks. The diff is not that long but I can split > it in smaller parts if it helps. Thank you for this. Definitely would have make the recent changes a bit easier. The code makes more sense to me now and works as expected. ok rob@ > Index: usr.bin/ldap/ber.c > === > RCS file: /d/cvs/src/usr.bin/ldap/ber.c,v > retrieving revision 1.11 > diff -u -p -r1.11 ber.c > --- usr.bin/ldap/ber.c4 Jul 2018 15:21:24 - 1.11 > +++ usr.bin/ldap/ber.c6 Jul 2018 11:50:24 - > @@ -31,8 +31,6 @@ > > #include "ber.h" > > -#define MINIMUM(a, b)(((a) < (b)) ? (a) : (b)) > - > #define BER_TYPE_CONSTRUCTED 0x20/* otherwise primitive */ > #define BER_TYPE_SINGLE_MAX 30 > #define BER_TAG_MASK 0x1f > @@ -48,7 +46,6 @@ static ssize_t get_id(struct ber *b, uns > int *cstruct); > static ssize_t get_len(struct ber *b, ssize_t *len); > static ssize_t ber_read_element(struct ber *ber, struct ber_element > *elm); > -static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes); > static ssize_t ber_getc(struct ber *b, u_char *c); > static ssize_t ber_read(struct ber *ber, void *buf, size_t len); > > @@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct > return totlen; > } > > -static ssize_t > -ber_readbuf(struct ber *b, void *buf, size_t nbytes) > -{ > - size_t sz, len; > - > - if (b->br_rbuf == NULL) > - return -1; > - > - sz = b->br_rend - b->br_rptr; > - len = MINIMUM(nbytes, sz); > - if (len == 0) { > - errno = ECANCELED; > - return (-1);/* end of buffer and parser wants more data */ > - } > - > - bcopy(b->br_rptr, buf, len); > - b->br_rptr += len; > - b->br_offs += len; > - > - return (len); > -} > - > void > ber_set_readbuf(struct ber *b, void *buf, size_t len) > { > @@ -1269,23 +1244,28 @@ ber_free(struct ber *b) > static ssize_t > ber_getc(struct ber *b, u_char *c) > { > - return ber_readbuf(b, c, 1); > + return ber_read(b, c, 1); > } > > static ssize_t > ber_read(struct ber *ber, void *buf, size_t len) > { > - u_char *b = buf; > - ssize_t r, remain = len; > + size_t sz; > > - while (remain > 0) { > - r = ber_readbuf(ber, b, remain); > - if (r == -1) > - return -1; > - b += r; > - remain -= r; > + if (ber->br_rbuf == NULL) > + return -1; > + > + sz = ber->br_rend - ber->br_rptr; > + if (len > sz) { > + errno = ECANCELED; > + return -1; /* parser wants more data than available */ > } > - return (b - (u_char *)buf); > + > + bcopy(ber->br_rptr, buf, len); > + ber->br_rptr += len; > + ber->br_offs += len; > + > + return len; > } > > int > Index: usr.sbin/ldapd/ber.c > === > RCS file: /d/cvs/src/usr.sbin/ldapd/ber.c,v > retrieving revision 1.21 > diff -u -p -r1.21 ber.c > --- usr.sbin/ldapd/ber.c 4 Jul 2018 15:21:24 - 1.21 > +++ usr.sbin/ldapd/ber.c 6 Jul 2018 11:47:55 - > @@ -31,8 +31,6 @@ > > #include "ber.h" > > -#define MINIMUM(a, b)(((a) < (b)) ? (a) : (b)) > - > #define BER_TYPE_CONSTRUCTED 0x20/* otherwise primitive */ > #define BER_TYPE_SINGLE_MAX 30 > #define BER_TAG_MASK 0x1f > @@ -48,7 +46,6 @@ static ssize_t
prefer memset() over bzero() for ber.c in snmpd and ypldap
A similar change was done to ber.c in ldapd by @mmcc. I just copied those changes for snmpd and ypldap. No binary change. Regards, Index: snmpd/ber.c === RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v retrieving revision 1.30 diff -u -p -r1.30 ber.c --- snmpd/ber.c 5 Dec 2015 06:42:18 - 1.30 +++ snmpd/ber.c 4 Mar 2016 21:34:32 - @@ -420,7 +420,7 @@ ber_string2oid(const char *oidstr, struc if (strlcpy(str, oidstr, sizeof(str)) >= sizeof(str)) return (-1); - bzero(o, sizeof(*o)); + memset(o, 0, sizeof(*o)); /* Parse OID strings in the common forms n.n.n, n_n_n_n, or n-n-n */ for (p = sp = str; p != NULL; sp = p) { @@ -505,7 +505,7 @@ ber_get_oid(struct ber_element *elm, str if (!buf[i]) return (-1); - bzero(o, sizeof(*o)); + memset(o, 0, sizeof(*o)); o->bo_id[j++] = buf[i] / 40; o->bo_id[j++] = buf[i++] % 40; for (; i < len && j < BER_MAX_OID_LEN; i++) { @@ -639,7 +639,7 @@ ber_scanf_elements(struct ber_element *b struct ber_oid *o; struct ber_element *parent[_MAX_SEQ], **e; - bzero(parent, sizeof(struct ber_element *) * _MAX_SEQ); + memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ); va_start(ap, fmt); while (*fmt) { Index: ypldap/ber.c === RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v retrieving revision 1.10 diff -u -p -r1.10 ber.c --- ypldap/ber.c5 Dec 2015 19:10:19 - 1.10 +++ ypldap/ber.c4 Mar 2016 21:34:33 - @@ -420,7 +420,7 @@ ber_string2oid(const char *oidstr, struc if (strlcpy(str, oidstr, sizeof(str)) >= sizeof(str)) return (-1); - bzero(o, sizeof(*o)); + memset(o, 0, sizeof(*o)); /* Parse OID strings in the common forms n.n.n, n_n_n_n, or n-n-n */ for (p = sp = str; p != NULL; sp = p) { @@ -505,7 +505,7 @@ ber_get_oid(struct ber_element *elm, str if (!buf[i]) return (-1); - bzero(o, sizeof(*o)); + memset(o, 0, sizeof(*o)); o->bo_id[j++] = buf[i] / 40; o->bo_id[j++] = buf[i++] % 40; for (; i < len && j < BER_MAX_OID_LEN; i++) { @@ -638,7 +638,7 @@ ber_scanf_elements(struct ber_element *b struct ber_oid *o; struct ber_element *parent[_MAX_SEQ], **e; - bzero(parent, sizeof(struct ber_element *) * _MAX_SEQ); + memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ); va_start(ap, fmt); while (*fmt) {
Re: rcctl ls faulty -> failed
> From: "Antoine Jacoutot" > To: "Ian Darwin" > Cc: "tech" > Sent: Tuesday, March 29, 2016 10:59:54 AM > Subject: Re: rcctl ls faulty -> failed > On Tue, Mar 29, 2016 at 10:48:17AM -0400, Ian Darwin wrote: > > On Tue, Mar 29, 2016 at 03:29:27PM +0200, Antoine Jacoutot wrote: > > > Hi. > > > We'd like to rename the 'faulty' listing to 'failed'. > > > i.e. rcctl ls failed > > > Index: etc/daily > > > === > > > RCS file: /cvs/src/etc/daily,v > > > retrieving revision 1.85 > > > diff -u -p -u -p -r1.85 daily > > > --- etc/daily 28 Jan 2016 15:45:34 - 1.85 > > > +++ etc/daily 29 Mar 2016 13:25:59 - > > > @@ -127,7 +127,7 @@ while [ "X$ROOTBACKUP" = X1 ]; do > > > done > > > next_part "Services that should run but don't:" > > While you're there, can you please change "should run but don't" to > > "should be running but aren't" ? The current wording is awkward, > > and also implies that they don't run (ie. they fail to start) > > when in fact they may have been running but been shut down > > manually, or failed. Language should be precise as well as concise. > Sure. > -- > Antoine Contractions aren't necessary. http://courses.cs.vt.edu/cs3604/support/Writing/writing.caveats.html
pstat diff fixed segmentation fault
Running pstat with either [-M core] or [-N system] along with the -T option results in a segmentation fault. Make sure kvm access is initialized prior to running kvm_read() when -T is specified. This issue appears to have been introduced when setgid kmem support was removed in revision 1.99. Rob Index: pstat.c === RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v retrieving revision 1.101 diff -u -p -r1.101 pstat.c --- pstat.c 11 Dec 2015 11:53:52 - 1.101 +++ pstat.c 11 Apr 2016 18:22:35 - @@ -193,7 +193,7 @@ main(int argc, char *argv[]) need_nlist = vnodeflag || dformat; if (nlistf != NULL || memf != NULL) { - if (fileflag) + if (fileflag || totalflag) need_nlist = 1; }
pledge pstat
Hoist sysct and kvm calls, and pledge stdio, rpath, vminfo. Rob Index: pstat.c === RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v retrieving revision 1.102 diff -u -p -r1.102 pstat.c --- pstat.c 12 Apr 2016 16:53:42 - 1.102 +++ pstat.c 12 Apr 2016 20:20:39 - @@ -82,6 +82,8 @@ struct nlist vnodenl[] = { { NULL } }; +struct itty *globalitp; +struct kinfo_file *kf; struct nlist *globalnl; struct e_vnode { @@ -89,6 +91,10 @@ struct e_vnode { struct vnode vnode; }; +intnumvnodes; +intmaxfile; +intnfile; +intntty; intusenumflag; inttotalflag; intkflag; @@ -111,15 +117,17 @@ kvm_t *kd = NULL; } void filemode(void); +void filemodeprep(void); struct mount * getmnt(struct mount *); struct e_vnode * - kinfo_vnodes(int *); + kinfo_vnodes(void); void mount_print(struct mount *); void nfs_header(void); intnfs_print(struct vnode *); void swapmode(void); void ttymode(void); +void ttymodeprep(void); void ttyprt(struct itty *); void tty2itty(struct tty *tp, struct itty *itp); void ufs_header(void); @@ -130,6 +138,7 @@ voidusage(void); void vnode_header(void); void vnode_print(struct vnode *, struct vnode *); void vnodemode(void); +void vnodemodeprep(void); inthideroot; @@ -202,6 +211,25 @@ main(int argc, char *argv[]) O_RDONLY | (need_nlist ? 0 : KVM_NO_FILES), buf)) == 0) errx(1, "kvm_openfiles: %s", buf); + if (need_nlist) + if (kvm_nlist(kd, vnodenl) == -1) + errx(1, "kvm_nlist: %s", kvm_geterr(kd)); + + if (!(fileflag | vnodeflag | ttyflag | swapflag | totalflag || dformat)) + usage(); + + if(!dformat) { + if (fileflag || totalflag) + filemodeprep(); + if (vnodeflag || totalflag) + vnodemodeprep(); + if (ttyflag) + ttymodeprep(); + } + + if (pledge("stdio rpath vminfo", NULL) == -1) + err(1, "pledge"); + if (dformat) { struct nlist *nl; int longformat = 0, stringformat = 0, error = 0, n; @@ -314,12 +342,6 @@ main(int argc, char *argv[]) exit(error); } - if (need_nlist) - if (kvm_nlist(kd, vnodenl) == -1) - errx(1, "kvm_nlist: %s", kvm_geterr(kd)); - - if (!(fileflag | vnodeflag | ttyflag | swapflag | totalflag || dformat)) - usage(); if (fileflag || totalflag) filemode(); if (vnodeflag || totalflag) @@ -337,11 +359,10 @@ vnodemode(void) struct e_vnode *e_vnodebase, *endvnode, *evp; struct vnode *vp; struct mount *maddr, *mp = NULL; - int numvnodes; globalnl = vnodenl; - e_vnodebase = kinfo_vnodes(&numvnodes); + e_vnodebase = kinfo_vnodes(); if (totalflag) { (void)printf("%7d vnodes\n", numvnodes); return; @@ -398,6 +419,21 @@ vnodemode(void) } void +vnodemodeprep(void) +{ + int mib[2]; + size_t num; + + if (kd == 0) { + mib[0] = CTL_KERN; + mib[1] = KERN_NUMVNODES; + num = sizeof(numvnodes); + if (sysctl(mib, 2, &numvnodes, &num, NULL, 0) < 0) + err(1, "sysctl(KERN_NUMVNODES) failed"); + } +} + +void vnode_header(void) { (void)printf("%*s TYP VFLAG USE HOLD", 2 * (int)sizeof(long), "ADDR"); @@ -790,24 +826,17 @@ mount_print(struct mount *mp) * simulate what a running kernel does in kinfo_vnode */ struct e_vnode * -kinfo_vnodes(int *avnodes) +kinfo_vnodes(void) { struct mntlist kvm_mountlist; struct mount *mp, mount; struct vnode *vp, vnode; char *vbuf, *evbuf, *bp; - int mib[2], numvnodes; + int mib[2]; size_t num; - if (kd == 0) { - mib[0] = CTL_KERN; - mib[1] = KERN_NUMVNODES; - num = sizeof(numvnodes); - if (sysctl(mib, 2, &numvnodes, &num, NULL, 0) < 0) - err(1, "sysctl(KERN_NUMVNODES) failed"); - } else + if (kd != 0) KGET(V_NUMV, numvnodes); - *avnodes = numvnodes; if (totalflag) return NULL; if ((vbuf = calloc(numvnodes + 20, @@ -835,7 +864,7 @@ kinfo_vnodes(int *avnodes) num++; } } - *avnodes = num; + numvnodes = num; return ((struct e_vnode *)vbuf); } @@ -864,32 +893,18 @@ ttymode(void) { struct ttylist_head tty_head; struct tty *tp, tty; - int mib[3], ntty, i; - struct itty itty, *itp; + int mib[3], i; + struct itty itty; size_t nlen; - i
Re: pledge pstat
From: "Mike Belopuhov" To: "Rob Pierce" Cc: "tech" Sent: Tuesday, April 12, 2016 6:05:19 PM Subject: Re: pledge pstat BQ_BEGIN On 12 April 2016 at 22:25, Rob Pierce wrote: > Hoist sysct and kvm calls, and pledge stdio, rpath, vminfo. > > Rob > Please make sure that "pstat -d" still works. It's about the most useful thing it can do. BQ_END Yes, of course. I have received some feedback and will be submitting an updated diff shortly.
Re: pledge pstat
On Tue, Apr 12, 2016 at 04:25:01PM -0400, Rob Pierce wrote: > Hoist sysct and kvm calls, and pledge stdio, rpath, vminfo. > > Rob Update diff based on comments from tb@: Changes include the following: - kd is a pointer, so compare with NULL (not 0). - simply nlistf/memf condition statment in filemode() now that kd is global Here is the revised diff: Index: pstat.c === RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v retrieving revision 1.102 diff -u -p -r1.102 pstat.c --- pstat.c 12 Apr 2016 16:53:42 - 1.102 +++ pstat.c 13 Apr 2016 15:06:48 - @@ -82,6 +82,8 @@ struct nlist vnodenl[] = { { NULL } }; +struct itty *globalitp; +struct kinfo_file *kf; struct nlist *globalnl; struct e_vnode { @@ -89,6 +91,10 @@ struct e_vnode { struct vnode vnode; }; +intnumvnodes; +intmaxfile; +intnfile; +intntty; intusenumflag; inttotalflag; intkflag; @@ -111,15 +117,17 @@ kvm_t *kd = NULL; } void filemode(void); +void filemodeprep(void); struct mount * getmnt(struct mount *); struct e_vnode * - kinfo_vnodes(int *); + kinfo_vnodes(void); void mount_print(struct mount *); void nfs_header(void); intnfs_print(struct vnode *); void swapmode(void); void ttymode(void); +void ttymodeprep(void); void ttyprt(struct itty *); void tty2itty(struct tty *tp, struct itty *itp); void ufs_header(void); @@ -130,6 +138,7 @@ voidusage(void); void vnode_header(void); void vnode_print(struct vnode *, struct vnode *); void vnodemode(void); +void vnodemodeprep(void); inthideroot; @@ -202,6 +211,25 @@ main(int argc, char *argv[]) O_RDONLY | (need_nlist ? 0 : KVM_NO_FILES), buf)) == 0) errx(1, "kvm_openfiles: %s", buf); + if (need_nlist) + if (kvm_nlist(kd, vnodenl) == -1) + errx(1, "kvm_nlist: %s", kvm_geterr(kd)); + + if (!(fileflag | vnodeflag | ttyflag | swapflag | totalflag || dformat)) + usage(); + + if(!dformat) { + if (fileflag || totalflag) + filemodeprep(); + if (vnodeflag || totalflag) + vnodemodeprep(); + if (ttyflag) + ttymodeprep(); + } + + if (pledge("stdio rpath vminfo", NULL) == -1) + err(1, "pledge"); + if (dformat) { struct nlist *nl; int longformat = 0, stringformat = 0, error = 0, n; @@ -314,12 +342,6 @@ main(int argc, char *argv[]) exit(error); } - if (need_nlist) - if (kvm_nlist(kd, vnodenl) == -1) - errx(1, "kvm_nlist: %s", kvm_geterr(kd)); - - if (!(fileflag | vnodeflag | ttyflag | swapflag | totalflag || dformat)) - usage(); if (fileflag || totalflag) filemode(); if (vnodeflag || totalflag) @@ -337,11 +359,10 @@ vnodemode(void) struct e_vnode *e_vnodebase, *endvnode, *evp; struct vnode *vp; struct mount *maddr, *mp = NULL; - int numvnodes; globalnl = vnodenl; - e_vnodebase = kinfo_vnodes(&numvnodes); + e_vnodebase = kinfo_vnodes(); if (totalflag) { (void)printf("%7d vnodes\n", numvnodes); return; @@ -398,6 +419,21 @@ vnodemode(void) } void +vnodemodeprep(void) +{ + int mib[2]; + size_t num; + + if (kd == NULL) { + mib[0] = CTL_KERN; + mib[1] = KERN_NUMVNODES; + num = sizeof(numvnodes); + if (sysctl(mib, 2, &numvnodes, &num, NULL, 0) < 0) + err(1, "sysctl(KERN_NUMVNODES) failed"); + } +} + +void vnode_header(void) { (void)printf("%*s TYP VFLAG USE HOLD", 2 * (int)sizeof(long), "ADDR"); @@ -790,24 +826,17 @@ mount_print(struct mount *mp) * simulate what a running kernel does in kinfo_vnode */ struct e_vnode * -kinfo_vnodes(int *avnodes) +kinfo_vnodes(void) { struct mntlist kvm_mountlist; struct mount *mp, mount; struct vnode *vp, vnode; char *vbuf, *evbuf, *bp; - int mib[2], numvnodes; + int mib[2]; size_t num; - if (kd == 0) { - mib[0] = CTL_KERN; - mib[1] = KERN_NUMVNODES; - num = sizeof(numvnodes); - if (sysctl(mib, 2, &numvnodes, &num, NULL, 0) < 0) - err(1, "sysctl(KERN_NUMVNODES) failed"); - } else + if (kd != NULL) KGET(V_NUMV, numvnodes); - *avnodes = numvnodes; if (totalflag) return NULL; if ((vbuf = calloc(numvnodes + 20, @@ -835,7 +8
intro.2 mention of pledge(2)
Does this make sense? Rob Index: intro.2 === RCS file: /cvs/src/lib/libc/sys/intro.2,v retrieving revision 1.63 diff -u -p -r1.63 intro.2 --- intro.2 6 Mar 2016 22:32:09 - 1.63 +++ intro.2 15 Apr 2016 16:26:24 - @@ -42,6 +42,9 @@ .Sh DESCRIPTION The manual pages in section 2 provide an overview of the system calls, their error returns, and other common definitions and concepts. +.Pp +Programs may be restricted to a subset of system calls with +.Xr pledge 2 . .\".Pp .\".Sy System call restart .\".Pp @@ -725,6 +728,7 @@ Each socket has an address chosen from t socket was created. .El .Sh SEE ALSO +.Xr pledge 2 , .Xr intro 3 , .Xr perror 3 .Sh HISTORY
update logging in ifstated
This minimizes differences with the latest log.c. I was not sure how to handle verbosity, as the current implementation is verbose by default in debug mode. The diff below requires actually requesting (double) verbosity on the command line in order to retain the same behaviour (in debug mode). Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.41 diff -u -p -r1.41 ifstated.c --- ifstated.c 30 May 2013 19:22:48 - 1.41 +++ ifstated.c 11 Jun 2017 21:51:16 - @@ -32,10 +32,11 @@ #include #include +#include #include #include #include -#include +#include #include #include #include @@ -86,14 +87,12 @@ main(int argc, char *argv[]) { struct timeval tv; int ch; - int debug = 0; - - log_init(1); + int debug = 0, verbose = 0; while ((ch = getopt(argc, argv, "dD:f:hniv")) != -1) { switch (ch) { case 'd': - debug = 1; + debug = 2; break; case 'D': if (cmdline_symset(optarg) < 0) @@ -113,6 +112,7 @@ main(int argc, char *argv[]) opt_inhibit = 1; break; case 'v': + verbose++; if (opts & IFSD_OPT_VERBOSE) opts |= IFSD_OPT_VERBOSE2; opts |= IFSD_OPT_VERBOSE; @@ -122,6 +122,9 @@ main(int argc, char *argv[]) } } + /* log to stderr until daemonized */ + log_init(debug ? debug : 1, LOG_DAEMON); + argc -= optind; argv += optind; if (argc > 0) @@ -138,7 +141,8 @@ main(int argc, char *argv[]) daemon(1, 0); event_init(); - log_init(debug); + log_init(debug, LOG_DAEMON); + log_setverbose(verbose); signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -169,12 +173,12 @@ startup_handler(int fd, short event, voi rtfilter = ROUTE_FILTER(RTM_IFINFO); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("startup_handler: setsockopt msgfilter"); + log_warn("%s: setsockopt msgfilter", __func__); rtfilter = RTABLE_ANY; if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("startup_handler: setsockopt tablefilter"); + log_warn("%s: setsockopt tablefilter", __func__); event_set(&rt_msg_ev, rt_fd, EV_READ|EV_PERSIST, rt_msg_handler, NULL); event_add(&rt_msg_ev, NULL); @@ -580,7 +584,7 @@ do_action(struct ifsd_action *action) } break; default: - log_debug("do_action: unknown action %d", action->type); + log_debug("%s: unknown action %d", __func__, action->type); break; } } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.10 diff -u -p -r1.10 ifstated.h --- ifstated.h 19 Jul 2016 08:04:53 - 1.10 +++ ifstated.h 11 Jun 2017 21:51:16 - @@ -137,7 +137,10 @@ intcmdline_symset(char *); void clear_config(struct ifsd_config *); /* log.c */ -void log_init(int); +void log_init(int, int); +void log_procinit(const char *); +void log_setverbose(int); +intlog_getverbose(void); void log_warn(const char *, ...) __attribute__((__format__ (printf, 1, 2))); void log_warnx(const char *, ...) @@ -146,9 +149,11 @@ void log_info(const char *, ...) __attribute__((__format__ (printf, 1, 2))); void log_debug(const char *, ...) __attribute__((__format__ (printf, 1, 2))); -void vlog(int, const char *, va_list) - __attribute__((__format__ (printf, 2, 0))); void logit(int, const char *, ...) __attribute__((__format__ (printf, 2, 3))); -void fatal(const char *) __dead; -void fatalx(const char *) __dead; +void vlog(int, const char *, va_list) + __attribute__((__format__ (printf, 2, 0))); +__dead void fatal(const char *, ...) + __attribute__((__format__ (printf, 1, 2))); +__dead void fatalx(const char *, ...) + __attribute__((__format__ (printf, 1, 2))); Index: log.c === RCS file: /cvs/src/usr.sbin/ifstated/log.c,v retrieving revision 1.4 diff -u -p -r1.4 log.c --- log.c 21 Mar 2017 12:06:55 - 1.4 +++ log.c 11 Jun 2017 21:51:16 - @@ -16,40 +16,74 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE
Re: update logging in ifstated
On Tue, Jun 13, 2017 at 08:44:46AM +0200, Sebastian Benoit wrote: > Rob Pierce(r...@2keys.ca) on 2017.06.11 18:04:31 -0400: > > This minimizes differences with the latest log.c. > > > > I was not sure how to handle verbosity, as the current implementation is > > verbose by default in debug mode. The diff below requires actually > > requesting (double) verbosity on the command line in order to retain the > > same behaviour (in debug mode). > > Thanks. > > can you redo this with the log.c from bgpd/ospfd/... ? > The difference is exactly in the handling of verbose, and that way we dont > change the -v semantics right now. > > I havent unified the verbose handling yet, but if you diff them you'll see > its only a small difference now. My plan is to narrow all the log.c users > down to this small difference and then fix that in all of them in one go. > > Also please add a log.h with the declarations just like in bgpd/ospfd/... > > /Benno Ah yes, thank you for the pointer. How is this? log.[ch] are now identical to bgpd and ospfd. Rob cvs server: Diffing . Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.41 diff -u -p -r1.41 ifstated.c --- ifstated.c 30 May 2013 19:22:48 - 1.41 +++ ifstated.c 13 Jun 2017 23:09:05 - @@ -36,12 +36,14 @@ #include #include #include +#include #include #include #include #include #include "ifstated.h" +#include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; @@ -88,7 +90,8 @@ main(int argc, char *argv[]) int ch; int debug = 0; - log_init(1); + log_init(1, LOG_DAEMON);/* log to stderr until daemonized */ + log_setverbose(1); while ((ch = getopt(argc, argv, "dD:f:hniv")) != -1) { switch (ch) { @@ -138,7 +141,8 @@ main(int argc, char *argv[]) daemon(1, 0); event_init(); - log_init(debug); + log_init(debug, LOG_DAEMON); + log_setverbose(opts & IFSD_OPT_VERBOSE); signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -169,12 +173,12 @@ startup_handler(int fd, short event, voi rtfilter = ROUTE_FILTER(RTM_IFINFO); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("startup_handler: setsockopt msgfilter"); + log_warn("%s: setsockopt msgfilter", __func__); rtfilter = RTABLE_ANY; if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("startup_handler: setsockopt tablefilter"); + log_warn("%s: setsockopt tablefilter", __func__); event_set(&rt_msg_ev, rt_fd, EV_READ|EV_PERSIST, rt_msg_handler, NULL); event_add(&rt_msg_ev, NULL); @@ -580,7 +584,7 @@ do_action(struct ifsd_action *action) } break; default: - log_debug("do_action: unknown action %d", action->type); + log_debug("%s: unknown action %d", __func__, action->type); break; } } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.10 diff -u -p -r1.10 ifstated.h --- ifstated.h 19 Jul 2016 08:04:53 - 1.10 +++ ifstated.h 13 Jun 2017 23:09:05 - @@ -135,20 +135,3 @@ enum { IFSD_EVTIMER_ADD, IFSD_EVTIMER_DE struct ifsd_config *parse_config(char *, int); intcmdline_symset(char *); void clear_config(struct ifsd_config *); - -/* log.c */ -void log_init(int); -void log_warn(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_warnx(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_info(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_debug(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void vlog(int, const char *, va_list) - __attribute__((__format__ (printf, 2, 0))); -void logit(int, const char *, ...) - __attribute__((__format__ (printf, 2, 3))); -void fatal(const char *) __dead; -void fatalx(const char *) __dead; Index: log.c === RCS file: /cvs/src/usr.sbin/ifstated/log.c,v retrieving revision 1.4 diff -u -p -r1.4 log.c --- log.c 21 Mar 2017 12:06:55 - 1.4 +++ log.c 13 Jun 2017 23:09:05 - @@ -16,40 +16,55 @@ * OR IN CONNECTION WITH THE USE
minor bgpd.c diff
rfd does not need to be global. Rob Index: bgpd.c === RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v retrieving revision 1.189 diff -u -p -r1.189 bgpd.c --- bgpd.c 28 May 2017 15:16:33 - 1.189 +++ bgpd.c 27 Jun 2017 15:52:02 - @@ -48,7 +48,6 @@ int dispatch_imsg(struct imsgbuf *, int intcontrol_setup(struct bgpd_config *); intimsg_send_sockets(struct imsgbuf *, struct imsgbuf *); -int rfd = -1; int cflags; volatile sig_atomic_t mrtdump; volatile sig_atomic_t quit; @@ -108,6 +107,7 @@ main(int argc, char *argv[]) char*saved_argv0; int debug = 0; int rflag = 0, sflag = 0; + int rfd = -1; int ch, timeout, status; int pipe_m2s[2]; int pipe_m2r[2];
ifstated.c hoist code in prep for future work
Hoist some privileged code in preparation for future work. Is this the correct use of intptr_t? Based on an approach in vmd with mc146818/ns8250. No intended functional change. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.42 diff -u -p -r1.42 ifstated.c --- ifstated.c 18 Jun 2017 12:03:47 - 1.42 +++ ifstated.c 27 Jun 2017 16:44:16 - @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -87,8 +88,9 @@ int main(int argc, char *argv[]) { struct timeval tv; - int ch; + int ch, rt_fd; int debug = 0; + unsigned int rtfilter; log_init(1, LOG_DAEMON);/* log to stderr until daemonized */ log_setverbose(1); @@ -144,12 +146,25 @@ main(int argc, char *argv[]) log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); + if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) + err(1, "no routing socket"); + + rtfilter = ROUTE_FILTER(RTM_IFINFO); + if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, + &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ + log_warn("%s: setsockopt msgfilter", __func__); + + rtfilter = RTABLE_ANY; + if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER, + &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ + log_warn("%s: setsockopt tablefilter", __func__); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); /* Loading the config needs to happen in the event loop */ timerclear(&tv); - evtimer_set(&startup_ev, startup_handler, NULL); + evtimer_set(&startup_ev, startup_handler, (void *)(intptr_t)rt_fd); evtimer_add(&startup_ev, &tv); event_loop(0); @@ -159,28 +174,14 @@ main(int argc, char *argv[]) void startup_handler(int fd, short event, void *arg) { - int rt_fd; - unsigned int rtfilter; - - if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) - err(1, "no routing socket"); + int rfd = (intptr_t)arg; if (load_config() != 0) { log_warnx("unable to load config"); exit(1); } - - rtfilter = ROUTE_FILTER(RTM_IFINFO); - if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, - &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("%s: setsockopt msgfilter", __func__); - - rtfilter = RTABLE_ANY; - if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER, - &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ - log_warn("%s: setsockopt tablefilter", __func__); - event_set(&rt_msg_ev, rt_fd, EV_READ|EV_PERSIST, rt_msg_handler, NULL); + event_set(&rt_msg_ev, rfd, EV_READ|EV_PERSIST, rt_msg_handler, NULL); event_add(&rt_msg_ev, NULL); signal_set(&sighup_ev, SIGHUP, sighup_handler, NULL);
remove errant ifstated whitespace
Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.43 diff -u -p -r1.43 ifstated.c --- ifstated.c 27 Jun 2017 20:46:34 - 1.43 +++ ifstated.c 28 Jun 2017 01:30:02 - @@ -151,12 +151,12 @@ main(int argc, char *argv[]) rtfilter = ROUTE_FILTER(RTM_IFINFO); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, - &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ + &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt msgfilter", __func__); rtfilter = RTABLE_ANY; if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER, - &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ + &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); @@ -605,7 +605,7 @@ fetch_state(void) for (ifa = ifap; ifa; ifa = ifa->ifa_next) { struct ifreq ifr; - struct if_data ifrdat; + struct if_data ifrdat; if (oname && !strcmp(oname, ifa->ifa_name)) continue; @@ -623,8 +623,6 @@ fetch_state(void) freeifaddrs(ifap); close(sock); } - - /* * Clear the config. Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.11 diff -u -p -r1.11 ifstated.h --- ifstated.h 18 Jun 2017 12:03:47 - 1.11 +++ ifstated.h 28 Jun 2017 01:30:02 - @@ -29,7 +29,6 @@ #include #include - struct ifsd_expression; TAILQ_HEAD(ifsd_expression_list, ifsd_expression); @@ -80,7 +79,6 @@ struct ifsd_action { #define IFSD_ACTION_CHANGESTATE2 #define IFSD_ACTION_CONDITION 3 }; - struct ifsd_expression { TAILQ_ENTRY(ifsd_expression) entries;
rename variable in ifstated
never.never say always.always. Rename one of the "always" variables to "body" for improved readability. No functional change. >From ifstated.conf(5): "Each state consistes of an init block and a body. The init block is used to initialize the state and is executed each time the state is entered. The body of a state is only executed when that state is the current state and an event occurs." Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.45 diff -u -p -r1.45 ifstated.c --- ifstated.c 28 Jun 2017 11:10:08 - 1.45 +++ ifstated.c 1 Jul 2017 21:49:47 - @@ -218,7 +218,7 @@ load_config(void) conf->nextstate = conf->curstate; conf->curstate = NULL; while (state_change()) - do_action(conf->curstate->always); + do_action(conf->curstate->body); } return (0); } @@ -530,9 +530,9 @@ eval_state(struct ifsd_state *state) struct ifsd_external *external = TAILQ_FIRST(&state->external_tests); if (external == NULL || external->lastexec >= state->entered || external->lastexec == 0) { - do_action(state->always); + do_action(state->body); while (state_change()) - do_action(conf->curstate->always); + do_action(conf->curstate->body); } } @@ -639,12 +639,12 @@ clear_config(struct ifsd_config *oconf) while ((state = TAILQ_FIRST(&oconf->states)) != NULL) { TAILQ_REMOVE(&oconf->states, state, entries); remove_action(state->init, state); - remove_action(state->always, state); + remove_action(state->body, state); free(state->name); free(state); } remove_action(oconf->always.init, &oconf->always); - remove_action(oconf->always.always, &oconf->always); + remove_action(oconf->always.body, &oconf->always); free(oconf); } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.12 diff -u -p -r1.12 ifstated.h --- ifstated.h 28 Jun 2017 11:10:08 - 1.12 +++ ifstated.h 1 Jul 2017 21:49:47 - @@ -110,7 +110,7 @@ struct ifsd_state { struct ifsd_external_listexternal_tests; TAILQ_ENTRY(ifsd_state) entries; struct ifsd_action *init; - struct ifsd_action *always; + struct ifsd_action *body; u_int32_tentered; char*name; }; Index: parse.y === RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v retrieving revision 1.41 diff -u -p -r1.41 parse.y --- parse.y 18 Jun 2017 12:03:47 - 1.41 +++ parse.y 1 Jul 2017 21:49:47 - @@ -245,9 +245,9 @@ init: INIT { curaction = conf->always.init; } action_block { if (curstate != NULL) - curaction = curstate->always; + curaction = curstate->body; else - curaction = conf->always.always; + curaction = conf->always.body; } ; @@ -339,11 +339,11 @@ state : STATE string { init_state(state); state->name = $2; curstate = state; - curaction = state->always; + curaction = state->body; } optnl '{' optnl stateopts_l '}' { TAILQ_INSERT_TAIL(&conf->states, curstate, entries); curstate = NULL; - curaction = conf->always.always; + curaction = conf->always.body; } ; @@ -747,7 +747,7 @@ parse_config(char *filename, int opts) TAILQ_INIT(&conf->states); init_state(&conf->always); - curaction = conf->always.always; + curaction = conf->always.body; conf->opts = opts; yyparse(); @@ -755,7 +755,7 @@ parse_config(char *filename, int opts) /* Link states */ TAILQ_FOREACH(state, &conf->states, entries) { link_states(state->init); - link_states(state->always); + link_states(state->body); } errors = file->errors; @@ -928,10 +928,10 @@ init_state(struct ifsd_state *state) state->init->type = IFSD_ACTION_CONDITION; TAILQ_INIT(&state->init->act.c.actions); - if ((state->always = calloc(1, sizeof(*state->always))) == NULL) + i
ifstated whitespace var assignment diff
Remove variable assignment in declaration and add whitespace to improve readability. No functional change. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.45 diff -u -p -r1.45 ifstated.c --- ifstated.c 28 Jun 2017 11:10:08 - 1.45 +++ ifstated.c 2 Jul 2017 03:52:04 - @@ -527,7 +527,9 @@ adjust_expressions(struct ifsd_expressio void eval_state(struct ifsd_state *state) { - struct ifsd_external *external = TAILQ_FIRST(&state->external_tests); + struct ifsd_external *external; + + external = TAILQ_FIRST(&state->external_tests); if (external == NULL || external->lastexec >= state->entered || external->lastexec == 0) { do_action(state->always);
ifstated unused variable
Remove unused variable from header file. Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.12 diff -u -p -r1.12 ifstated.h --- ifstated.h 28 Jun 2017 11:10:08 - 1.12 +++ ifstated.h 2 Jul 2017 04:30:01 - @@ -70,7 +70,6 @@ struct ifsd_action { struct { struct ifsd_action_list actions; struct ifsd_expression *expression; - u_int8_t ignore_init; } c; } act; u_int32_ttype;
Re: rename variable in ifstated
On Sun, Jul 02, 2017 at 02:56:06PM +0200, Stefan Sperling wrote: > On Sat, Jul 01, 2017 at 05:53:54PM -0400, Rob Pierce wrote: > > never.never say always.always. > > > > Rename one of the "always" variables to "body" for improved readability. > > > > No functional change. > > > > >From ifstated.conf(5): > > > > "Each state consistes of an init block and a body. The init block is used > > to initialize the state and is executed each time the state is entered. > > The body of a state is only executed when that state is the current state > > and an event occurs." > > > > Rob > > I agree with this change. > > Could you make the same change for struct ifsd_config in a follow-up diff? Will do. I have made the change for struct ifsd_config locally, but wanted to get this first change committed prior to sending the next diff to make it less confusing. It is still somewhat awkward because there is both an init state (i.e. init-state) as well as an init block within each state. I have renamed the second (struct ifsd_config) instance of "always" to "initstate". Stay tuned. Rob
ifstated ifsd_config variable name change
Second diff to rename additional "always" variable in struct ifsd_config. Requested by stsp@. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.47 diff -u -p -r1.47 ifstated.c --- ifstated.c 2 Jul 2017 14:28:45 - 1.47 +++ ifstated.c 2 Jul 2017 14:47:49 - @@ -207,11 +207,11 @@ load_config(void) if (conf != NULL) clear_config(conf); conf = newconf; - conf->always.entered = time(NULL); + conf->initstate.entered = time(NULL); fetch_state(); - external_evtimer_setup(&conf->always, IFSD_EVTIMER_ADD); - adjust_external_expressions(&conf->always); - eval_state(&conf->always); + external_evtimer_setup(&conf->initstate, IFSD_EVTIMER_ADD); + adjust_external_expressions(&conf->initstate); + eval_state(&conf->initstate); if (conf->curstate != NULL) { log_info("initial state: %s", conf->curstate->name); conf->curstate->entered = time(NULL); @@ -250,7 +250,7 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { - check_external_status(&conf->always); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); } @@ -463,8 +463,8 @@ scan_ifstate(int ifindex, int s, int do_ struct ifsd_state *state; int cur_eval = 0; - if (scan_ifstate_single(ifindex, s, &conf->always) && do_eval) - eval_state(&conf->always); + if (scan_ifstate_single(ifindex, s, &conf->initstate) && do_eval) + eval_state(&conf->initstate); TAILQ_FOREACH(state, &conf->states, entries) { if (scan_ifstate_single(ifindex, s, state) && (do_eval && state == conf->curstate)) @@ -635,7 +635,7 @@ clear_config(struct ifsd_config *oconf) { struct ifsd_state *state; - external_evtimer_setup(&conf->always, IFSD_EVTIMER_DEL); + external_evtimer_setup(&conf->initstate, IFSD_EVTIMER_DEL); if (conf != NULL && conf->curstate != NULL) external_evtimer_setup(conf->curstate, IFSD_EVTIMER_DEL); while ((state = TAILQ_FIRST(&oconf->states)) != NULL) { @@ -645,8 +645,8 @@ clear_config(struct ifsd_config *oconf) free(state->name); free(state); } - remove_action(oconf->always.init, &oconf->always); - remove_action(oconf->always.body, &oconf->always); + remove_action(oconf->initstate.init, &oconf->initstate); + remove_action(oconf->initstate.body, &oconf->initstate); free(oconf); } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.14 diff -u -p -r1.14 ifstated.h --- ifstated.h 2 Jul 2017 14:30:35 - 1.14 +++ ifstated.h 2 Jul 2017 14:47:49 - @@ -117,7 +117,7 @@ struct ifsd_state { TAILQ_HEAD(ifsd_state_list, ifsd_state); struct ifsd_config { - struct ifsd_statealways; + struct ifsd_stateinitstate; struct ifsd_state_list states; struct ifsd_state *curstate; struct ifsd_state *nextstate; Index: parse.y === RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v retrieving revision 1.42 diff -u -p -r1.42 parse.y --- parse.y 2 Jul 2017 14:27:30 - 1.42 +++ parse.y 2 Jul 2017 14:47:49 - @@ -242,12 +242,12 @@ init : INIT { if (curstate != NULL) curaction = curstate->init; else - curaction = conf->always.init; + curaction = conf->initstate.init; } action_block { if (curstate != NULL) curaction = curstate->body; else - curaction = conf->always.body; + curaction = conf->initstate.body; } ; @@ -343,7 +343,7 @@ state : STATE string { } optnl '{' optnl stateopts_l '}' { TAILQ_INSERT_TAIL(&conf->states, curstate, entries); curstate = NULL; - curaction = conf->always.body; + curaction = conf->initstate.body; } ; @@ -746,8 +746,8 @@ parse_config(char *filename, int opts) TAILQ_INIT(&conf->states); - init_state(&conf->always); - curaction = conf->always.body; + init_state(&conf->initstate); + curaction = conf->initstate.body; conf->opts = opts;
add simple ifstated regression test script
I am currently using this regression script for basic ifstated sanity testing. Still a work in progress. Requesting commit for safe keeping. Regards, Rob Index: usr.sbin/ifstated/ifstated.sh === RCS file: usr.sbin/ifstated/ifstated.sh diff -N usr.sbin/ifstated/ifstated.sh --- /dev/null 1 Jan 1970 00:00:00 - +++ usr.sbin/ifstated/ifstated.sh 2 Jul 2017 16:01:29 - @@ -0,0 +1,156 @@ +# $OpenBSD$ + +# basic ifstated regression test script + +# Modify variables as required +NIC=em0 +VHID=254 +PREFIX=172.16.0 +PING=8.8.8.8 +DEMOTE=ifconfig +PROMOTE=ifconfig +EVERY=3 +SLEEP=6 + +if [ "$(pgrep ifstated)" ] +then + echo "The ifstated daemon is already running." + echo "Please stop ifstated before commencing regression testing." + exit 1 +fi + +if [ -n "$(grep ifstated_test_host /etc/hosts)" ] +then + echo "ifstated_test_host is already present in /etc/hosts." + echo "Please remove before commencing regression testing." + exit 1 +fi + +if [ -n "$(grep 1.2.3.4 /etc/hosts)" ] +then + echo "1.2.3.4 is already present in /etc/hosts." + echo "Please remove before commencing regression testing." + exit 1 +fi + +if [ -f /etc/hostname.carp${VHID} ] +then + echo "/etc/hostname.carp${VHID} already exists." + echo "Please remove before commencing regression testing." + exit 1 +else + echo "inet ${PREFIX}.${VHID} 255.255.255.0 ${PREFIX}.255 vhid ${VHID} carpdev ${NIC}" > hostname.carp${VHID} + cp hostname.carp${VHID} /etc + chmod 0640 /etc/hostname.carp${VHID} +fi + +mkdir -p working +cd working + +cat > output.test <> /etc/hosts < ifstated.conf < ifstated.log & +echo + +echo -n "Performing ifstated regression testing..." +sleep ${SLEEP} +ifconfig ${NIC} down +sleep ${SLEEP} +sh /etc/netstart ${NIC} >/dev/null 2>&1 +sleep ${SLEEP} +ifconfig carp${VHID} down +sleep ${SLEEP} +sh /etc/netstart carp${VHID} +sleep ${SLEEP} +ifconfig ${NIC} down +ifconfig carp${VHID} down +sleep ${SLEEP} +sh /etc/netstart ${NIC} >/dev/null 2>&1 +sleep ${SLEEP} +sh /etc/netstart carp${VHID} +sleep ${SLEEP} +kill -HUP $(pgrep ifstated) +sleep ${SLEEP} +cat /etc/hosts | sed "s/.* ifstated_test_host/1.2.3.4 ifstated_test_host/" > /etc/hosts +sleep ${SLEEP} +cat /etc/hosts | sed "s/.* ifstated_test_host/${PING} ifstated_test_host/" > /etc/hosts +sleep ${SLEEP} +echo + +grep ^changing ifstated.log > output.new + +echo -n "Stopping ifstated..." +kill $(pgrep ifstated) +echo + +diff output.test output.new +case $? in +0) echo Test Passed. + echo -n "Cleaning up..." + ifconfig carp${VHID} down + ifconfig carp${VHID} delete + ifconfig carp${VHID} destroy + rm /etc/hostname.carp${VHID} + cp ./hosts /etc + echo + echo Done. + exit 0 + ;; +1) echo Test Failed. + echo Please examine the interfaces and review files in the working directory. + echo You will need to manually delete the carp interface and remove residual files. + exit 1 + ;; +esac
Re: add simple ifstated regression test script
> From: "Sebastian Benoit" > To: "Rob Pierce" > Cc: "tech" > Sent: Sunday, July 2, 2017 12:29:07 PM > Subject: Re: add simple ifstated regression test script > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: > > I am currently using this regression script for basic ifstated sanity > > testing. > > Still a work in progress. Requesting commit for safe keeping. > Hi, > this should go into /usr/src/regress/usr.sbin/ifstated > (which does not esist yet). > Also, it should hook into the regress framework (bsd.regress.mk(5)). > As it needs some network configuration, maybe it should be similar to > relayd regress tests. > Happy to work with you on that. > /B. I have it locally in src/usr.sbin/regress/ but did my cvs diff too high in the tree. :) I didn't want to hook it up yet, I just wanted to commit it so I can continue to work on it in the tree. I have been studying the other regression scripts, but took the same approach as disklabel (i.e. just a script for local use). The next most straight forward example I found was newfs. I can work on it a bit more and send a new diff for review and comments. Regards, Rob
ifstated readability diff
Remove obvious clear_config() comments and misleading state_change() comments. Also relocate do_action() calls for the init block from change_state() to occur with the corresponding do_action() calls for the body block within the calling function for improved readability. No functional change. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.48 diff -u -p -r1.48 ifstated.c --- ifstated.c 2 Jul 2017 15:28:26 - 1.48 +++ ifstated.c 3 Jul 2017 03:41:14 - @@ -218,6 +218,7 @@ load_config(void) conf->nextstate = conf->curstate; conf->curstate = NULL; while (state_change()) + do_action(conf->curstate->init); do_action(conf->curstate->body); } return (0); @@ -534,13 +535,11 @@ eval_state(struct ifsd_state *state) external->lastexec == 0) { do_action(state->body); while (state_change()) + do_action(conf->curstate->init); do_action(conf->curstate->body); } } -/* - *If a previous action included a state change, process it. - */ int state_change(void) { @@ -556,7 +555,6 @@ state_change(void) conf->curstate->entered = time(NULL); external_evtimer_setup(conf->curstate, IFSD_EVTIMER_ADD); adjust_external_expressions(conf->curstate); - do_action(conf->curstate->init); return (1); } return (0); @@ -627,9 +625,6 @@ fetch_state(void) close(sock); } -/* - * Clear the config. - */ void clear_config(struct ifsd_config *oconf) {
Re: ifstated readability diff
On Sun, Jul 02, 2017 at 11:50:56PM -0400, Rob Pierce wrote: > Remove obvious clear_config() comments and misleading state_change() comments. > > Also relocate do_action() calls for the init block from change_state() to > occur with the corresponding do_action() calls for the body block within > the calling function for improved readability. > > No functional change. > > Rob That diff was broken. This one is correct. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.48 diff -u -p -r1.48 ifstated.c --- ifstated.c 2 Jul 2017 15:28:26 - 1.48 +++ ifstated.c 3 Jul 2017 13:42:13 - @@ -217,8 +217,10 @@ load_config(void) conf->curstate->entered = time(NULL); conf->nextstate = conf->curstate; conf->curstate = NULL; - while (state_change()) + while (state_change()) { + do_action(conf->curstate->init); do_action(conf->curstate->body); + } } return (0); } @@ -533,14 +535,13 @@ eval_state(struct ifsd_state *state) if (external == NULL || external->lastexec >= state->entered || external->lastexec == 0) { do_action(state->body); - while (state_change()) + while (state_change()) { + do_action(conf->curstate->init); do_action(conf->curstate->body); + } } } -/* - *If a previous action included a state change, process it. - */ int state_change(void) { @@ -556,7 +557,6 @@ state_change(void) conf->curstate->entered = time(NULL); external_evtimer_setup(conf->curstate, IFSD_EVTIMER_ADD); adjust_external_expressions(conf->curstate); - do_action(conf->curstate->init); return (1); } return (0); @@ -627,9 +627,6 @@ fetch_state(void) close(sock); } -/* - * Clear the config. - */ void clear_config(struct ifsd_config *oconf) {
ifstated whitespace diff
Fix some variable alignment whitespace. Rob Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.15 diff -u -p -r1.15 ifstated.h --- ifstated.h 2 Jul 2017 15:28:26 - 1.15 +++ ifstated.h 3 Jul 2017 19:40:21 - @@ -92,9 +92,9 @@ struct ifsd_expression { } u; int depth; u_int32_ttype; -#define IFSD_OPER_AND 1 -#define IFSD_OPER_OR 2 -#define IFSD_OPER_NOT 3 +#define IFSD_OPER_AND 1 +#define IFSD_OPER_OR 2 +#define IFSD_OPER_NOT 3 #define IFSD_OPER_EXTERNAL 4 #define IFSD_OPER_IFSTATE 5 u_int8_t truth;
ifstated diff rename variables to avoid state confusion
ifstated monitors interface state and the return state of invoked commands, and takes action accordingly, all of which is managed with the help of a finite state machine. That makes for a lot of "state" references in the code. The following diff renames variables to make a distinction between link status and command execution status (now referenced as "status"), and the various states within the state machine (referred to as "state"). As an example of the current confusion, the link_states() function in parse.y actually links states within the finite state machine (as oppose to having anything to do with interface link states). This diff helps to make my head hurt less. For your consideration. Regards, Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.49 diff -u -p -r1.49 ifstated.c --- ifstated.c 3 Jul 2017 18:45:34 - 1.49 +++ ifstated.c 3 Jul 2017 19:59:43 - @@ -18,7 +18,7 @@ */ /* - * ifstated listens to link_state transitions on interfaces + * ifstated listens to link status transitions on interfaces * and executes predefined commands. */ @@ -62,9 +62,9 @@ void external_handler(int, short, void void external_exec(struct ifsd_external *, int); void check_external_status(struct ifsd_state *); void external_evtimer_setup(struct ifsd_state *, int); -void scan_ifstate(int, int, int); -intscan_ifstate_single(int, int, struct ifsd_state *); -void fetch_state(void); +void scan_ifstatus(int, int, int); +intscan_ifstatus_single(int, int, struct ifsd_state *); +void fetch_ifstatus(void); __dead voidusage(void); void adjust_expressions(struct ifsd_expression_list *, int); void adjust_external_expressions(struct ifsd_state *); @@ -208,7 +208,7 @@ load_config(void) clear_config(conf); conf = newconf; conf->initstate.entered = time(NULL); - fetch_state(); + fetch_ifstatus(); external_evtimer_setup(&conf->initstate, IFSD_EVTIMER_ADD); adjust_external_expressions(&conf->initstate); eval_state(&conf->initstate); @@ -246,7 +246,7 @@ rt_msg_handler(int fd, short event, void return; memcpy(&ifm, rtm, sizeof(ifm)); - scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + scan_ifstatus(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); } void @@ -419,37 +419,37 @@ external_evtimer_setup(struct ifsd_state #defineLINK_STATE_IS_DOWN(_s) (!LINK_STATE_IS_UP((_s))) int -scan_ifstate_single(int ifindex, int s, struct ifsd_state *state) +scan_ifstatus_single(int ifindex, int s, struct ifsd_state *state) { - struct ifsd_ifstate *ifstate; + struct ifsd_ifstatus *ifstatus; struct ifsd_expression_list expressions; int changed = 0; TAILQ_INIT(&expressions); - TAILQ_FOREACH(ifstate, &state->interface_states, entries) { - if (ifstate->ifindex == ifindex) { - if (ifstate->prevstate != s && - (ifstate->prevstate != -1 || !opt_inhibit)) { + TAILQ_FOREACH(ifstatus, &state->interfaces_status, entries) { + if (ifstatus->ifindex == ifindex) { + if (ifstatus->prevstatus != s && + (ifstatus->prevstatus != -1 || !opt_inhibit)) { struct ifsd_expression *expression; int truth; truth = - (ifstate->ifstate == IFSD_LINKUNKNOWN && + (ifstatus->ifstatus == IFSD_LINKUNKNOWN && s == LINK_STATE_UNKNOWN) || - (ifstate->ifstate == IFSD_LINKDOWN && + (ifstatus->ifstatus == IFSD_LINKDOWN && LINK_STATE_IS_DOWN(s)) || - (ifstate->ifstate == IFSD_LINKUP && + (ifstatus->ifstatus == IFSD_LINKUP && LINK_STATE_IS_UP(s)); TAILQ_FOREACH(expression, - &ifstate->expressions, entries) { + &ifstatus->expressions, entries) { expression->truth = truth; TAILQ_INSERT_TAIL(&expressions, expression, eval); changed = 1; } - ifstate->prevstate = s; + ifstatus->prevstatus = s; } } } @@ -460,15 +460,15
ifstated parse.y removed unused tokens
These tokens have existed since version 1.1 but have never been used. Can we delete them? Rob Index: parse.y === RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v retrieving revision 1.43 diff -u -p -r1.43 parse.y --- parse.y 2 Jul 2017 15:28:26 - 1.43 +++ parse.y 4 Jul 2017 09:33:17 - @@ -106,7 +106,7 @@ typedef struct { %} %token STATE INITSTATE -%token LINK UP DOWN UNKNOWN ADDED REMOVED +%token LINK UP DOWN UNKNOWN %token IF RUN SETSTATE EVERY INIT %left AND OR %left UNARY @@ -390,14 +390,12 @@ lookup(char *s) /* this has to be sorted always */ static const struct keywords keywords[] = { { "&&", AND}, - { "added", ADDED}, { "down", DOWN}, { "every", EVERY}, { "if", IF}, { "init", INIT}, { "init-state", INITSTATE}, { "link", LINK}, - { "removed",REMOVED}, { "run",RUN}, { "set-state", SETSTATE}, { "state", STATE},
Re: ifstated diff rename variables to avoid state confusion
On Mon, Jul 03, 2017 at 04:24:30PM -0400, Rob Pierce wrote: > ifstated monitors interface state and the return state of invoked commands, > and takes action accordingly, all of which is managed with the help of a > finite state machine. That makes for a lot of "state" references in the code. > > The following diff renames variables to make a distinction between link status > and command execution status (now referenced as "status"), and the various > states > within the state machine (referred to as "state"). > > As an example of the current confusion, the link_states() function in parse.y > actually links states within the finite state machine (as oppose to having > anything to do with interface link states). > > This diff helps to make my head hurt less. > > For your consideration. > > Regards, > > Rob Ok, that was overkill. I would be happy with the following diff instead. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.49 diff -u -p -r1.49 ifstated.c --- ifstated.c 3 Jul 2017 18:45:34 - 1.49 +++ ifstated.c 4 Jul 2017 11:31:08 - @@ -64,7 +64,7 @@ void check_external_status(struct ifsd_ void external_evtimer_setup(struct ifsd_state *, int); void scan_ifstate(int, int, int); intscan_ifstate_single(int, int, struct ifsd_state *); -void fetch_state(void); +void fetch_ifstate(void); __dead voidusage(void); void adjust_expressions(struct ifsd_expression_list *, int); void adjust_external_expressions(struct ifsd_state *); @@ -208,7 +208,7 @@ load_config(void) clear_config(conf); conf = newconf; conf->initstate.entered = time(NULL); - fetch_state(); + fetch_ifstate(); external_evtimer_setup(&conf->initstate, IFSD_EVTIMER_ADD); adjust_external_expressions(&conf->initstate); eval_state(&conf->initstate); @@ -597,7 +597,7 @@ do_action(struct ifsd_action *action) * Fetch the current link states. */ void -fetch_state(void) +fetch_ifstate(void) { struct ifaddrs *ifap, *ifa; char *oname = NULL;
ifstated remove unused logging code
This code has been here since version 1.1/1.2, but never used. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 6 Jul 2017 01:04:40 - @@ -656,9 +656,6 @@ remove_action(struct ifsd_action *action return; switch (action->type) { - case IFSD_ACTION_LOG: - free(action->act.logmessage); - break; case IFSD_ACTION_COMMAND: free(action->act.command); break; Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.16 diff -u -p -r1.16 ifstated.h --- ifstated.h 4 Jul 2017 21:04:14 - 1.16 +++ ifstated.h 6 Jul 2017 01:04:40 - @@ -63,7 +63,6 @@ struct ifsd_action { TAILQ_ENTRY(ifsd_action) entries; struct ifsd_action *parent; union { - char*logmessage; char*command; struct ifsd_state *nextstate; char*statename; @@ -73,7 +72,6 @@ struct ifsd_action { } c; } act; u_int32_ttype; -#define IFSD_ACTION_LOG0 #define IFSD_ACTION_COMMAND1 #define IFSD_ACTION_CHANGESTATE2 #define IFSD_ACTION_CONDITION 3
Re: add simple ifstated regression test script
On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote: > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: > > I am currently using this regression script for basic ifstated sanity > > testing. > > > > Still a work in progress. Requesting commit for safe keeping. > > > Hi, > > this should go into /usr/src/regress/usr.sbin/ifstated > (which does not esist yet). > > Also, it should hook into the regress framework (bsd.regress.mk(5)). > > As it needs some network configuration, maybe it should be similar to > relayd regress tests. > > Happy to work with you on that. > > /B. I have updated the ifstated regression scripts based on your feedback. I also added a script to test drive the state machine. Both should be more systematic in coverage, but hopefully it is a good start. Regards, Rob Index: regress/usr.sbin/ifstated/Makefile === RCS file: regress/usr.sbin/ifstated/Makefile diff -N regress/usr.sbin/ifstated/Makefile --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.sbin/ifstated/Makefile 6 Jul 2017 16:55:57 - @@ -0,0 +1,13 @@ +# $OpenBSD$ + +# Regress tests for ifstated + +REGRESS_TARGETS = run-regress-statemachine run-regress-ifstated + +run-regress-statemachine: + sh ${.CURDIR}/statemachine + +run-regress-ifstated: + sh ${.CURDIR}/ifstated + +.include Index: regress/usr.sbin/ifstated/ifstated === RCS file: regress/usr.sbin/ifstated/ifstated diff -N regress/usr.sbin/ifstated/ifstated --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.sbin/ifstated/ifstated 6 Jul 2017 16:55:57 - @@ -0,0 +1,148 @@ +# $OpenBSD$ + +# Basic ifstated regression script to test interface changes. + +# Golbal variables +VHIDA=252 +VHIDB=253 +PREFIX=172.16.0 +DEMOTE=ifconfig +PROMOTE=ifconfig +EVERY=5 +SLEEP=10 + +cleanup() { + ifconfig carp${VHIDA} destroy > /dev/null 2>&1 + ifconfig carp${VHIDB} destroy > /dev/null 2>&1 + rm working/ifstated.conf >/dev/null 2>&1 + rm working/ifstated.log >/dev/null 2>&1 + rm working/output.test >/dev/null 2>&1 + rm working/output.new >/dev/null 2>&1 + rm working/nohup.out >/dev/null 2>&1 + rmdir working >/dev/null 2>&1 +} + +fail() { + echo FAILED + cleanup + exit 1 +} + +skip() { + echo SKIPPED + cleanup + exit 0 +} + +trap 'skip' INT + +# look for a suitable physical interface for carp +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')" +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')" + +if [ "$STATUS" != "active" ] +then + echo "No suitable physical interface found." + echo SKIPPED + exit 0 +fi + +if [ "$(pgrep ifstated)" ] +then + echo "The ifstated daemon is already running." + echo SKIPPED + exit 0 +fi + +for interface in carp${VHIDA} carp${VHIDB} +do + ifconfig ${interface} > /dev/null 2>&1 + if [ $? -eq 0 ] + then + echo "Interface $interface already exists." + echo SKIPPED + exit 0 + fi +done + +mkdir -p working + +cat > working/ifstated.conf < working/output.test < ifstated.log 2>&1) & + +sleep ${SLEEP} +ifconfig carp${VHIDA} down +sleep ${SLEEP} +ifconfig carp${VHIDA} up +sleep ${SLEEP} +ifconfig carp${VHIDB} destroy +sleep ${SLEEP} +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 broadcast \ + ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} +sleep ${SLEEP} +ifconfig carp${VHIDA} down +sleep ${SLEEP} +ifconfig carp${VHIDB} destroy +sleep ${SLEEP} +ifconfig carp${VHIDA} up +sleep ${SLEEP} +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 broadcast \ + ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} +sleep ${SLEEP} +kill -HUP $(pgrep ifstated) >/dev/null 2>&1 +sleep ${SLEEP} + +grep ^changing working/ifstated.log > working/output.new + +kill $(pgrep ifstated) >/dev/null 2>&1 + +diff working/output.test working/output.new +case $? in +0) echo PASSED + cleanup + exit 0 + ;; +1) fail + ;; +esac Index: regress/usr.sbin/ifstated/statemachine === RCS file: regress/usr.sbin/ifstated/statemachine diff -N regress/usr.sbin/ifstated/statemachine --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.sbin/ifstated/statemachine 6 Jul 2017 16:55:57 - @@ -0,0 +1,183 @@ +# $OpenBSD$ + +# Basic ifstated regression script to test the finite state machine. + +# +# NOTE: Increase LSLEEP as required when adding additional test sta
pledge ifstated
The following diff is loosely based on the approach that was taken for pledging mountd. Other code/approaches leveraged from various networking daemons. This first step moves the ioctl with SIOCGIFDATA call to a privileged child so we can at least pledge "stdio rpath dns inet proc exec" without (intentionally) changing existing behaviour. Comments and suggestions welcome. Thanks! Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile13 Jul 2017 16:17:20 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 13 Jul 2017 16:17:20 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,22 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + setproctitle("parent"); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +184,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatal("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +636,75 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void) { + struct imsg imsg; struct ifaddrs *ifap, *ifa; + struct pollfd pfd[1]; char *oname = NULL; int sock = socket(AF_INET, SOCK_DGRAM, 0); + int done = 0, state, rv; + ssize_t n; if (getifaddrs(&ifap) != 0) err(1, "getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { struct ifreq ifr; - struct if_data ifrdat; if (oname && !strcmp(oname, ifa->ifa_name)) continue; oname = ifa->ifa_name; strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name)); - ifr.ifr_data = (caddr_t)&ifrdat; - - if (ioctl(sock, SIOCGIFDATA, (caddr_t)&ifr) == -1) - continue; - scan_ifstate(if_nametoindex(ifa->ifa_name), - ifrdat.ifi_link_state, 0); + /* +* synchronous imsg request/response to privchild +*/ + if (imsg_compose(&ibuf, IMSG_IFSTATE_REQ, 0, 0, -1, &ifr, +
Re: add simple ifstated regression test script
Sure, no problem. Thank you. Rob From: "Sebastian Benoit" To: "Rob Pierce" Cc: "tech" Sent: Thursday, July 13, 2017 6:12:14 PM Subject: Re: add simple ifstated regression test script BQ_BEGIN Hi, i wanted to commit this, but saw that it does not have a licence yet. Can i add /usr/share/misc/license.template with your name and email-Adress? /Benno Rob Pierce(r...@2keys.ca) on 2017.07.06 13:12:26 -0400: > On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote: > > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: > > > I am currently using this regression script for basic ifstated sanity > > > testing. > > > > > > Still a work in progress. Requesting commit for safe keeping. > > > > > > Hi, > > > > this should go into /usr/src/regress/usr.sbin/ifstated > > (which does not esist yet). > > > > Also, it should hook into the regress framework (bsd.regress.mk(5)). > > > > As it needs some network configuration, maybe it should be similar to > > relayd regress tests. > > > > Happy to work with you on that. > > > > /B. > > I have updated the ifstated regression scripts based on your feedback. > > I also added a script to test drive the state machine. > > Both should be more systematic in coverage, but hopefully it is a good start. > > Regards, > > Rob > > Index: regress/usr.sbin/ifstated/Makefile > === > RCS file: regress/usr.sbin/ifstated/Makefile > diff -N regress/usr.sbin/ifstated/Makefile > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/Makefile 6 Jul 2017 16:55:57 - > @@ -0,0 +1,13 @@ > +# $OpenBSD$ > + > +# Regress tests for ifstated > + > +REGRESS_TARGETS = run-regress-statemachine run-regress-ifstated > + > +run-regress-statemachine: > + sh ${.CURDIR}/statemachine > + > +run-regress-ifstated: > + sh ${.CURDIR}/ifstated > + > +.include > > Index: regress/usr.sbin/ifstated/ifstated > === > RCS file: regress/usr.sbin/ifstated/ifstated > diff -N regress/usr.sbin/ifstated/ifstated > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/ifstated 6 Jul 2017 16:55:57 - > @@ -0,0 +1,148 @@ > +# $OpenBSD$ > + > +# Basic ifstated regression script to test interface changes. > + > +# Golbal variables > +VHIDA=252 > +VHIDB=253 > +PREFIX=172.16.0 > +DEMOTE=ifconfig > +PROMOTE=ifconfig > +EVERY=5 > +SLEEP=10 > + > +cleanup() { > + ifconfig carp${VHIDA} destroy > /dev/null 2>&1 > + ifconfig carp${VHIDB} destroy > /dev/null 2>&1 > + rm working/ifstated.conf >/dev/null 2>&1 > + rm working/ifstated.log >/dev/null 2>&1 > + rm working/output.test >/dev/null 2>&1 > + rm working/output.new >/dev/null 2>&1 > + rm working/nohup.out >/dev/null 2>&1 > + rmdir working >/dev/null 2>&1 > +} > + > +fail() { > + echo FAILED > + cleanup > + exit 1 > +} > + > +skip() { > + echo SKIPPED > + cleanup > + exit 0 > +} > + > +trap 'skip' INT > + > +# look for a suitable physical interface for carp > +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')" > +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')" > + > +if [ "$STATUS" != "active" ] > +then > + echo "No suitable physical interface found." > + echo SKIPPED > + exit 0 > +fi > + > +if [ "$(pgrep ifstated)" ] > +then > + echo "The ifstated daemon is already running." > + echo SKIPPED > + exit 0 > +fi > + > +for interface in carp${VHIDA} carp${VHIDB} > +do > + ifconfig ${interface} > /dev/null 2>&1 > + if [ $? -eq 0 ] > + then > + echo "Interface $interface already exists." > + echo SKIPPED > + exit 0 > + fi > +done > + > +mkdir -p working > + > +cat > working/ifstated.conf < +# This is a config template for ifstated regression testing > +carp = "carp${VHIDA}.link.up" > +init-state primary > +net = '( "ping -q -c 1 -w 1 ${PREFIX}.${VHIDB} > /dev/null" every ${EVERY})' > +state primary { > + init { > + run "ifconfig" > + } > + if ! \$net > + set-state demoted > + if ! \$carp > + set-state demoted > +} > +state demoted { > + i
Re: pledge ifstated
On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote: > The following diff is loosely based on the approach that was taken for > pledging mountd. Other code/approaches leveraged from various networking > daemons. > > This first step moves the ioctl with SIOCGIFDATA call to a privileged > child so we can at least pledge "stdio rpath dns inet proc exec" without > (intentionally) changing existing behaviour. > > Comments and suggestions welcome. > > Thanks! > > Rob An unnecessary call to log_info snuck in. Here is a clean diff. Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile14 Jul 2017 01:12:58 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 14 Jul 2017 01:12:58 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,22 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + setproctitle("parent"); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +184,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatal("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +636,74 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void) { + struct imsg imsg; struct ifaddrs *ifap, *ifa; + struct pollfd pfd[1]; char *oname = NULL; int sock = socket(AF_INET, SOCK_DGRAM, 0); + int done = 0, state, rv; + ssize_t n; if (getifaddrs(&ifap) != 0) err(1, "getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { struct ifreq ifr; - struct if_data ifrdat; if (oname && !strcmp(oname, ifa->ifa_name)) continue; oname = ifa->ifa_name; strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name)); - ifr.ifr_data
ifstated regress statemachine whitespace diff
Remove some unwanted whitespace that creeped in. Rob Index: statemachine === RCS file: /cvs/src/regress/usr.sbin/ifstated/statemachine,v retrieving revision 1.1 diff -u -p -r1.1 statemachine --- statemachine14 Jul 2017 10:41:30 - 1.1 +++ statemachine14 Jul 2017 11:52:58 - @@ -74,7 +74,7 @@ test1 = '( "test -f ${FILE1}" every $EVE test2 = '( "test -f ${FILE2}" every $EVERY )' state one { init { - run "sleep $SLEEP && ( test -f ${FILE2} || touch ${FILE1} )" + run "sleep $SLEEP && ( test -f ${FILE2} || touch ${FILE1} )" } if \$test1 && ! \$test2 set-state two @@ -100,14 +100,14 @@ state five { init { run "sleep $SLEEP && touch ${FILE1}" } - if ( \$false || \$true ) && ( ! \$true || \$test1 ) + if ( \$false || \$true ) && ( ! \$true || \$test1 ) set-state six } state six { init { run "sleep $SLEEP && touch ${FILE1}" } - if ( \$false || \$true ) && ( ! \$true || \$test1 ) + if ( \$false || \$true ) && ( ! \$true || \$test1 ) if \$true set-state seven }
ifstated regress small tweak
I added this recently for some additional coverage so we hit link.down and link.unknown as well. Rob Index: ifstated === RCS file: /cvs/src/regress/usr.sbin/ifstated/ifstated,v retrieving revision 1.1 diff -u -p -r1.1 ifstated --- ifstated14 Jul 2017 10:41:30 - 1.1 +++ ifstated14 Jul 2017 11:59:37 - @@ -100,7 +100,7 @@ state demoted { init { run "ifconfig" } - if \$net && \$carp + if \$net && ( ! carp${VHIDA}.link.down && ! carp${VHIDA}.link.unknown ) set-state primary } EOF
ifstated diff cleanup before delegating proc/exec to privchild
This diff should probably be three separate diffs, but changes are colliding (i.e. adjacent lines of code are being modified within different diffs). The diff contains three modifications: - removal of unused logging code (see previous diff); - a cleanup of parse.y, converting it to use log.c; and - string to character array changes in preparation for the next pledge diff. Regards, Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 15 Jul 2017 17:56:31 - @@ -639,7 +639,6 @@ clear_config(struct ifsd_config *oconf) TAILQ_REMOVE(&oconf->states, state, entries); remove_action(state->init, state); remove_action(state->body, state); - free(state->name); free(state); } remove_action(oconf->initstate.init, &oconf->initstate); @@ -656,12 +655,7 @@ remove_action(struct ifsd_action *action return; switch (action->type) { - case IFSD_ACTION_LOG: - free(action->act.logmessage); - break; case IFSD_ACTION_COMMAND: - free(action->act.command); - break; case IFSD_ACTION_CHANGESTATE: break; case IFSD_ACTION_CONDITION: @@ -697,7 +691,6 @@ remove_expression(struct ifsd_expression if (--expression->u.external->refcount == 0) { TAILQ_REMOVE(&state->external_tests, expression->u.external, entries); - free(expression->u.external->command); event_del(&expression->u.external->ev); free(expression->u.external); } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.16 diff -u -p -r1.16 ifstated.h --- ifstated.h 4 Jul 2017 21:04:14 - 1.16 +++ ifstated.h 15 Jul 2017 17:56:31 - @@ -28,6 +28,7 @@ #include #include +#include struct ifsd_expression; TAILQ_HEAD(ifsd_expression_list, ifsd_expression); @@ -45,15 +46,15 @@ struct ifsd_ifstate { }; struct ifsd_external { - TAILQ_ENTRY(ifsd_external) entries; - struct event ev; - struct ifsd_expression_list expressions; - char*command; - int prevstatus; - u_int32_tfrequency; - u_int32_trefcount; - u_int32_tlastexec; - pid_tpid; + TAILQ_ENTRY(ifsd_external) entries; + struct eventev; + struct ifsd_expression_list expressions; + charcommand[LINE_MAX]; + int prevstatus; + u_int32_t frequency; + u_int32_t refcount; + u_int32_t lastexec; + pid_t pid; }; struct ifsd_action; @@ -63,17 +64,15 @@ struct ifsd_action { TAILQ_ENTRY(ifsd_action) entries; struct ifsd_action *parent; union { - char*logmessage; - char*command; + charcommand[LINE_MAX]; struct ifsd_state *nextstate; - char*statename; + charstatename[NAME_MAX]; struct { struct ifsd_action_list actions; struct ifsd_expression *expression; } c; } act; u_int32_ttype; -#define IFSD_ACTION_LOG0 #define IFSD_ACTION_COMMAND1 #define IFSD_ACTION_CHANGESTATE2 #define IFSD_ACTION_CONDITION 3 @@ -111,7 +110,7 @@ struct ifsd_state { struct ifsd_action *init; struct ifsd_action *body; u_int32_tentered; - char*name; + charname[NAME_MAX]; }; TAILQ_HEAD(ifsd_state_list, ifsd_state); Index: parse.y === RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v retrieving revision 1.44 diff -u -p -r1.44 parse.y --- parse.y 4 Jul 2017 21:13:03 - 1.44 +++ parse.y 15 Jul 2017 17:56:31 - @@ -190,11 +190,12 @@ action: RUN STRING{ struct ifsd_action *action; if ((action = callo
Re: pledge ifstated
On Thu, Jul 13, 2017 at 09:16:14PM -0400, Rob Pierce wrote: > On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote: > > The following diff is loosely based on the approach that was taken for > > pledging mountd. Other code/approaches leveraged from various networking > > daemons. > > > > This first step moves the ioctl with SIOCGIFDATA call to a privileged > > child so we can at least pledge "stdio rpath dns inet proc exec" without > > (intentionally) changing existing behaviour. > > > > Comments and suggestions welcome. > > > > Thanks! > > > > Rob > > An unnecessary call to log_info snuck in. Here is a clean diff. > > Rob My original diff to initially pledge ifstated with "stdio rpath dns inet proc exec" was incorrectly polling from fetch_ifstate which resulted in the delayed completion of that function. This new diff resolves the problem. I also changed one instance of fatal to fatalx in sigchld_handler. Regards, Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile16 Jul 2017 20:38:48 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 16 Jul 2017 20:38:48 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,22 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + setproctitle("parent"); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +184,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatalx("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +636,60 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void) { + struct imsg imsg; struct ifaddrs *ifap, *ifa; char *oname = NULL; int sock = socket(AF_INET, SOCK_DGRAM, 0); + int done = 0, state, rv; + ssize_t n; if (getifaddrs(&ifap) != 0) err(1, &qu
Re: pledge ifstated
On Sun, Jul 16, 2017 at 04:47:07PM -0400, Rob Pierce wrote: > On Thu, Jul 13, 2017 at 09:16:14PM -0400, Rob Pierce wrote: > > On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote: > > > The following diff is loosely based on the approach that was taken for > > > pledging mountd. Other code/approaches leveraged from various networking > > > daemons. > > > > > > This first step moves the ioctl with SIOCGIFDATA call to a privileged > > > child so we can at least pledge "stdio rpath dns inet proc exec" without > > > (intentionally) changing existing behaviour. > > > > > > Comments and suggestions welcome. > > > > > > Thanks! > > > > > > Rob > > > > An unnecessary call to log_info snuck in. Here is a clean diff. > > > > Rob > > My original diff to initially pledge ifstated with "stdio rpath dns inet proc > exec" was incorrectly polling from fetch_ifstate which resulted in the delayed > completion of that function. This new diff resolves the problem. > > I also changed one instance of fatal to fatalx in sigchld_handler. Running setproctitle in the parent breaks rcctl. This diff removes the parent call to setproctitle so rcctl works again. Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile17 Jul 2017 16:38:23 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 17 Jul 2017 16:38:23 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,21 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +183,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +278,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatalx("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +635,60 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void)
simple ifstated pledge
With the most recent commit ifstated can now be pledged in a straight forward manner. A better pledge is possible with more work. Does it make sense to get this one in now? Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.52 diff -u -p -r1.52 ifstated.c --- ifstated.c 21 Jul 2017 16:32:18 - 1.52 +++ ifstated.c 22 Jul 2017 03:58:23 - @@ -160,6 +160,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL);
Re: simple ifstated pledge
On Sun, Jul 23, 2017 at 12:26:53AM +0200, Jeremie Courreges-Anglas wrote: > On Sat, Jul 22 2017, Rob Pierce wrote: > > With the most recent commit ifstated can now be pledged in a straight > > forward > > manner. A better pledge is possible with more work. > > > > Does it make sense to get this one in now? > > Regress tests pass. I think this is the way to go. ok jca@ I just realized that we can do a stricter pledge with route instead of inet. Rob Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.53 diff -u -p -r1.53 ifstated.c --- ifstated.c 22 Jul 2017 19:52:01 - 1.53 +++ ifstated.c 22 Jul 2017 23:36:15 - @@ -159,6 +159,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath route proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL);
unused struct in dhcpd.h
This struct appears to be unused at the moment. Regards, Rob Index: dhcpd.h === RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v retrieving revision 1.64 diff -u -p -r1.64 dhcpd.h --- dhcpd.h 24 Apr 2017 14:58:36 - 1.64 +++ dhcpd.h 23 Jul 2017 15:19:18 - @@ -54,11 +54,6 @@ struct iaddr { unsigned char iabuf[16]; }; -struct iaddrlist { - struct iaddrlist *next; - struct iaddr addr; -}; - #define DEFAULT_HASH_SIZE 97 struct hash_bucket {
broken base build at src/usr/lib/libpcap?
My build breaks at usr/lib/libpcap: cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c pcap.c -o pcap.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c inet.c -o inet.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c gencode.c -o gencode.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c optimize.c -o optimize.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c nametoaddr.c -o nametoaddr.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c etherent.c -o etherent.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c savefile.c -o savefile.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c /usr/src/lib/libpcap/../../sys/net/bpf_filter.c -o bpf_filter.o cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c bpf_image.c -o bpf_image.o yacc -ppcap_yy -d grammar.y cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c -o grammar.o y.tab.c rm -f y.tab.c lex -Ppcap_yy scanner.l cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 -DHAVE_BSD_IEEE80211 -c -o scanner.o lex.yy.c cc: lex.yy.c: No such file or directory cc: no input files *** Error 1 in /usr/src/lib/libpcap (:196 'scanner.o')
Re: broken base build at src/usr/lib/libpcap?
> From: "Rob Pierce" > To: "tech" > Sent: Sunday, July 23, 2017 5:40:24 PM > Subject: broken base build at src/usr/lib/libpcap? > My build breaks at usr/lib/libpcap: Sorry, I jumped the gun. Works fine after another update.
Re: broken base build at src/usr/lib/libpcap?
> From: "Marc Espie" > To: "Rob Pierce" > Cc: "tech" > Sent: Sunday, July 23, 2017 7:36:29 PM > Subject: Re: broken base build at src/usr/lib/libpcap? > On Sun, Jul 23, 2017 at 05:40:24PM -0400, Rob Pierce wrote: > > yacc -ppcap_yy -d grammar.y >> cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval >> -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR >> -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 > > -DHAVE_BSD_IEEE80211 -c -o grammar.o y.tab.c > > rm -f y.tab.c > > lex -Ppcap_yy scanner.l >> cc -O2 -pipe -g -I. -I/usr/src/lib/libpcap -Dyylval=pcap_yylval >> -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON -DHAVE_STRERROR >> -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H -DINET6 > > -DHAVE_BSD_IEEE80211 -c -o scanner.o lex.yy.c > > cc: lex.yy.c: No such file or directory > > cc: no input files > Building part of source seldom goes well. > Especially WITHOUT updating share/mk first. Yes, sorry for the noise. All I needed to do was review the FAQ section on following -current: http://www.openbsd.org/faq/current.html Rob
pledge snmpctl
snmpclient pledges after calling chroot(2) and requires a dns promise for sendto(2) with non-NULL destination. Rob Index: snmpclient.c === RCS file: /cvs/src/usr.sbin/snmpctl/snmpclient.c,v retrieving revision 1.13 diff -u -p -r1.13 snmpclient.c --- snmpclient.c16 Jan 2015 06:40:21 - 1.13 +++ snmpclient.c25 Jul 2017 19:05:37 - @@ -160,6 +160,9 @@ snmpclient(struct parse_result *res) #endif } + if (pledge("stdio dns", NULL) == -1) + fatal("pledge"); + sc.sc_fd = s; sc.sc_community = res->community; sc.sc_version = res->version; Index: snmpctl.c === RCS file: /cvs/src/usr.sbin/snmpctl/snmpctl.c,v retrieving revision 1.22 diff -u -p -r1.22 snmpctl.c --- snmpctl.c 28 Oct 2016 20:49:32 - 1.22 +++ snmpctl.c 25 Jul 2017 19:05:37 - @@ -123,6 +123,8 @@ main(int argc, char *argv[]) usage(); break; case SHOW_MIB: + if (pledge("stdio", NULL) == -1) + fatal("pledge"); show_mib(); break; case WALK: @@ -131,6 +133,8 @@ main(int argc, char *argv[]) snmpclient(res); break; default: + if (pledge("stdio unix", NULL) == -1) + fatal("pledge"); goto connect; } @@ -155,6 +159,9 @@ main(int argc, char *argv[]) } err(1, "connect: %s", sock); } + + if (pledge("stdio", NULL) == -1) + fatal("pledge"); imsg_init(&ibuf, ctl_sock); done = 0;
ioctl under route promise for pledging snmpd
snmpe calls kif_update on an interface change which performs an ioctl with SIOCGIFDESCR, currently disallowed by pledge. No other network daemons do this. The only other programs that make this call appear to be ifconfig and systat. ifnet.if_description simply contains an optional user defined interface description. vmd performs an ioctl with SIOCSIFDESCR to set ifnet.if_description, and this is done in a privileged process that is not pledged. The following diff proposal allows for an ioctl on SIOCGIFDESCR under a route promise. Thoughts? Rob Index: kern_pledge.c === RCS file: /cvs/src/sys/kern/kern_pledge.c,v retrieving revision 1.216 diff -u -p -r1.216 kern_pledge.c --- kern_pledge.c 29 Jun 2017 04:10:07 - 1.216 +++ kern_pledge.c 26 Jul 2017 18:14:04 - @@ -1305,6 +1305,7 @@ pledge_ioctl(struct proc *p, long com, s if ((p->p_p->ps_pledge & PLEDGE_ROUTE)) { switch (com) { case SIOCGIFADDR: + case SIOCGIFDESCR: case SIOCGIFFLAGS: case SIOCGIFMETRIC: case SIOCGIFGMEMB:
ifstated diff: handling interface depature/arrival
Good evening all, Currently, ifstated does not detect the removal of an IFT_CARP pseudo device. As such, you can delete a carp interface and have ifstated happily remain in the current state without detecting any interface change. The reasons are two fold: 1. The routing socket is only monitored for RTM_IFINFO messages and therefore misses interface departure and arrival events. On an interface departure, for example, there is only one RTM_IFINFO message, and that particular message identifies the departed (carp) interface as up and master. 2. As I have recently discovered, LINK_STATE_UNKNOWN is equivalent to LINK_STATE_UP, so even if a removal was detected, the state change would go unnoticed (i.e. there would be no state change). I ran into this while trying to address (1). The following diff resolves this problem by adding RTM_IFANNOUNCE to the routing socket filter, and the route message handler now handles interface departures and arrivals. Upon departure, a rescan is performed with a (forced) state of LINK_STATE_DOWN, and upon the arrival of a previously configured pseudo device the interface is re-indexed and a rescan is performed with the actual interface state. I have also updated the regression test to cover carp interface departure and arrival (diff included for testing convenience). At this stage I am only looking for general comments on the approach and/or some testing if you are an ifstated user. For example, maybe it is unnecessary to save and check the interface type, assuming that interface departure and arrival will always be on a (supported) pseudo device (e.g. IFT_CARP), and actual physical interfaces will not behave in this manner. Also, ifi_type is not available via an ioctl under pledge, so obtaining this information dynamically through routing messages may be a bit of a hack. Thanks and regards, Rob Index: src/usr.sbin/ifstated/ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.56 diff -u -p -r1.56 ifstated.c --- src/usr.sbin/ifstated/ifstated.c24 Jul 2017 12:33:59 - 1.56 +++ src/usr.sbin/ifstated/ifstated.c31 Jul 2017 20:48:42 - @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -73,6 +74,8 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void set_iftype(int, u_char); +void reindex_if(int, char *); __dead void usage(void) @@ -149,7 +152,7 @@ main(int argc, char *argv[]) if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) err(1, "no routing socket"); - rtfilter = ROUTE_FILTER(RTM_IFINFO); + rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_IFANNOUNCE); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt msgfilter", __func__); @@ -231,24 +234,79 @@ void rt_msg_handler(int fd, short event, void *arg) { char msg[2048]; + char ifname[IFNAMSIZ]; struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; struct if_msghdr ifm; + struct if_announcemsghdr ifan; + struct ifaddrs *ifap, *ifa; ssize_t len; len = read(fd, msg, sizeof(msg)); /* XXX ignore errors? */ - if (len < sizeof(struct rt_msghdr)) + if ((size_t)len < sizeof(rtm->rtm_msglen) || len < rtm->rtm_msglen) return; if (rtm->rtm_version != RTM_VERSION) return; - if (rtm->rtm_type != RTM_IFINFO) - return; - - memcpy(&ifm, rtm, sizeof(ifm)); - scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + switch (rtm->rtm_type) { + /* +* We can't access ifi_type via an ioctl under pledge, but we can get +* it for free in an RTM_IFINFO and update ifstate accordingly. This is +* (may be?) required for reindexing, and reindexing isn't necessary +* until we have received an RTM_IFINFO message. +*/ + case RTM_IFINFO: + memcpy(&ifm, rtm, sizeof(ifm)); + set_iftype(ifm.ifm_index, ifm.ifm_data.ifi_type); + scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + break; + case RTM_IFANNOUNCE: + memcpy(&ifan, rtm, sizeof(ifan)); + switch (ifan.ifan_what) { + case IFAN_ARRIVAL: + /* +* We only reindex IFT_CARP pseudo devices that have +* already been configured, and then rescan with the +* actual interface state. +*/ + log_warnx("interface #%d arrived", ifan.ifan_index); +
Re: ifstated diff: handling interface depature/arrival
On Mon, Jul 31, 2017 at 05:59:46PM -0400, Rob Pierce wrote: > Good evening all, > > Currently, ifstated does not detect the removal of an IFT_CARP pseudo device. > As such, you can delete a carp interface and have ifstated happily remain in > the current state without detecting any interface change. > > The reasons are two fold: > > 1. The routing socket is only monitored for RTM_IFINFO messages and therefore >misses interface departure and arrival events. On an interface departure, >for example, there is only one RTM_IFINFO message, and that particular >message identifies the departed (carp) interface as up and master. > > 2. As I have recently discovered, LINK_STATE_UNKNOWN is equivalent to >LINK_STATE_UP, so even if a removal was detected, the state change would go >unnoticed (i.e. there would be no state change). I ran into this while >trying to address (1). > > The following diff resolves this problem by adding RTM_IFANNOUNCE to the > routing socket filter, and the route message handler now handles interface > departures and arrivals. Upon departure, a rescan is performed with a (forced) > state of LINK_STATE_DOWN, and upon the arrival of a previously configured > pseudo device the interface is re-indexed and a rescan is performed with the > actual interface state. > > I have also updated the regression test to cover carp interface departure and > arrival (diff included for testing convenience). > > At this stage I am only looking for general comments on the approach and/or > some testing if you are an ifstated user. > > For example, maybe it is unnecessary to save and check the interface type, > assuming that interface departure and arrival will always be on a (supported) > pseudo device (e.g. IFT_CARP), and actual physical interfaces will not behave > in this manner. Also, ifi_type is not available via an ioctl under pledge, so > obtaining this information dynamically through routing messages may be a bit > of a hack. > > Thanks and regards, > > Rob I need to rethink this approach. A departing interface must be handled appropriately, but the arrival of a new interface is problematic as we really need to confirm that the interface is configured exactly as expected (i.e. as it was prior to departure). Pledge currently prevents us from obtaining some of that information upon arrival for comparison purposes (e.g. SIOCGVH). This will also be an issue on reload under the current design, as interface parameters are not inherently addressed - only their state is considered. Maybe a simpler diff that only handles interface departure would be better, with the assumption that the interface will not come back, and indeed is not permitted to return after departure. This would at least ensure that a fail-over was handled appropriately within a pair of CARPed firewealls on interface departure. The departing interface would need to be re-configured and ifstated restarted in order for that host to return to a normal operating state. Rob > > Index: src/usr.sbin/ifstated/ifstated.c > === > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v > retrieving revision 1.56 > diff -u -p -r1.56 ifstated.c > --- src/usr.sbin/ifstated/ifstated.c 24 Jul 2017 12:33:59 - 1.56 > +++ src/usr.sbin/ifstated/ifstated.c 31 Jul 2017 20:48:42 - > @@ -28,6 +28,7 @@ > #include > > #include > +#include > #include > #include > > @@ -73,6 +74,8 @@ voiddo_action(struct ifsd_action *); > void remove_action(struct ifsd_action *, struct ifsd_state *); > void remove_expression(struct ifsd_expression *, > struct ifsd_state *); > +void set_iftype(int, u_char); > +void reindex_if(int, char *); > > __dead void > usage(void) > @@ -149,7 +152,7 @@ main(int argc, char *argv[]) > if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) > err(1, "no routing socket"); > > - rtfilter = ROUTE_FILTER(RTM_IFINFO); > + rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_IFANNOUNCE); > if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, > &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ > log_warn("%s: setsockopt msgfilter", __func__); > @@ -231,24 +234,79 @@ void > rt_msg_handler(int fd, short event, void *arg) > { > char msg[2048]; > + char ifname[IFNAMSIZ]; > struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; > struct if_msghdr ifm; > + struct if_announcemsghdr ifan; > + struct ifaddrs *ifap, *ifa; > ssize_t len; > > len = read(fd, msg, sizeof(msg)); >
dhcpd: remove unused structs
I can confirm that the following structs have not been used since at least OpenBSD 4.9. From Edgar Pettijohn. Ok? Index: dhcpd.h === RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v retrieving revision 1.65 diff -u -p -r1.65 dhcpd.h --- dhcpd.h 31 Jul 2017 19:00:40 - 1.65 +++ dhcpd.h 1 Aug 2017 22:41:34 - @@ -73,25 +73,6 @@ struct option_data { u_int8_t *data; }; -struct string_list { - struct string_list *next; - char *string; -}; - -/* A name server, from /etc/resolv.conf. */ -struct name_server { - struct name_server *next; - struct sockaddr_in addr; - time_t rcdate; -}; - -/* A domain search list element. */ -struct domain_search_list { - struct domain_search_list *next; - char *domain; - time_t rcdate; -}; - /* A dhcp packet and the pointers to its option values. */ struct packet { struct dhcp_packet *raw; @@ -294,12 +275,6 @@ struct interface_info { int is_udpsock; ssize_t (*send_packet)(struct interface_info *, struct dhcp_packet *, size_t, struct in_addr, struct sockaddr_in *, struct hardware *); -}; - -struct hardware_link { - struct hardware_link *next; - char name[IFNAMSIZ]; - struct hardware address; }; struct dhcpd_timeout {
ifstated: consistent use of log.c
As a result ifstated.c no longer needs err.h. Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.56 diff -u -p -r1.56 ifstated.c --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 +++ ifstated.c 3 Aug 2017 23:59:13 - @@ -37,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -102,7 +101,7 @@ main(int argc, char *argv[]) break; case 'D': if (cmdline_symset(optarg) < 0) - errx(1, "could not parse macro definition %s", + fatalx("could not parse macro definition %s", optarg); break; case 'f': @@ -135,7 +134,7 @@ main(int argc, char *argv[]) if (opts & IFSD_OPT_NOACTION) { if ((newconf = parse_config(configfile, opts)) == NULL) exit(1); - warnx("configuration OK"); + fprintf(stderr, "configuration OK\n"); exit(0); } @@ -147,7 +146,7 @@ main(int argc, char *argv[]) log_setverbose(opts & IFSD_OPT_VERBOSE); if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) - err(1, "no routing socket"); + fatal("no routing socket"); rtfilter = ROUTE_FILTER(RTM_IFINFO); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, @@ -604,7 +603,7 @@ fetch_ifstate(void) struct ifaddrs *ifap, *ifa; if (getifaddrs(&ifap) != 0) - err(1, "getifaddrs"); + fatal("getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { if (ifa->ifa_addr->sa_family == AF_LINK) {
switchd: no need to include err.h
switchd no longer requires err.h. Index: packet.c === RCS file: /cvs/src/usr.sbin/switchd/packet.c,v retrieving revision 1.4 diff -u -p -r1.4 packet.c --- packet.c26 Sep 2016 08:55:43 - 1.4 +++ packet.c4 Aug 2017 00:03:59 - @@ -29,7 +29,6 @@ #include #include #include -#include #include #include Index: parse.y === RCS file: /cvs/src/usr.sbin/switchd/parse.y,v retrieving revision 1.4 diff -u -p -r1.4 parse.y --- parse.y 5 Jan 2017 12:42:19 - 1.4 +++ parse.y 4 Aug 2017 00:03:59 - @@ -29,7 +29,6 @@ #include #include -#include #include #include #include
ifstated: improve routing socket error handling
Improve error checking during processing of routing messages. Handling of RTM_DESYNC encouraged by deraadt. Regression tests pass. I have another diff ready to go that handles interface depature, but I thought it best to separate them. Ok? Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.56 diff -u -p -r1.56 ifstated.c --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 +++ ifstated.c 6 Aug 2017 03:49:54 - @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -63,7 +64,7 @@ void check_external_status(struct ifsd_ void external_evtimer_setup(struct ifsd_state *, int); void scan_ifstate(int, int, int); intscan_ifstate_single(int, int, struct ifsd_state *); -void fetch_ifstate(void); +void fetch_ifstate(int); __dead voidusage(void); void adjust_expressions(struct ifsd_expression_list *, int); void adjust_external_expressions(struct ifsd_state *); @@ -210,7 +211,7 @@ load_config(void) clear_config(conf); conf = newconf; conf->initstate.entered = time(NULL); - fetch_ifstate(); + fetch_ifstate(0); external_evtimer_setup(&conf->initstate, IFSD_EVTIMER_ADD); adjust_external_expressions(&conf->initstate); eval_state(&conf->initstate); @@ -235,20 +236,30 @@ rt_msg_handler(int fd, short event, void struct if_msghdr ifm; ssize_t len; - len = read(fd, msg, sizeof(msg)); + if ((len = read(fd, msg, sizeof(msg))) == -1) { + if (errno == EAGAIN || errno == EINTR) + return; + fatal("%s: routing socket read error", __func__); + } - /* XXX ignore errors? */ - if (len < sizeof(struct rt_msghdr)) - return; + if (len == 0) + fatal("%s: routing socket closed", __func__); if (rtm->rtm_version != RTM_VERSION) return; - if (rtm->rtm_type != RTM_IFINFO) - return; - - memcpy(&ifm, rtm, sizeof(ifm)); - scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + switch (rtm->rtm_type) { + case RTM_IFINFO: + memcpy(&ifm, rtm, sizeof(ifm)); + scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + break; + case RTM_DESYNC: + fetch_ifstate(1); + break; + default: + break; + } + return; } void @@ -599,7 +610,7 @@ do_action(struct ifsd_action *action) * Fetch the current link states. */ void -fetch_ifstate(void) +fetch_ifstate(int do_eval) { struct ifaddrs *ifap, *ifa; @@ -610,7 +621,7 @@ fetch_ifstate(void) if (ifa->ifa_addr->sa_family == AF_LINK) { struct if_data *ifdata = ifa->ifa_data; scan_ifstate(if_nametoindex(ifa->ifa_name), - ifdata->ifi_link_state, 0); + ifdata->ifi_link_state, do_eval); } }
ifstated: add handing of departed interfaces
The following diff adds support for detecting the state change of a departed interface. ifstated is not a very verbose daemon, so this diff quietly does the right thing (i.e. there is no exttra warning about a departing interface). The re-arrival of a departed interface involves re-indexing the interface and possibly other complexities that require more consideration, but for now at least this obvious condition is handled in what I believe is a more appropriate manner. Updated regression tests pass, and the corresponding regression diff is also attached. Ok? Index: regress/usr.sbin/ifstated/ifstated === RCS file: /cvs/src/regress/usr.sbin/ifstated/ifstated,v retrieving revision 1.3 diff -u -p -r1.3 ifstated --- regress/usr.sbin/ifstated/ifstated 31 Jul 2017 18:41:21 - 1.3 +++ regress/usr.sbin/ifstated/ifstated 6 Aug 2017 23:29:11 - @@ -124,6 +124,7 @@ changing state to primary changing state to demoted changing state to primary changing state to primary +changing state to demoted EOF (cd working && nohup ifstated -dvf ./ifstated.conf > ifstated.log 2>&1) & @@ -148,6 +149,8 @@ ifconfig carp${VHIDB} inet ${PREFIX}.${V ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} sleep ${SLEEP} kill -HUP $(pgrep ifstated) >/dev/null 2>&1 +sleep ${SLEEP} +ifconfig carp${VHIDA} destroy sleep ${SLEEP} grep ^changing working/ifstated.log > working/output.new Index: usr.sbin/ifstated/ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.57 diff -u -p -r1.57 ifstated.c --- usr.sbin/ifstated/ifstated.c6 Aug 2017 19:27:54 - 1.57 +++ usr.sbin/ifstated/ifstated.c6 Aug 2017 23:29:12 - @@ -1,4 +1,4 @@ -/* $OpenBSD: ifstated.c,v 1.57 2017/08/06 19:27:54 rob Exp $ */ +/* $OpenBSD: ifstated.c,v 1.56 2017/07/24 12:33:59 jca Exp $ */ /* * Copyright (c) 2004 Marco Pfatschbacher @@ -61,6 +61,7 @@ void rt_msg_handler(int, short, void *) void external_handler(int, short, void *); void external_exec(struct ifsd_external *, int); void check_external_status(struct ifsd_state *); +void check_for_ifdeparture(void); void external_evtimer_setup(struct ifsd_state *, int); void scan_ifstate(int, int, int); intscan_ifstate_single(int, int, struct ifsd_state *); @@ -150,7 +151,7 @@ main(int argc, char *argv[]) if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) err(1, "no routing socket"); - rtfilter = ROUTE_FILTER(RTM_IFINFO); + rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_IFANNOUNCE); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt msgfilter", __func__); @@ -234,6 +235,7 @@ rt_msg_handler(int fd, short event, void char msg[2048]; struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; struct if_msghdr ifm; + struct if_announcemsghdr ifan; ssize_t len; if ((len = read(fd, msg, sizeof(msg))) == -1) { @@ -253,8 +255,19 @@ rt_msg_handler(int fd, short event, void memcpy(&ifm, rtm, sizeof(ifm)); scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); break; + case RTM_IFANNOUNCE: + memcpy(&ifan, rtm, sizeof(ifan)); + switch (ifan.ifan_what) { + case IFAN_DEPARTURE: + scan_ifstate(ifan.ifan_index, LINK_STATE_DOWN, 1); + break; + default: + break; + } + break; case RTM_DESYNC: fetch_ifstate(1); + check_for_ifdeparture(); break; default: break; @@ -626,6 +639,26 @@ fetch_ifstate(int do_eval) } freeifaddrs(ifap); +} + +void +check_for_ifdeparture(void) +{ + struct ifsd_state *state; + struct ifsd_ifstate *ifstate; + char ifnamebuf[IF_NAMESIZE]; + char *if_name; + + TAILQ_FOREACH(state, &conf->states, entries) { + TAILQ_FOREACH(ifstate, &state->interface_states, entries) { + if_name = if_indextoname(ifstate->ifindex, ifnamebuf); + if (if_name == NULL) { + scan_ifstate(ifstate->ifindex, + LINK_STATE_DOWN, 1); + } + } + } + return; } void
Re: ifstated: consistent use of log.c
On Sun, Aug 06, 2017 at 06:47:38PM +0200, Jeremie Courreges-Anglas wrote: > On Thu, Aug 03 2017, Rob Pierce wrote: > > As a result ifstated.c no longer needs err.h. > > > > Index: ifstated.c > > === > > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v > > retrieving revision 1.56 > > diff -u -p -r1.56 ifstated.c > > --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 > > +++ ifstated.c 3 Aug 2017 23:59:13 - > > @@ -37,7 +37,6 @@ > > #include > > #include > > #include > > -#include > > #include > > #include > > #include > > @@ -102,7 +101,7 @@ main(int argc, char *argv[]) > > break; > > case 'D': > > if (cmdline_symset(optarg) < 0) > > - errx(1, "could not parse macro definition %s", > > + fatalx("could not parse macro definition %s", > > optarg); > > break; > > case 'f': > > @@ -135,7 +134,7 @@ main(int argc, char *argv[]) > > if (opts & IFSD_OPT_NOACTION) { > > if ((newconf = parse_config(configfile, opts)) == NULL) > > exit(1); > > - warnx("configuration OK"); > > + fprintf(stderr, "configuration OK\n"); > > This changes the output from > > ifstated: configuration OK > > to > > configuration OK > > which is a bit less helpful. To keep the output the same, you could use > warnx, fprintf + __progname/getprogname instead, or just > > errx(0, "configuration OK"); > > I don't really have a preference here. Good catch. I honestly didn't notice the change in output that was introduced by my diff. I was following the code in other networking daemons. I would like to standardize on log.c and remove the err.h include, so maybe the __progname approach is best if we want to keep the same output, or maybe having a few lingering uses of err.h is ok. Many network daemons (below) do not reference themselves when confirming the validity of their configuration file with the -n option. Is it better for ifstated to retain it's historical output on the configuration check, or to be made more consistent with other network daemons? Regards, Rob bgpd/bgpd.c:fprintf(stderr, "configuration OK\n"); dvmrpd/dvmrpd.c:fprintf(stderr, "configuration OK\n"); eigrpd/eigrpd.c:fprintf(stderr, "configuration OK\n"); httpd/httpd.c: fprintf(stderr, "configuration OK\n"); ldapd/ldapd.c: fprintf(stderr, "configuration ok\n"); ldpd/ldpd.c:fprintf(stderr, "configuration OK\n"); ntpd/ntpd.c:fprintf(stderr, "configuration OK\n"); ospf6d/ospf6d.c:fprintf(stderr, "configuration OK\n"); ospfd/ospfd.c: fprintf(stderr, "configuration OK\n"); radiusd/radiusd.c: fprintf(stderr, "configuration OK\n"); relayd/relayd.c:fprintf(stderr, "configuration OK\n"); ripd/ripd.c:fprintf(stderr, "configuration OK\n"); sasyncd/sasyncd.c: fprintf(stderr, "configuration OK\n"); smtpd/smtpd.c: fprintf(stderr, "configuration OK\n"); snmpd/snmpd.c: fprintf(stderr, "configuration ok\n"); switchd/switchd.c: fprintf(stderr, "configuration OK\n"); vmd/vmd.c: fprintf(stderr, "configuration OK\n"); ypldap/ypldap.c:fprintf(stderr, "configuration OK\n"); > > exit(0); > > } > > > > @@ -147,7 +146,7 @@ main(int argc, char *argv[]) > > log_setverbose(opts & IFSD_OPT_VERBOSE); > > > > if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) > > - err(1, "no routing socket"); > > + fatal("no routing socket"); > > > > rtfilter = ROUTE_FILTER(RTM_IFINFO); > > if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, > > @@ -604,7 +603,7 @@ fetch_ifstate(void) > > struct ifaddrs *ifap, *ifa; > > > > if (getifaddrs(&ifap) != 0) > > - err(1, "getifaddrs"); > > + fatal("getifaddrs"); > > > > for (ifa = ifap; ifa; ifa = ifa->ifa_next) { > > if (ifa->ifa_addr->sa_family == AF_LINK) { > > > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
fix inconsistency in ifstated.8
All other network daemon man pages reference Configtest as one word. Ok? Index: ifstated.8 === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.8,v retrieving revision 1.9 diff -u -p -r1.9 ifstated.8 --- ifstated.8 25 Sep 2015 14:27:26 - 1.9 +++ ifstated.8 8 Aug 2017 01:24:40 - @@ -61,7 +61,7 @@ Print help message. .It Fl i Ignore initial interface states. .It Fl n -Config test mode. +Configtest mode. Check config validity, then exit. .It Fl v Verbose mode.
Re: ifstated: add handing of departed interfaces
On Tue, Aug 08, 2017 at 12:12:43AM +0200, Jeremie Courreges-Anglas wrote: > On Sun, Aug 06 2017, Rob Pierce wrote: > > The following diff adds support for detecting the state change of a departed > > interface. ifstated is not a very verbose daemon, so this diff quietly does > > the right thing (i.e. there is no exttra warning about a departing > > interface). > > But maybe there should be at least a big scary message. This is not > exactly a normal situation. Yes, I agree. This is a special case. > > The re-arrival of a departed interface involves re-indexing the interface > > and > > possibly other complexities that require more consideration, but for now at > > least this obvious condition is handled in what I believe is a more > > appropriate manner. > > I wonder what's the most useful behavior here: complain loudly and fail > hard (exit) or just consider the link down and monitor the rest of the > remaining interfaces, as in your diff. I do like the idea of complaining loudly and failing hard. I had originally considered a fatal (after forcing a demote of course). > Destroying then reconfiguring a carp(4) interface won't give you a very > user-friendly behavior. Same thing with disconnecting/reconnecting > interfaces like urtwn(4) or urndis(4). ifstated(8) won't do anything > about your new interface, even if you checked that said interface was > in good shape. > > Also if we keep running, the link is now considered down and we're > likely to execute commands that refer to an interface that has left; > that might be good or bad, I don't know what people put in their > ifstated.conf. > > > Updated regression tests pass, and the corresponding regression diff is also > > attached. > > > > Ok? > > I'm not sure yet which path we should follow. Let's discuss this in > Toronto, shal we? In the meantime, please see the nits below. Sounds like a plan. Looking forward to it! Rob > > Index: regress/usr.sbin/ifstated/ifstated > > === > > RCS file: /cvs/src/regress/usr.sbin/ifstated/ifstated,v > > retrieving revision 1.3 > > diff -u -p -r1.3 ifstated > > --- regress/usr.sbin/ifstated/ifstated 31 Jul 2017 18:41:21 - > > 1.3 > > +++ regress/usr.sbin/ifstated/ifstated 6 Aug 2017 23:29:11 - > > @@ -124,6 +124,7 @@ changing state to primary > > changing state to demoted > > changing state to primary > > changing state to primary > > +changing state to demoted > > EOF > > > > (cd working && nohup ifstated -dvf ./ifstated.conf > ifstated.log 2>&1) & > > @@ -148,6 +149,8 @@ ifconfig carp${VHIDB} inet ${PREFIX}.${V > > ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} > > sleep ${SLEEP} > > kill -HUP $(pgrep ifstated) >/dev/null 2>&1 > > +sleep ${SLEEP} > > +ifconfig carp${VHIDA} destroy > > sleep ${SLEEP} > > > > grep ^changing working/ifstated.log > working/output.new > > Index: usr.sbin/ifstated/ifstated.c > > === > > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v > > retrieving revision 1.57 > > diff -u -p -r1.57 ifstated.c > > --- usr.sbin/ifstated/ifstated.c6 Aug 2017 19:27:54 - 1.57 > > +++ usr.sbin/ifstated/ifstated.c6 Aug 2017 23:29:12 - > > @@ -1,4 +1,4 @@ > > -/* $OpenBSD: ifstated.c,v 1.57 2017/08/06 19:27:54 rob Exp $ */ > > +/* $OpenBSD: ifstated.c,v 1.56 2017/07/24 12:33:59 jca Exp $ */ > > > > /* > > * Copyright (c) 2004 Marco Pfatschbacher > > @@ -61,6 +61,7 @@ void rt_msg_handler(int, short, void *) > > void external_handler(int, short, void *); > > void external_exec(struct ifsd_external *, int); > > void check_external_status(struct ifsd_state *); > > +void check_for_ifdeparture(void); > > check_ifdeparture() would be shorter and just as descriptive. > > > void external_evtimer_setup(struct ifsd_state *, int); > > void scan_ifstate(int, int, int); > > intscan_ifstate_single(int, int, struct ifsd_state *); > > @@ -150,7 +151,7 @@ main(int argc, char *argv[]) > > if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) > > err(1, "no routing socket"); > > > > - rtfilter = ROUTE_FILTER(RTM_IFINFO); > > + rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_IFANNOUNCE); > > if (setsockopt(rt_fd, PF_ROU
bsd.regress.mk.5 typo fix
I just ran across this. Ok? Index: bsd.regress.mk.5 === RCS file: /cvs/src/share/man/man5/bsd.regress.mk.5,v retrieving revision 1.14 diff -u -p -r1.14 bsd.regress.mk.5 --- bsd.regress.mk.53 Jul 2017 18:19:55 - 1.14 +++ bsd.regress.mk.511 Aug 2017 16:39:12 - @@ -155,7 +155,7 @@ to run before, this directory or symlink may not exist. Then the test should run anyway. .Pp -Test are executed with +Tests are executed with .Sy make Cm regress , but running .Sy make Cm all
diff: pledge snmpd
The following diff pledges two of three processes in snmpd: the parent snmpd process and the trap handler. We cannot currently pledge snmpe as snmp requests asking for privileged kernel info are disallowed by pledge. I have included a commented pledge block in snmpe.c below (which will not be committed) which I believe would be possible if we moved the code that violates pledge to another unpledged process. If we think that is worth while I could pursue it further. In the mean time I am looking for comments and/or ok's for the snmpd.c and traphandler.c diffs below. This passes the newly committed snmpd regression tests. Regards, Rob Index: snmpd.c === RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v retrieving revision 1.36 diff -u -p -r1.36 snmpd.c --- snmpd.c 4 Apr 2017 02:37:15 - 1.36 +++ snmpd.c 11 Aug 2017 20:10:50 - @@ -255,6 +255,9 @@ main(int argc, char *argv[]) proc_connect(ps); + if (pledge("stdio rpath cpath sendfd dns proc exec id", NULL) == -1) + fatal("pledge"); + event_dispatch(); log_debug("%d parent exiting", getpid()); Index: snmpe.c === RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v retrieving revision 1.48 diff -u -p -r1.48 snmpe.c --- snmpe.c 27 Jul 2017 14:04:16 - 1.48 +++ snmpe.c 11 Aug 2017 20:10:50 - @@ -105,6 +105,10 @@ snmpe_init(struct privsep *ps, struct pr snmpe_recvmsg, env); event_add(&so->s_ev, NULL); } +/* + if (pledge("stdio recvfd inet vminfo route", NULL) == -1) + fatal("pledge"); + */ } void Index: traphandler.c === RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v retrieving revision 1.8 diff -u -p -r1.8 traphandler.c --- traphandler.c 9 Jan 2017 14:49:22 - 1.8 +++ traphandler.c 11 Aug 2017 20:10:50 - @@ -96,6 +96,9 @@ traphandler_init(struct privsep *ps, str struct snmpd*env = ps->ps_env; struct listen_sock *so; + if (pledge("stdio recvfd proc exec id", NULL) == -1) + fatal("pledge"); + if (!env->sc_traphandler) return;
ifstated: stop tracking interface indexes
ifstated currently tracks and maintains the index of each monitored interface and does not maintain interface names. This means we need to re-index on interface departure and arrival. The following diff moves away from indexes to names. Indexes are still required, but easily obtained dynamically as needed. This helps simplify the next diff that will provide support for interface departure and arrival. Suggested by deraadt. No intended functional change. Regress tests pass. Ok? Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.59 diff -u -p -r1.59 ifstated.c --- ifstated.c 14 Aug 2017 03:15:28 - 1.59 +++ ifstated.c 15 Aug 2017 00:34:11 - @@ -61,8 +61,8 @@ void external_handler(int, short, void void external_exec(struct ifsd_external *, int); void check_external_status(struct ifsd_state *); void external_evtimer_setup(struct ifsd_state *, int); -void scan_ifstate(int, int, int); -intscan_ifstate_single(int, int, struct ifsd_state *); +void scan_ifstate(const char *, int, int); +intscan_ifstate_single(const char *, int, struct ifsd_state *); void fetch_ifstate(int); __dead voidusage(void); void adjust_expressions(struct ifsd_expression_list *, int); @@ -233,6 +233,8 @@ rt_msg_handler(int fd, short event, void char msg[2048]; struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; struct if_msghdr ifm; + char ifnamebuf[IFNAMSIZ]; + char *ifname; ssize_t len; if ((len = read(fd, msg, sizeof(msg))) == -1) { @@ -250,7 +252,10 @@ rt_msg_handler(int fd, short event, void switch (rtm->rtm_type) { case RTM_IFINFO: memcpy(&ifm, rtm, sizeof(ifm)); - scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); + ifname = if_indextoname(ifm.ifm_index, ifnamebuf); + /* ifname is NULL on interface departure */ + if (ifname != NULL) + scan_ifstate(ifname, ifm.ifm_data.ifi_link_state, 1); break; case RTM_DESYNC: fetch_ifstate(1); @@ -431,7 +436,7 @@ external_evtimer_setup(struct ifsd_state #defineLINK_STATE_IS_DOWN(_s) (!LINK_STATE_IS_UP((_s))) int -scan_ifstate_single(int ifindex, int s, struct ifsd_state *state) +scan_ifstate_single(const char *ifname, int s, struct ifsd_state *state) { struct ifsd_ifstate *ifstate; struct ifsd_expression_list expressions; @@ -440,7 +445,7 @@ scan_ifstate_single(int ifindex, int s, TAILQ_INIT(&expressions); TAILQ_FOREACH(ifstate, &state->interface_states, entries) { - if (ifstate->ifindex == ifindex) { + if (strcmp(ifstate->ifname, ifname) == 0) { if (ifstate->prevstate != s && (ifstate->prevstate != -1 || !opt_inhibit)) { struct ifsd_expression *expression; @@ -472,15 +477,15 @@ scan_ifstate_single(int ifindex, int s, } void -scan_ifstate(int ifindex, int s, int do_eval) +scan_ifstate(const char *ifname, int s, int do_eval) { struct ifsd_state *state; int cur_eval = 0; - if (scan_ifstate_single(ifindex, s, &conf->initstate) && do_eval) + if (scan_ifstate_single(ifname, s, &conf->initstate) && do_eval) eval_state(&conf->initstate); TAILQ_FOREACH(state, &conf->states, entries) { - if (scan_ifstate_single(ifindex, s, state) && + if (scan_ifstate_single(ifname, s, state) && (do_eval && state == conf->curstate)) cur_eval = 1; } @@ -619,8 +624,8 @@ fetch_ifstate(int do_eval) for (ifa = ifap; ifa; ifa = ifa->ifa_next) { if (ifa->ifa_addr->sa_family == AF_LINK) { struct if_data *ifdata = ifa->ifa_data; - scan_ifstate(if_nametoindex(ifa->ifa_name), - ifdata->ifi_link_state, do_eval); + scan_ifstate(ifa->ifa_name, ifdata->ifi_link_state, + do_eval); } } Index: ifstated.h === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v retrieving revision 1.18 diff -u -p -r1.18 ifstated.h --- ifstated.h 14 Aug 2017 03:15:28 - 1.18 +++ ifstated.h 15 Aug 2017 00:34:11 - @@ -41,7 +41,7 @@ struct ifsd_ifstate { #define IFSD_LINKUP2 int prevstate; u_int32_trefcount; - u_short ifindex; + char ifname[IFNAMSIZ]; }; struct ifsd_external { Index: parse.y
Re: ksh(1) history lines allocation
On Mon, Aug 14, 2017 at 10:26:48PM -0400, Jeremie Courreges-Anglas wrote: > > So I tinkered with the way ksh(1) tracks memory allocation, trying to > make it faster in the general case. One approach used a RB tree, > I wrote since a simple hash table implementation which seems to work > rather well. > > But the actual problem I'd first like to solve is a corner case. I use > HISTSIZE=2, and when the actual line count in my histfile approaches > 25000 (1.25 * HISTSIZE), ksh(1) has a hard time handling it. The main > reason is that it calls afree() ~5000 times in a loop, with afree() > traversing the APERM freelist, which contains >2 elements. This is > expensive. > > For history lines, we don't actually need to keep track of allocations > using an area, history lines are private to history.c and no gc/whatever > is needed there. So here's a diff that just uses strdup(3)/free(3). > > Comments? ok? I was able to reproduce the problem with a HISTSIZE of 10 which at 125000 entries rendered my system unusable. With the patch I am running fine with a HISTSIZE of 12 and have come back several times after hitting the 1.25x threshold. Regression tests pass. Rob > Index: history.c > === > RCS file: /d/cvs/src/bin/ksh/history.c,v > retrieving revision 1.64 > diff -u -p -p -u -r1.64 history.c > --- history.c 11 Aug 2017 19:37:58 - 1.64 > +++ history.c 15 Aug 2017 01:14:58 - > @@ -428,7 +428,7 @@ histbackup(void) > > if (histptr >= history && last_line != hist_source->line) { > hist_source->line--; > - afree(*histptr, APERM); > + free(*histptr); > histptr--; > last_line = hist_source->line; > } > @@ -613,14 +613,15 @@ histsave(int lno, const char *cmd, int d > #endif > } > > - c = str_save(cmd, APERM); > + if ((c = strdup(cmd)) == NULL) > + internal_errorf(1, "unable to allocate memory"); > if ((cp = strrchr(c, '\n')) != NULL) > *cp = '\0'; > > if (histptr < history + histsize - 1) > histptr++; > else { /* remove oldest command */ > - afree(*history, APERM); > + free(*history); > memmove(history, history + 1, > (histsize - 1) * sizeof(*history)); > } > > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE >
Re: ifstated: stop tracking interface indexes
On Mon, Aug 14, 2017 at 11:26:46PM -0400, Jeremie Courreges-Anglas wrote: > On Mon, Aug 14 2017, Rob Pierce wrote: > > ifstated currently tracks and maintains the index of each monitored > > interface > > and does not maintain interface names. This means we need to re-index on > > interface departure and arrival. > > > > The following diff moves away from indexes to names. Indexes are still > > required, > > but easily obtained dynamically as needed. This helps simplify the next diff > > that will provide support for interface departure and arrival. > > > > Suggested by deraadt. > > > > No intended functional change. Regress tests pass. > > > > Ok? > > The idea looks sound to me, however I would keep the "interface" symbol > in parse.y (your diff doesn't remove all "interface" references btw). > > The current code checks the existence of the interface at startup. If > the interface doesn't exists, you get a syntax error. This could happen > because of a missing interface (an interesting case), or because of > a typo. Whether or not we're erroring out, it is nice to print > a diagnostic message. > > I'm not sure this change was intended, so here's a tentative diff that > that keeps the existing behavior. Regress tests pass. Yes, I see that now. That change was not intended. Thanks! Your diff looks good. Ok? > Index: ifstated.c > === > RCS file: /d/cvs/src/usr.sbin/ifstated/ifstated.c,v > retrieving revision 1.59 > diff -u -p -r1.59 ifstated.c > --- ifstated.c14 Aug 2017 03:15:28 - 1.59 > +++ ifstated.c15 Aug 2017 03:04:47 - > @@ -61,8 +61,8 @@ voidexternal_handler(int, short, void > void external_exec(struct ifsd_external *, int); > void check_external_status(struct ifsd_state *); > void external_evtimer_setup(struct ifsd_state *, int); > -void scan_ifstate(int, int, int); > -int scan_ifstate_single(int, int, struct ifsd_state *); > +void scan_ifstate(const char *, int, int); > +int scan_ifstate_single(const char *, int, struct ifsd_state *); > void fetch_ifstate(int); > __dead void usage(void); > void adjust_expressions(struct ifsd_expression_list *, int); > @@ -233,6 +233,8 @@ rt_msg_handler(int fd, short event, void > char msg[2048]; > struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; > struct if_msghdr ifm; > + char ifnamebuf[IFNAMSIZ]; > + char *ifname; > ssize_t len; > > if ((len = read(fd, msg, sizeof(msg))) == -1) { > @@ -250,7 +252,10 @@ rt_msg_handler(int fd, short event, void > switch (rtm->rtm_type) { > case RTM_IFINFO: > memcpy(&ifm, rtm, sizeof(ifm)); > - scan_ifstate(ifm.ifm_index, ifm.ifm_data.ifi_link_state, 1); > + ifname = if_indextoname(ifm.ifm_index, ifnamebuf); > + /* ifname is NULL on interface departure */ > + if (ifname != NULL) > + scan_ifstate(ifname, ifm.ifm_data.ifi_link_state, 1); > break; > case RTM_DESYNC: > fetch_ifstate(1); > @@ -431,7 +436,7 @@ external_evtimer_setup(struct ifsd_state > #define LINK_STATE_IS_DOWN(_s) (!LINK_STATE_IS_UP((_s))) > > int > -scan_ifstate_single(int ifindex, int s, struct ifsd_state *state) > +scan_ifstate_single(const char *ifname, int s, struct ifsd_state *state) > { > struct ifsd_ifstate *ifstate; > struct ifsd_expression_list expressions; > @@ -440,7 +445,7 @@ scan_ifstate_single(int ifindex, int s, > TAILQ_INIT(&expressions); > > TAILQ_FOREACH(ifstate, &state->interface_states, entries) { > - if (ifstate->ifindex == ifindex) { > + if (strcmp(ifstate->ifname, ifname) == 0) { > if (ifstate->prevstate != s && > (ifstate->prevstate != -1 || !opt_inhibit)) { > struct ifsd_expression *expression; > @@ -472,15 +477,15 @@ scan_ifstate_single(int ifindex, int s, > } > > void > -scan_ifstate(int ifindex, int s, int do_eval) > +scan_ifstate(const char *ifname, int s, int do_eval) > { > struct ifsd_state *state; > int cur_eval = 0; > > - if (scan_ifstate_single(ifindex, s, &conf->initstate) && do_eval) > + if (scan_ifstate_single(ifname, s, &conf->initstate) && do_eval) > eval_state(&conf->initstate); > TAILQ_FOREACH(state, &conf-&g
Re: ifstated: stop tracking interface indexes
On Tue, Aug 15, 2017 at 02:37:22PM -0400, Jeremie Courreges-Anglas wrote: > On Tue, Aug 15 2017, Rob Pierce wrote: > > On Mon, Aug 14, 2017 at 11:26:46PM -0400, Jeremie Courreges-Anglas wrote: > >> On Mon, Aug 14 2017, Rob Pierce wrote: > >> > ifstated currently tracks and maintains the index of each monitored > >> > interface > >> > and does not maintain interface names. This means we need to re-index on > >> > interface departure and arrival. > >> > > >> > The following diff moves away from indexes to names. Indexes are still > >> > required, > >> > but easily obtained dynamically as needed. This helps simplify the next > >> > diff > >> > that will provide support for interface departure and arrival. > >> > > >> > Suggested by deraadt. > >> > > >> > No intended functional change. Regress tests pass. > >> > > >> > Ok? > >> > >> The idea looks sound to me, however I would keep the "interface" symbol > >> in parse.y (your diff doesn't remove all "interface" references btw). > >> > >> The current code checks the existence of the interface at startup. If > >> the interface doesn't exists, you get a syntax error. This could happen > >> because of a missing interface (an interesting case), or because of > >> a typo. Whether or not we're erroring out, it is nice to print > >> a diagnostic message. > >> > >> I'm not sure this change was intended, so here's a tentative diff that > >> that keeps the existing behavior. Regress tests pass. > > > > Yes, I see that now. That change was not intended. Thanks! > > > > Your diff looks good. > > > > Ok? > > Please use four spaces, not three, for second level indents; see > style(9). > > There's a check that should be fixed, please see below. Ok, will do. Not sure why I started using three spaces for 2nd level indents... > With those items addressed, ok jca@ > > It feels a bit weird to reject unknown interface names at startup but to > cope with departed/joining interfaces at runtime. But I guess we'll > have to live with this. I don't disagree. I am most concerned about handling the departure condition more appropriately, which is (in my opinion) currently broken. As discussed, it is an option to do the right thing on departure (i.e. demote with a state change to down) and then fail hard, which would mirror the (current) startup behaviour more closely (i.e. fail on startup due to the interfaces not being present). I suppose we could also allow startup with unknown interfaces (present in the configuration file) to occur (which would be a change in behaviour), and simply mark those interfaces as down and immediately demote but keep running, like we do for an interface that is present at startup but in a down state. This diff does not yet introduce interface departure and arrival, and with your updates will not change any behaviour, and I think this change makes sense regardless of which approach is taken in the next step. Thank you for your thorough review and comments. I will update my diff accordingly and wait a few days for any further comments before committing. Regards, Rob > >> Index: ifstated.c > >> === > >> RCS file: /d/cvs/src/usr.sbin/ifstated/ifstated.c,v > >> retrieving revision 1.59 > >> diff -u -p -r1.59 ifstated.c > >> --- ifstated.c 14 Aug 2017 03:15:28 - 1.59 > >> +++ ifstated.c 15 Aug 2017 03:04:47 - > >> @@ -61,8 +61,8 @@ void external_handler(int, short, void > >> void external_exec(struct ifsd_external *, int); > >> void check_external_status(struct ifsd_state *); > >> void external_evtimer_setup(struct ifsd_state *, int); > >> -void scan_ifstate(int, int, int); > >> -int scan_ifstate_single(int, int, struct ifsd_state *); > >> +void scan_ifstate(const char *, int, int); > >> +int scan_ifstate_single(const char *, int, struct > >> ifsd_state *); > >> void fetch_ifstate(int); > >> __dead void usage(void); > >> void adjust_expressions(struct ifsd_expression_list *, int); > >> @@ -233,6 +233,8 @@ rt_msg_handler(int fd, short event, void > >>char msg[2048]; > >>struct rt_msghdr *rtm = (struct rt_msghdr *)&msg; > >>struct if_msghdr ifm; > >&g
Re: ksh(1) history lines allocation
On Tue, Aug 15, 2017 at 02:03:43PM -0400, Jeremie Courreges-Anglas wrote: > On Tue, Aug 15 2017, Rob Pierce wrote: > > [...] > > > I was able to reproduce the problem with a HISTSIZE of 10 which at > > 125000 > > entries rendered my system unusable. With the patch I am running fine with a > > HISTSIZE of 12 and have come back several times after hitting the 1.25x > > threshold. > > > > Regression tests pass. > > Thanks for your feedback. As I said privately, I did a bad job at > analyzing how to fix the slowness of afree(). With latest commit to > alloc.c, switching history lines allocation to strdup(3)/free(3) is not > needed any more; also it is probably better for us to use the same > allocation pattern everywhere. Sounds good. Your latest commit passed my local tests with a large HISTSIZE. Regards, Rob > Thanks, > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
delaying the start of ifstated in /etc/rc
Depending on the use case for ifstated, dependencies may exist with other daemons for performing interface checks and/or external tests. For example, one might use ifstated to check a dhcpd enabled interface, or connectivity to a vmd virtual machine. Does anyone have any objections with delaying the start of ifstated? Comments? Ok? Index: src/etc/rc === RCS file: /cvs/src/etc/rc,v retrieving revision 1.517 diff -u -p -r1.517 rc --- src/etc/rc 29 Aug 2017 16:56:13 - 1.517 +++ src/etc/rc 31 Aug 2017 00:17:06 - @@ -558,7 +558,7 @@ echo 'preserving editor files.'; /usr/li run_upgrade_script sysmerge echo -n 'starting network daemons:' -start_daemon ldomd sshd switchd snmpd ldpd ripd ospfd ospf6d bgpd ifstated +start_daemon ldomd sshd switchd snmpd ldpd ripd ospfd ospf6d bgpd start_daemon relayd dhcpd dhcrelay mrouted dvmrpd radiusd eigrpd if ifconfig lo0 inet6 >/dev/null 2>&1; then @@ -569,7 +569,7 @@ fi start_daemon hostapd lpd smtpd slowcgi httpd ftpd start_daemon ftpproxy ftpproxy6 tftpd tftpproxy identd inetd rarpd bootparamd -start_daemon rbootd mopd vmd spamd spamlogd sndiod +start_daemon rbootd mopd vmd spamd spamlogd sndiod ifstated echo '.' # If rc.firsttime exists, run it just once, and make sure it is deleted.
Re: syslogd: accept space-deliminated fields
- Original Message - > From: "Todd C. Miller" > To: "tech" > Sent: Friday, July 1, 2016 12:55:11 PM > Subject: syslogd: accept space-deliminated fields > Linux, Net and Free also support space-deliminated fields. Maybe > we should too... > - todd > Index: usr.sbin/syslogd/syslog.conf.5 > === > RCS file: /cvs/src/usr.sbin/syslogd/syslog.conf.5,v > retrieving revision 1.33 > diff -u -p -u -r1.33 syslog.conf.5 > --- usr.sbin/syslogd/syslog.conf.5 10 Sep 2015 15:16:44 - 1.33 > +++ usr.sbin/syslogd/syslog.conf.5 1 Jul 2016 16:50:37 - > @@ -55,7 +55,7 @@ The > .Em selector > field is separated from the > .Em action > -field by one or more tab characters. > +field by one or more tab or space characters. > .Pp > The > .Em selectors > @@ -334,6 +334,10 @@ file appeared in > .Bx 4.3 , > along with > .Xr syslogd 8 . > +.Pp > +Historic versions of > +.Xr syslogd 8 > +did not support space-delimited fields. > .Sh BUGS > The effects of multiple selectors are sometimes not intuitive. > For example > Index: usr.sbin/syslogd/syslogd.c > === > RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v > retrieving revision 1.207 > diff -u -p -u -r1.207 syslogd.c > --- usr.sbin/syslogd/syslogd.c 1 Jul 2016 15:47:15 - 1.207 > +++ usr.sbin/syslogd/syslogd.c 1 Jul 2016 16:50:37 - > @@ -2454,19 +2454,19 @@ cfline(char *line, char *progblock, char > f->f_hostname = strdup(hostblock); > /* scan through the list of selectors */ > - for (p = line; *p && *p != '\t';) { > + for (p = line; *p && *p != '\t' && *p != ' ';) { > /* find the end of this facility name list */ > - for (q = p; *q && *q != '\t' && *q++ != '.'; ) > + for (q = p; *q && *q != '\t' && *q != ' ' && *q++ != '.'; ) > continue; > /* collect priority name */ > - for (bp = buf; *q && !strchr("\t,;", *q); ) > + for (bp = buf; *q && !strchr("\t,; ", *q); ) > *bp++ = *q++; > *bp = '\0'; > /* skip cruft */ > - while (*q && strchr(", ;", *q)) > + while (*q && strchr(",;", *q)) > q++; > /* decode priority name */ > @@ -2489,8 +2489,8 @@ cfline(char *line, char *progblock, char > } > /* scan facilities */ > - while (*p && !strchr("\t.;", *p)) { > - for (bp = buf; *p && !strchr("\t,;.", *p); ) > + while (*p && !strchr("\t.; ", *p)) { > + for (bp = buf; *p && !strchr("\t,;. ", *p); ) > *bp++ = *p++; > *bp = '\0'; > if (*buf == '*') > @@ -2516,7 +2516,7 @@ cfline(char *line, char *progblock, char > } > /* skip to action part */ > - while (*p == '\t') > + while (*p == '\t' || *p == ' ') > p++; > switch (*p) { This passed some basic testing on my end. Thanks! Rob
60.html on ntpd and pledge
ntpd was pledged in 5.9. Rob Index: 60.html === RCS file: /cvs/www/60.html,v retrieving revision 1.70 diff -u -p -r1.70 60.html --- 60.html 24 Aug 2016 20:47:30 - 1.70 +++ 60.html 25 Aug 2016 07:48:55 - @@ -598,7 +598,7 @@ to 6.0. Moved the execution of constraints from the ntp process to the parent process, allowing for better privilege separation since the ntp process can be further restricted. -Added +Improved http://man.openbsd.org/pledge.2";>pledge(2) support. Fixed high CPU usage when the network is down.
minor diff for faq15.html
There is only one result mentioned: ready-to-install binary packages. Rob Index: faq15.html === RCS file: /cvs/www/faq/faq15.html,v retrieving revision 1.141 diff -u -p -r1.141 faq15.html --- faq15.html 1 Sep 2016 12:05:14 - 1.141 +++ faq15.html 3 Sep 2016 18:23:13 - @@ -110,7 +110,7 @@ to OpenBSD's shared library specificatio secure whenever possible, etc. -The end result of the porting effort are ready-to-install binary +The end result of the porting effort is ready-to-install binary packages. The aim of the package system is to keep track of which software gets installed, so that it may at any time be updated or removed very easily.
mention relayd in faq6.html
Not sure if this is the right place or the right wording, but I think it deserves a mention somewhere in the faq. Regards, Rob Index: faq6.html === RCS file: /cvs/www/faq/faq6.html,v retrieving revision 1.383 diff -u -p -r1.383 faq6.html --- faq6.html 15 Aug 2016 02:22:13 - 1.383 +++ faq6.html 4 Sep 2016 01:05:49 - @@ -602,6 +602,11 @@ providing bandwidth control and packet p to create powerful and flexible firewalls. It is described in the PF User's Guide. + +See also OpenBSD's http://man.openbsd.org/relayd";>relayd(8) +for implementing load-balancers, application layer gateways, and +transparent proxies. + Dynamic Host Configuration Protocol Dynamic Host Configuration Protocol is a way to configure network
whitespace in /etc/rc
Index: rc === RCS file: /cvs/src/etc/rc,v retrieving revision 1.486 diff -u -p -r1.486 rc --- rc 10 Jul 2016 09:08:18 - 1.486 +++ rc 5 Sep 2016 15:45:09 - @@ -490,7 +490,6 @@ echo clearing /tmp # rc.securelevel did not specifically set -1 or 2, so select the default: 1. (($(sysctl -n kern.securelevel) == 0)) && sysctl kern.securelevel=1 - # Patch /etc/motd. if [[ ! -f /etc/motd ]]; then install -c -o root -g wheel -m 664 /dev/null /etc/motd
minor diff for vmctl.8
"host" is not a command argument. Rob Index: vmctl.8 === RCS file: /cvs/src/usr.sbin/vmctl/vmctl.8,v retrieving revision 1.13 diff -u -p -r1.13 vmctl.8 --- vmctl.8 18 Aug 2016 16:12:05 - 1.13 +++ vmctl.8 12 Sep 2016 01:36:59 - @@ -28,8 +28,7 @@ The .Nm utility is used to control the virtual machine monitor (VMM) subsystem. -A VMM manages virtual machines (VMs) on a -.Ar host . +A VMM manages virtual machines (VMs) on a host. The VMM subsystem is responsible for creating, destroying, and executing VMs. .Pp
fix regress in pstat.c
ttymode() needs nlist, otherwise "pstat -tf" will fail since kd will not be NULL and the calls from ttymode() to KGET will error as follows: pstat: cannot read ntty: invalid address (0) pstat: cannot read tty_head: invalid address (0) KGET(TTY_NTTY, ntty) and KGET(TTY_TTYLIST, tty_head) both result in a kvm_read() with an address (globalnl[TTY_NTTY].n_value) of zero. The following diff resolves this issue. Took the opportunity to reorder the flags to match the switch statement. With help from tb@. Rob Index: pstat.c === RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v retrieving revision 1.108 diff -u -p -r1.108 pstat.c --- pstat.c 14 Aug 2016 22:47:26 - 1.108 +++ pstat.c 17 Sep 2016 17:00:28 - @@ -190,7 +190,7 @@ main(int argc, char *argv[]) if ((dformat == NULL && argc > 0) || (dformat && argc == 0)) usage(); - need_nlist = vnodeflag || dformat; + need_nlist = dformat || (fileflag && ttyflag) || vnodeflag; if (nlistf != NULL || memf != NULL) { if (fileflag || totalflag)
Re: fix regress in pstat.c
On Sat, Sep 17, 2016 at 09:20:31PM +0200, Theo Buehler wrote: > While this patch avoids the bug, it isn't quite right: pstat -ft should > be equivalent to "pstat -f && pstat -t" but... > > $ pstat -ft > pstat: kvm_openfiles: /dev/mem: Permission denied > > The actual problem is in ttymode(): if kd != NULL (which is the case if > -f is combined with -t), there are accesses of the kvm datastructures > without them being properly initialized: > > 879 if (kd != NULL) > 880 KGET(TTY_NTTY, ntty); > > and > > 896 KGET(TTY_TTYLIST, tty_head); > 897 for (tp = TAILQ_FIRST(&tty_head); tp; > 898 tp = TAILQ_NEXT(&tty, tty_link)) { > 899 KGET2(tp, &tty, sizeof tty, "tty struct"); > 900 tty2itty(&tty, &itty); > 901 ttyprt(&itty); > 902 } > > These code paths must not be hit unless need_nlist is true, so expose it > globally and replace 'kd != NULL' with the proper condition need_nlist > > The following patch works in all affected cases, pstat -t, pstat -f, > pstat -tf, both as root and non-root as well as pstat -tv as root > (combinations with -d aren't interesting since pstat exits before > doing anything else but -d). > > Index: pstat.c > === > RCS file: /var/cvs/src/usr.sbin/pstat/pstat.c,v > retrieving revision 1.108 > diff -u -p -r1.108 pstat.c > --- pstat.c 14 Aug 2016 22:47:26 - 1.108 > +++ pstat.c 17 Sep 2016 17:57:38 - > @@ -89,6 +89,7 @@ struct e_vnode { > struct vnode vnode; > }; > > +int need_nlist; > int usenumflag; > int totalflag; > int kflag; > @@ -141,7 +142,7 @@ main(int argc, char *argv[]) > const char *dformat = NULL; > extern char *optarg; > extern int optind; > - int i, need_nlist; > + int i; > > hideroot = getuid(); > > @@ -869,7 +870,7 @@ ttymode(void) > struct itty itty, *itp; > size_t nlen; > > - if (kd == 0) { > + if (!need_nlist) { > mib[0] = CTL_KERN; > mib[1] = KERN_TTYCOUNT; > nlen = sizeof(ntty); > @@ -879,7 +880,7 @@ ttymode(void) > KGET(TTY_NTTY, ntty); > (void)printf("%d terminal device%s\n", ntty, ntty == 1 ? "" : "s"); > (void)printf("%s", hdr); > - if (kd == 0) { > + if (!need_nlist) { > mib[0] = CTL_KERN; > mib[1] = KERN_TTY; > mib[2] = KERN_TTY_INFO; > Thanks Theo. Fixes the regress and works well with your pending pledge diff. Rob
remove kern.arand from sysctl.8 and rnd.c (comment)
Index: sysctl.8 === RCS file: /cvs/src/sbin/sysctl/sysctl.8,v retrieving revision 1.205 diff -u -p -r1.205 sysctl.8 --- sysctl.87 Sep 2016 17:30:12 - 1.205 +++ sysctl.823 Sep 2016 02:16:14 - @@ -144,7 +144,6 @@ and a few require a kernel compiled with .It kern.sysvmsg Ta integer Ta no .It kern.sysvsem Ta integer Ta no .It kern.sysvshm Ta integer Ta no -.It kern.arandom Ta u_int Ta no .It kern.msgbufsize Ta integer Ta no .It kern.malloc.buckets Ta string Ta no .It kern.malloc.bucket. Ta string Ta no Index: rnd.c === RCS file: /cvs/src/sys/dev/rnd.c,v retrieving revision 1.186 diff -u -p -r1.186 rnd.c --- rnd.c 22 Sep 2016 22:04:02 - 1.186 +++ rnd.c 23 Sep 2016 02:25:06 - @@ -85,9 +85,8 @@ * The output of this single ChaCha20 engine is then shared amongst many * consumers in the kernel and userland via a few interfaces: * arc4random_buf(), arc4random(), arc4random_uniform(), randomread() - * for the set of /dev/random nodes, the sysctl kern.arandom, and the - * system call getentropy(), which provides seeds for process-context - * pseudorandom generators. + * for the set of /dev/random nodes, and the system call getentropy(), + * which provides seeds for process-context pseudorandom generators. * * Acknowledgements: * =