Re: pfctl: zap v4mask and v6mask in host()
On Mon, Jul 30, 2018 at 06:08:54PM +0200, Klemens Nanni wrote: > These two just complicate the code. Let's defer checks whether an > explicit mask has been specified to where it's actually set in host_*(). > > My motivation is to reduce address family specific code and perhaps even > merge `host_v4()' and `host_v6()' eventually. > > Feedback? OK? reads OK to me, no objections. sashan
Re: ahci@acpi: match on _CLS
On Mon, Jul 30, 2018 at 06:20:21PM +0200, Mark Kettenis wrote: > It seems that ARM SoC every vendor invents their own _HID/_CID for the > their AHCI implementation. Instead of adding all of these to an ever > growing list, we can match on _CLS instead which return the "PCI" > class/subclass/interface that the device is compatible with. > > This adds a define to pcireg.h for the AHCI interface. The name is > aligned with NetBSD. > > ok? > ok mlarkin if you didn't get to it already > > Index: dev/acpi/acpi.c > === > RCS file: /cvs/src/sys/dev/acpi/acpi.c,v > retrieving revision 1.355 > diff -u -p -r1.355 acpi.c > --- dev/acpi/acpi.c 10 Jul 2018 17:11:42 - 1.355 > +++ dev/acpi/acpi.c 30 Jul 2018 16:19:26 - > @@ -123,9 +123,6 @@ void acpi_create_thread(void *); > > void acpi_indicator(struct acpi_softc *, int); > > -int acpi_matchhids(struct acpi_attach_args *aa, const char *hids[], > - const char *driver); > - > void acpi_init_pm(struct acpi_softc *); > > int acpi_founddock(struct aml_node *, void *); > @@ -505,6 +502,33 @@ acpi_getminbus(int crsidx, union acpi_re > *bbn = crs->lr_word._min; > } > return 0; > +} > + > +int > +acpi_matchcls(struct acpi_attach_args *aaa, int class, int subclass, > +int interface) > +{ > + struct acpi_softc *sc = acpi_softc; > + struct aml_value res; > + > + if (aaa->aaa_dev == NULL || aaa->aaa_node == NULL) > + return (0); > + > + if (aml_evalname(sc, aaa->aaa_node, "_CLS", 0, NULL, &res)) > + return (0); > + > + if (res.type != AML_OBJTYPE_PACKAGE || res.length != 3 || > + res.v_package[0]->type != AML_OBJTYPE_INTEGER || > + res.v_package[1]->type != AML_OBJTYPE_INTEGER || > + res.v_package[2]->type != AML_OBJTYPE_INTEGER) > + return (0); > + > + if (res.v_package[0]->v_integer == class && > + res.v_package[1]->v_integer == subclass && > + res.v_package[2]->v_integer == interface) > + return (1); > + > + return (0); > } > > int > Index: dev/acpi/acpivar.h > === > RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v > retrieving revision 1.96 > diff -u -p -r1.96 acpivar.h > --- dev/acpi/acpivar.h10 Jul 2018 17:11:42 - 1.96 > +++ dev/acpi/acpivar.h30 Jul 2018 16:19:26 - > @@ -366,6 +366,7 @@ void acpi_write_pmreg(struct acpi_softc > void acpi_poll(void *); > void acpi_sleep(int, char *); > > +int acpi_matchcls(struct acpi_attach_args *, int, int, int); > int acpi_matchhids(struct acpi_attach_args *, const char *[], const char *); > int acpi_parsehid(struct aml_node *, void *, char *, char *, size_t); > > Index: dev/acpi/ahci_acpi.c > === > RCS file: /cvs/src/sys/dev/acpi/ahci_acpi.c,v > retrieving revision 1.1 > diff -u -p -r1.1 ahci_acpi.c > --- dev/acpi/ahci_acpi.c 1 Jul 2018 15:54:59 - 1.1 > +++ dev/acpi/ahci_acpi.c 30 Jul 2018 16:19:26 - > @@ -49,21 +49,15 @@ struct cfattach ahci_acpi_ca = { > sizeof(struct ahci_acpi_softc), ahci_acpi_match, ahci_acpi_attach > }; > > -const char *ahci_hids[] = { > - "AMDI0600", > - "LNRO001E", > - NULL > -}; > - > int ahci_acpi_parse_resources(int, union acpi_resource *, void *); > > int > ahci_acpi_match(struct device *parent, void *match, void *aux) > { > struct acpi_attach_args *aaa = aux; > - struct cfdata *cf = match; > > - return acpi_matchhids(aaa, ahci_hids, cf->cf_driver->cd_name); > + return acpi_matchcls(aaa, PCI_CLASS_MASS_STORAGE, > + PCI_SUBCLASS_MASS_STORAGE_SATA, PCI_INTERFACE_SATA_AHCI10); > } > > void > Index: dev/pci/ahci_pci.c > === > RCS file: /cvs/src/sys/dev/pci/ahci_pci.c,v > retrieving revision 1.14 > diff -u -p -r1.14 ahci_pci.c > --- dev/pci/ahci_pci.c3 Jan 2018 20:10:40 - 1.14 > +++ dev/pci/ahci_pci.c30 Jul 2018 16:19:27 - > @@ -43,7 +43,6 @@ > #define AHCI_PCI_BAR 0x24 > #define AHCI_PCI_ATI_SB600_MAGIC 0x40 > #define AHCI_PCI_ATI_SB600_LOCKED0x01 > -#define AHCI_PCI_INTERFACE 0x01 > > struct ahci_pci_softc { > struct ahci_softc psc_ahci; > @@ -232,7 +231,7 @@ ahci_ati_sb_idetoahci(struct ahci_softc > pci_conf_write(pa->pa_pc, pa->pa_tag, PCI_CLASS_REG, > PCI_CLASS_MASS_STORAGE << PCI_CLASS_SHIFT | > PCI_SUBCLASS_MASS_STORAGE_SATA << PCI_SUBCLASS_SHIFT | > - AHCI_PCI_INTERFACE << PCI_INTERFACE_SHIFT | > + PCI_INTERFACE_SATA_AHCI10 << PCI_INTERFACE_SHIFT | > PCI_REVISION(pa->pa_class) << PCI_REVISION_SHIFT); > > pci_conf_write(pa->pa_pc, pa->pa_tag, > @@ -310,7 +309,7 @
[PATCH] find(1) man page, source comment: assumed -print
Hey, A statement in the find(1) man page and a source comment in find.c are outdated and incorrect. The man page didn't mention that using -delete or -execdir prevents -print from being assumed. Similarly for a comment in find.c, but this didn't mention -delete, -execdir, -ls or -print0. (These primaries can be tested and/or you can see where isoutput is set to 1 in function.c.) I have a patch below to correct these. Cheers, Kris Katterjohn Index: find.1 === RCS file: /cvs/src/usr.bin/find/find.1,v retrieving revision 1.93 diff -u -p -r1.93 find.1 --- find.1 3 Jan 2017 22:19:31 - 1.93 +++ find.1 30 Jul 2018 21:28:36 - @@ -60,7 +60,9 @@ In the absence of an expression, is assumed. If an expression is given, but none of the primaries +.Ic -delete , .Ic -exec , +.Ic -execdir , .Ic -ls , .Ic -ok , .Ic -print , Index: find.c === RCS file: /cvs/src/usr.bin/find/find.c,v retrieving revision 1.22 diff -u -p -r1.22 find.c --- find.c 4 Jan 2017 09:21:26 - 1.22 +++ find.c 30 Jul 2018 21:28:36 - @@ -86,9 +86,10 @@ find_formplan(char **argv) } /* -* if the user didn't specify one of -print, -ok or -exec, then -print -* is assumed so we bracket the current expression with parens, if -* necessary, and add a -print node on the end. +* if the user didn't specify one of -delete, -exec, -execdir, +* -ls, -ok, -print or -print0, then -print is assumed so we +* bracket the current expression with parens, if necessary, +* and add a -print node on the end. */ if (!isoutput) { if (plan == NULL) {
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
[PATCH] find(1) man page: -exec evaluation
Hey, The man page for find(1) does not mention when the -exec primary evaluates to true. -exec utility ... ; evaluates to true when the utility exits with a zero exit status, while -exec utility ... {} + always evaluates to true. I have a patch below with my attempt at a description. I tried to make the wording consistent with other parts of the man page. Cheers, Kris Katterjohn Index: find.1 === RCS file: /cvs/src/usr.bin/find/find.1,v retrieving revision 1.93 diff -u -p -r1.93 find.1 --- find.1 3 Jan 2017 22:19:31 - 1.93 +++ find.1 30 Jul 2018 19:08:16 - @@ -222,6 +222,10 @@ or a plus sign If terminated by a semicolon, the .Ar utility is executed once per path. +This form of the primary evaluates to true if the invocation +of +.Ar utility +exits with a zero exit status. If the string .Qq {} appears anywhere in the utility name or the @@ -233,6 +237,7 @@ primary is evaluated are aggregated into .Ar utility will be invoked once per set, similar to .Xr xargs 1 . +This form of the primary always evaluates to true. If any invocation exits with a non-zero exit status, then .Nm will eventually do so as well, but this does not cause
pr_attach w/o socket lock
When a per-protocol attach function is called the given socket is not yet reachable, so there's no need to lock it. So the diff below remove the solock/sounlock dance and shows where the NET_LOCK() is required to protect some specific global data structures. I reordered the different blocks in all pr_attach for consistency but also to be able to relax the assertions in soreserve() & friends. The interesting bits are in tcp_attach() since that's the only function which is also called from the packet processing path. Comments? Oks? Index: kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.226 diff -u -p -r1.226 uipc_socket.c --- kern/uipc_socket.c 30 Jul 2018 12:22:14 - 1.226 +++ kern/uipc_socket.c 30 Jul 2018 12:46:34 - @@ -141,15 +141,14 @@ socreate(int dom, struct socket **aso, i so->so_cpid = p->p_p->ps_pid; so->so_proto = prp; - s = solock(so); error = (*prp->pr_attach)(so, proto); if (error) { + s = solock(so); so->so_state |= SS_NOFDREF; /* sofree() calls sounlock(). */ sofree(so, s); return (error); } - sounlock(so, s); *aso = so; return (0); } Index: kern/uipc_socket2.c === RCS file: /cvs/src/sys/kern/uipc_socket2.c,v retrieving revision 1.96 diff -u -p -r1.96 uipc_socket2.c --- kern/uipc_socket2.c 10 Jul 2018 10:02:14 - 1.96 +++ kern/uipc_socket2.c 30 Jul 2018 12:46:34 - @@ -96,7 +96,9 @@ soisconnected(struct socket *so) { struct socket *head = so->so_head; - soassertlocked(so); + if ((so->so_pcb != NULL) || head != NULL) + soassertlocked(so); + so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING); so->so_state |= SS_ISCONNECTED; if (head && soqremque(so, 0)) { @@ -148,8 +150,7 @@ sonewconn(struct socket *head, int conns /* * XXXSMP as long as `so' and `head' share the same lock, we -* can call soreserve() and pr_attach() below w/o expliclitly -* locking `so'. +* can call soqinsque() below w/o expliclitly locking `so'. */ soassertlocked(head); @@ -189,12 +190,11 @@ sonewconn(struct socket *head, int conns so->so_rcv.sb_lowat = head->so_rcv.sb_lowat; so->so_rcv.sb_timeo = head->so_rcv.sb_timeo; - soqinsque(head, so, soqueue); if ((*so->so_proto->pr_attach)(so, 0)) { - (void) soqremque(so, soqueue); pool_put(&socket_pool, so); return (NULL); } + soqinsque(head, so, soqueue); if (connstatus) { sorwakeup(head); wakeup(&head->so_timeo); @@ -448,7 +448,8 @@ sowakeup(struct socket *so, struct sockb int soreserve(struct socket *so, u_long sndcc, u_long rcvcc) { - soassertlocked(so); + if (so->so_pcb != NULL) + soassertlocked(so); if (sbreserve(so, &so->so_snd, sndcc)) goto bad; @@ -478,7 +479,8 @@ int sbreserve(struct socket *so, struct sockbuf *sb, u_long cc) { KASSERT(sb == &so->so_rcv || sb == &so->so_snd); - soassertlocked(so); + if (so->so_pcb != NULL) + soassertlocked(so); if (cc == 0 || cc > sb_max) return (1); @@ -948,7 +950,8 @@ sbdrop(struct socket *so, struct sockbuf struct mbuf *next; KASSERT(sb == &so->so_rcv || sb == &so->so_snd); - soassertlocked(so); + if (so->so_pcb != NULL) + soassertlocked(so); next = (m = sb->sb_mb) ? m->m_nextpkt : 0; while (len > 0) { Index: kern/uipc_usrreq.c === RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v retrieving revision 1.134 diff -u -p -r1.134 uipc_usrreq.c --- kern/uipc_usrreq.c 9 Jul 2018 10:58:21 - 1.134 +++ kern/uipc_usrreq.c 30 Jul 2018 12:46:34 - @@ -336,7 +336,7 @@ uipc_attach(struct socket *so, int proto { struct unpcb *unp; int error; - + if (so->so_pcb) return EISCONN; if (so->so_snd.sb_hiwat == 0 || so->so_rcv.sb_hiwat == 0) { Index: net/pfkeyv2.c === RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.189 diff -u -p -r1.189 pfkeyv2.c --- net/pfkeyv2.c 10 Jul 2018 20:28:34 - 1.189 +++ net/pfkeyv2.c 30 Jul 2018 12:50:33 - @@ -266,23 +266,19 @@ pfkeyv2_attach(struct socket *so, int pr if ((so->so_state & SS_PRIV) == 0) return EACCES; - kp = malloc(sizeof(struct pkpcb), M_PCB, M_WAITOK | M_ZERO); - so->so_pcb = kp; - refcnt_init(&kp->kcb_refcnt); - error = soreserve(so, PFKEYSNDQ, PFKEYRCVQ); - if
Newer unveil diffs
unveil(2) is now enabled in -current. For those who want to play along at home, here are some diffs which use this in a variety of programs. Not all these diffs are correct or complete yet. This is a learning experience. Based upon what we learn, we may still change unveil(2) semantics slightly (similar to how pledge semantics were reached). These diffs are in snapshots. Index: bin/ps/ps.c === RCS file: /cvs/src/bin/ps/ps.c,v retrieving revision 1.71 diff -u -p -u -r1.71 ps.c --- bin/ps/ps.c 23 Sep 2016 06:28:08 - 1.71 +++ bin/ps/ps.c 12 Jul 2018 16:18:13 - @@ -276,6 +276,19 @@ main(int argc, char *argv[]) if (kd == NULL) errx(1, "%s", errbuf); + if (unveil(_PATH_DEVDB, "r") == -1) + err(1, "unveil"); + if (unveil("/dev", "r") == -1) + err(1, "unveil"); + if (swapf) + if (unveil(swapf, "r") == -1) + err(1, "unveil"); + if (nlistf) + if (unveil(nlistf, "r") == -1) + err(1, "unveil"); + if (memf) + if (unveil(memf, "r") == -1) + err(1, "unveil"); if (pledge("stdio rpath getpw ps", NULL) == -1) err(1, "pledge"); Index: libexec/comsat/comsat.c === RCS file: /cvs/src/libexec/comsat/comsat.c,v retrieving revision 1.48 diff -u -p -u -r1.48 comsat.c --- libexec/comsat/comsat.c 3 Apr 2017 17:23:39 - 1.48 +++ libexec/comsat/comsat.c 12 Jul 2018 16:18:13 - @@ -91,6 +91,12 @@ main(int argc, char *argv[]) exit(1); } + if (unveil(_PATH_MAILDIR, "r") == -1) + err(1, "unveil"); + if (unveil(_PATH_UTMP, "r") == -1) + err(1, "unveil"); + if (unveil("/tmp", "w") == -1) + err(1, "unveil"); if (pledge("stdio rpath wpath proc tty", NULL) == -1) err(1, "pledge"); Index: libexec/fingerd/fingerd.c === RCS file: /cvs/src/libexec/fingerd/fingerd.c,v retrieving revision 1.39 diff -u -p -u -r1.39 fingerd.c --- libexec/fingerd/fingerd.c 13 Nov 2015 01:26:33 - 1.39 +++ libexec/fingerd/fingerd.c 26 Jul 2018 16:22:32 - @@ -68,7 +68,7 @@ main(int argc, char *argv[]) char **ap, *av[ENTRIES + 1], line[8192], *lp, *hname; char hostbuf[HOST_NAME_MAX+1]; - if (pledge("stdio inet dns proc exec", NULL) == -1) + if (pledge("stdio unveil inet dns proc exec", NULL) == -1) err(1, "pledge"); prog = _PATH_FINGER; @@ -110,6 +110,9 @@ main(int argc, char *argv[]) default: usage(); } + + if (unveil(_PATH_FINGER, "x") == -1) + err(1, "unveil"); if (logging) { struct sockaddr_storage ss; Index: libexec/getty/main.c === RCS file: /cvs/src/libexec/getty/main.c,v retrieving revision 1.48 diff -u -p -u -r1.48 main.c --- libexec/getty/main.c29 May 2017 04:40:35 - 1.48 +++ libexec/getty/main.c12 Jul 2018 16:18:13 - @@ -169,6 +169,19 @@ main(int argc, char *argv[]) ioctl(0, FIOASYNC, &off); /* turn off async mode */ + if (unveil("/usr/bin/login", "x") == -1) { + syslog(LOG_ERR, "%s: %m", tname); + exit(1); + } + if (unveil(_PATH_GETTYTAB, "r") == -1) { + syslog(LOG_ERR, "%s: %m", tname); + exit(1); + } + if (unveil("/dev", "rw") == -1) { + syslog(LOG_ERR, "%s: %m", tname); + exit(1); + } + /* * The following is a work around for vhangup interactions * which cause great problems getting window systems started. Index: libexec/lockspool/lockspool.c === RCS file: /cvs/src/libexec/lockspool/lockspool.c,v retrieving revision 1.18 diff -u -p -u -r1.18 lockspool.c --- libexec/lockspool/lockspool.c 24 Nov 2015 00:19:29 - 1.18 +++ libexec/lockspool/lockspool.c 12 Jul 2018 16:18:13 - @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,8 @@ main(int argc, char *argv[]) char *from, c; int holdfd; + if (unveil(_PATH_MAILDIR, "rwc") == -1) + err(1, "unveil"); if (pledge("stdio rpath wpath getpw cpath fattr", NULL) == -1) err(1, "pledge"); Index: libexec/spamd/grey.c === RCS file: /cvs/src/libexec/spamd/grey.c,v retrieving revision 1.65 diff -u -p -u -r1.65 grey.c --- libexec/spamd/grey.c18 Oct 2017 17:31:01 - 1.65 +++
Re: unveil: incomplete unveil_flagmatch semantic
+ for (i=0; flags[i].pledge != 0; i++) + if (ISSET(pledge_flags, flags[i].pledge)) { + SET(permissions, flags[i].unveil); + CLR(pledge_flags, flags[i].pledge); + } Rather than iterating, can this be done as a direct lookup? table[PLEDGE_RPATH] = ... table[PLEDGE_RPATH | PLEDGE_WPATH] = .. unveil = table[pledge & range_enforcing_mask];
Re: unveil: incomplete unveil_flagmatch semantic
On Mon, Jul 30, 2018 at 07:55:35AM -0600, Bob Beck wrote: > yeah the latter will be the way to go > here it is. Some notes: - I changed flags definition from uint64_t to int - I defined `static inline' the function that do the conversion from pledge to unveil: having a function is more readable, but it is called often (for checking each compoment) thanks. -- Sebastien Marie Index: sys/proc.h === RCS file: /cvs/src/sys/sys/proc.h,v retrieving revision 1.254 diff -u -p -r1.254 proc.h --- sys/proc.h 28 Jul 2018 18:07:26 - 1.254 +++ sys/proc.h 30 Jul 2018 15:27:22 - @@ -130,7 +130,7 @@ struct tusage { struct unvname { char*un_name; size_t un_namesize; - uint64_tun_flags; + int un_flags; RBT_ENTRY(unvnmae) un_rbt; }; @@ -424,7 +424,7 @@ struct unveil { struct vnode*uv_vp; struct unvname_rbt uv_names; struct rwlock uv_lock; - u_int64_t uv_flags; + int uv_flags; }; struct uidinfo { Index: kern/kern_unveil.c === RCS file: /cvs/src/sys/kern/kern_unveil.c,v retrieving revision 1.9 diff -u -p -r1.9 kern_unveil.c --- kern/kern_unveil.c 30 Jul 2018 15:16:27 - 1.9 +++ kern/kern_unveil.c 30 Jul 2018 17:40:07 - @@ -40,6 +40,11 @@ #define UNVEIL_MAX_VNODES 128 #define UNVEIL_MAX_NAMES 128 +#defineUNVEIL_READ 0x01 +#defineUNVEIL_WRITE0x02 +#defineUNVEIL_EXEC 0x04 +#defineUNVEIL_CREATE 0x08 + static inline int unvname_compare(const struct unvname *n1, const struct unvname *n2) { @@ -50,7 +55,7 @@ unvname_compare(const struct unvname *n1 } struct unvname * -unvname_new(const char *name, size_t size, uint64_t flags) +unvname_new(const char *name, size_t size, int flags) { struct unvname *ret = malloc(sizeof(struct unvname), M_PROC, M_WAITOK); ret->un_name = malloc(size, M_PROC, M_WAITOK); @@ -118,7 +123,7 @@ unveil_delete_names(struct unveil *uv) } void -unveil_add_name(struct unveil *uv, char *name, uint64_t flags) +unveil_add_name(struct unveil *uv, char *name, int flags) { struct unvname *unvn; @@ -310,7 +315,7 @@ unveil_lookup(struct vnode *vp, struct p } int -unveil_parsepermissions(const char *permissions, uint64_t *perms) +unveil_parsepermissions(const char *permissions, int *perms) { size_t i = 0; char c; @@ -319,16 +324,16 @@ unveil_parsepermissions(const char *perm while ((c = permissions[i++]) != '\0') { switch (c) { case 'r': - *perms |= PLEDGE_RPATH; + *perms |= UNVEIL_READ; break; case 'w': - *perms |= PLEDGE_WPATH; + *perms |= UNVEIL_WRITE; break; case 'x': - *perms |= PLEDGE_EXEC; + *perms |= UNVEIL_EXEC; break; case 'c': - *perms |= PLEDGE_CPATH; + *perms |= UNVEIL_CREATE; break; default: return -1; @@ -338,7 +343,7 @@ unveil_parsepermissions(const char *perm } int -unveil_setflags(uint64_t *flags, uint64_t nflags) +unveil_setflags(int *flags, int nflags) { #if 0 if (((~(*flags)) & nflags) != 0) { @@ -403,7 +408,7 @@ unveil_add(struct proc *p, struct nameid struct unveil *uv; int directory_add; int ret = EINVAL; - u_int64_t flags; + int flags; KASSERT(ISSET(ndp->ni_cnd.cn_flags, HASBUF)); /* must have SAVENAME */ @@ -525,13 +530,50 @@ unveil_add(struct proc *p, struct nameid return ret; } +static inline int +unveil_pledgetopermissions(uint64_t pledge_flags) +{ + static const struct { + uint64_t pledge; + int unveil; + } flags[] = { + { PLEDGE_RPATH, UNVEIL_READ }, + { PLEDGE_WPATH, UNVEIL_WRITE }, + { PLEDGE_EXEC, UNVEIL_EXEC }, + { PLEDGE_CPATH, UNVEIL_CREATE }, + { PLEDGE_CHOWN, UNVEIL_WRITE }, + { PLEDGE_DPATH, UNVEIL_CREATE }, + { PLEDGE_FATTR, UNVEIL_WRITE }, + { PLEDGE_TTY, UNVEIL_WRITE }, + { PLEDGE_UNIX, UNVEIL_READ|UNVEIL_WRITE }, + { PLEDGE_STAT, 0 }, + { PLEDGE_STATLIE, 0 }, + { 0, 0 }, + }; + int i; + int permissions = 0; + + for (i=0; flags[i].pledge != 0; i++) + if (ISSET(pledge_flags, flags[i].pledge)) { + SET(permissions, flags[i].unveil); +
Re: unveil in sndiod
On Mon, Jul 30, 2018 at 10:02:51AM -0600, Theo de Raadt wrote: > Alexandre Ratchov wrote: > > > On Mon, Jul 30, 2018 at 07:56:00AM -0600, Theo de Raadt wrote: > > > there are a lot of files in /dev ... > > > > > > can you make this tighter? > > > > > > > Yes. I'm experimenting around this right now. I'm looking at the > > following possibilities: > > > > (1) during initialization, parse device names to determine paths, then > > call unveil() for each file. This can work because sndiod knows in > > advance all devices it will use. > > Good enough. > here's the diff. Index: sndiod.c === RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v retrieving revision 1.33 diff -u -p -r1.33 sndiod.c --- sndiod.c26 Jun 2018 07:12:35 - 1.33 +++ sndiod.c30 Jul 2018 16:57:31 - @@ -340,9 +340,26 @@ mkopt(char *path, struct dev *d, return o; } +static void +dounveil(char *name, char *prefix, char *path_prefix) +{ + size_t prefix_len; + char path[PATH_MAX]; + + prefix_len = strlen(prefix); + + if (strncmp(name, prefix, prefix_len) != 0) + errx(1, "%s: unsupported device or port format", name); + snprintf(path, sizeof(path), "%s%s", path_prefix, name + prefix_len); + if (unveil(path, "rw") < 0) + err(1, "unveil"); +} + static int start_helper(int background) { + struct dev *d; + struct port *p; struct passwd *pw; int s[2]; pid_t pid; @@ -378,6 +395,10 @@ start_helper(int background) setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) err(1, "cannot drop privileges"); } + for (d = dev_list; d != NULL; d = d->next) + dounveil(d->path, "rsnd/", "/dev/audio"); + for (p = port_list; p != NULL; p = p->next) + dounveil(p->path, "rmidi/", "/dev/rmidi"); if (pledge("stdio sendfd rpath wpath", NULL) < 0) err(1, "pledge"); while (file_poll())
Re: httpd server configuration evaluation bug
Sorry, this time with the correct diff. On 7/25/18 4:15 PM, Base Pr1me wrote: Hi, I discovered that the wrong server configuration is evaluated in the server_read_http function. Only the first server in httpd.conf is checked. For example, I have five servers setup in httpd.conf and the third server is the only one with connection { max request body } set, because I desire it to accept larger uploads than the other servers. When the upload is initiated, server one dictates the max request body size, globally. The attached diff moves the queue loop out of the server_response function in to its own function, as to not duplicate code. I don't know if this is the only place the wrong information is evaluated. Also, I'm not sure this is the best method to fix the problem, but it should point the powers that be in the right direction. Thanks, Tracey Index: src/usr.sbin/httpd/httpd.h === RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v retrieving revision 1.138 diff -u -p -u -r1.138 httpd.h --- src/usr.sbin/httpd/httpd.h 20 Jun 2018 16:43:05 - 1.138 +++ src/usr.sbin/httpd/httpd.h 30 Jul 2018 16:22:29 - @@ -691,6 +691,8 @@ const char * char *server_http_parsehost(char *, char *, size_t, int *); ssize_t server_http_time(time_t, char *, size_t); int server_log_http(struct client *, unsigned int, size_t); +int server_check_client_config(struct server_config *, struct client *, + struct kv *); /* server_file.c */ int server_file(struct httpd *, struct client *); Index: src/usr.sbin/httpd/server_http.c === RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v retrieving revision 1.122 diff -u -p -u -r1.122 server_http.c --- src/usr.sbin/httpd/server_http.c 20 Jun 2018 16:43:05 - 1.122 +++ src/usr.sbin/httpd/server_http.c 30 Jul 2018 16:22:29 - @@ -204,7 +204,7 @@ server_read_http(struct bufferevent *bev char *line = NULL, *key, *value; const char *errstr; size_t size, linelen; - struct kv *hdr = NULL; + struct kv *hdr = NULL, kv_key, *host; getmonotime(&clt->clt_tv_last); @@ -344,6 +344,16 @@ server_read_http(struct bufferevent *bev goto abort; } + kv_key.kv_key = "Host"; + if ((host = kv_find(&desc->http_headers, &kv_key)) != + NULL && host->kv_value == NULL) +host = NULL; + + if (server_check_client_config(srv_conf, clt, host)) +goto fail; + + srv_conf = clt->clt_srv_conf; + /* * Need to read data from the client after the * HTTP header. @@ -1168,10 +1178,7 @@ server_response(struct httpd *httpd, str struct server *srv = clt->clt_srv; struct server_config *srv_conf = &srv->srv_conf; struct kv *kv, key, *host; - struct str_find sm; - int portval = -1, ret; - char *hostval, *query; - const char *errstr = NULL; + char *query; /* Decode the URL */ if (desc->http_path == NULL || @@ -1219,58 +1226,10 @@ server_response(struct httpd *httpd, str if (clt->clt_pipelining && clt->clt_toread > 0) clt->clt_persist = 0; - /* - * Do we have a Host header and matching configuration? - * XXX the Host can also appear in the URL path. - */ - if (host != NULL) { - if ((hostval = server_http_parsehost(host->kv_value, - hostname, sizeof(hostname), &portval)) == NULL) - goto fail; + if (server_check_client_config(srv_conf, clt, host)) + goto fail; - TAILQ_FOREACH(srv_conf, &srv->srv_hosts, entry) { -#ifdef DEBUG - if ((srv_conf->flags & SRVFLAG_LOCATION) == 0) { -DPRINTF("%s: virtual host \"%s:%u\"" -" host \"%s\" (\"%s\")", -__func__, srv_conf->name, -ntohs(srv_conf->port), host->kv_value, -hostname); - } -#endif - if (srv_conf->flags & SRVFLAG_LOCATION) -continue; - else if (srv_conf->flags & SRVFLAG_SERVER_MATCH) { -str_find(hostname, srv_conf->name, -&sm, 1, &errstr); -ret = errstr == NULL ? 0 : -1; - } else { -ret = fnmatch(srv_conf->name, -hostname, FNM_CASEFOLD); - } - if (ret == 0 && - (portval == -1 || - (portval != -1 && portval == srv_conf->port))) { -/* Replace host configuration */ -clt->clt_srv_conf = srv_conf; -srv_conf = NULL; -break; - } - } - } - - if (srv_conf != NULL) { - /* Use the actual server IP address */ - if (server_http_host(&clt->clt_srv_ss, hostname, - sizeof(hostname)) == NULL) - goto fail; - } else { - /* Host header was valid and found */ - if (strlcpy(hostname, host->kv_value, sizeof(hostname)) >= - sizeof(hostname)) - goto fail; - srv_conf = clt->clt_srv_conf; - } + srv_conf = clt->clt_srv_conf; if ((desc->http_host = strdup(hostname)) == NULL) goto fail; @@ -1748,4 +1707,69 @@ done: free(agent_v); return (ret); +} + +int +server_check_client_config(struct server_config *srv_conf, struct client *clt, +struct kv *host) +{ + char hostname[HOST_NAME_MAX+1]; + struct serv
ahci@acpi: match on _CLS
It seems that ARM SoC every vendor invents their own _HID/_CID for the their AHCI implementation. Instead of adding all of these to an ever growing list, we can match on _CLS instead which return the "PCI" class/subclass/interface that the device is compatible with. This adds a define to pcireg.h for the AHCI interface. The name is aligned with NetBSD. ok? Index: dev/acpi/acpi.c === RCS file: /cvs/src/sys/dev/acpi/acpi.c,v retrieving revision 1.355 diff -u -p -r1.355 acpi.c --- dev/acpi/acpi.c 10 Jul 2018 17:11:42 - 1.355 +++ dev/acpi/acpi.c 30 Jul 2018 16:19:26 - @@ -123,9 +123,6 @@ voidacpi_create_thread(void *); void acpi_indicator(struct acpi_softc *, int); -intacpi_matchhids(struct acpi_attach_args *aa, const char *hids[], - const char *driver); - void acpi_init_pm(struct acpi_softc *); intacpi_founddock(struct aml_node *, void *); @@ -505,6 +502,33 @@ acpi_getminbus(int crsidx, union acpi_re *bbn = crs->lr_word._min; } return 0; +} + +int +acpi_matchcls(struct acpi_attach_args *aaa, int class, int subclass, +int interface) +{ + struct acpi_softc *sc = acpi_softc; + struct aml_value res; + + if (aaa->aaa_dev == NULL || aaa->aaa_node == NULL) + return (0); + + if (aml_evalname(sc, aaa->aaa_node, "_CLS", 0, NULL, &res)) + return (0); + + if (res.type != AML_OBJTYPE_PACKAGE || res.length != 3 || + res.v_package[0]->type != AML_OBJTYPE_INTEGER || + res.v_package[1]->type != AML_OBJTYPE_INTEGER || + res.v_package[2]->type != AML_OBJTYPE_INTEGER) + return (0); + + if (res.v_package[0]->v_integer == class && + res.v_package[1]->v_integer == subclass && + res.v_package[2]->v_integer == interface) + return (1); + + return (0); } int Index: dev/acpi/acpivar.h === RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v retrieving revision 1.96 diff -u -p -r1.96 acpivar.h --- dev/acpi/acpivar.h 10 Jul 2018 17:11:42 - 1.96 +++ dev/acpi/acpivar.h 30 Jul 2018 16:19:26 - @@ -366,6 +366,7 @@ voidacpi_write_pmreg(struct acpi_softc void acpi_poll(void *); void acpi_sleep(int, char *); +intacpi_matchcls(struct acpi_attach_args *, int, int, int); intacpi_matchhids(struct acpi_attach_args *, const char *[], const char *); intacpi_parsehid(struct aml_node *, void *, char *, char *, size_t); Index: dev/acpi/ahci_acpi.c === RCS file: /cvs/src/sys/dev/acpi/ahci_acpi.c,v retrieving revision 1.1 diff -u -p -r1.1 ahci_acpi.c --- dev/acpi/ahci_acpi.c1 Jul 2018 15:54:59 - 1.1 +++ dev/acpi/ahci_acpi.c30 Jul 2018 16:19:26 - @@ -49,21 +49,15 @@ struct cfattach ahci_acpi_ca = { sizeof(struct ahci_acpi_softc), ahci_acpi_match, ahci_acpi_attach }; -const char *ahci_hids[] = { - "AMDI0600", - "LNRO001E", - NULL -}; - intahci_acpi_parse_resources(int, union acpi_resource *, void *); int ahci_acpi_match(struct device *parent, void *match, void *aux) { struct acpi_attach_args *aaa = aux; - struct cfdata *cf = match; - return acpi_matchhids(aaa, ahci_hids, cf->cf_driver->cd_name); + return acpi_matchcls(aaa, PCI_CLASS_MASS_STORAGE, + PCI_SUBCLASS_MASS_STORAGE_SATA, PCI_INTERFACE_SATA_AHCI10); } void Index: dev/pci/ahci_pci.c === RCS file: /cvs/src/sys/dev/pci/ahci_pci.c,v retrieving revision 1.14 diff -u -p -r1.14 ahci_pci.c --- dev/pci/ahci_pci.c 3 Jan 2018 20:10:40 - 1.14 +++ dev/pci/ahci_pci.c 30 Jul 2018 16:19:27 - @@ -43,7 +43,6 @@ #define AHCI_PCI_BAR 0x24 #define AHCI_PCI_ATI_SB600_MAGIC 0x40 #define AHCI_PCI_ATI_SB600_LOCKED 0x01 -#define AHCI_PCI_INTERFACE 0x01 struct ahci_pci_softc { struct ahci_softc psc_ahci; @@ -232,7 +231,7 @@ ahci_ati_sb_idetoahci(struct ahci_softc pci_conf_write(pa->pa_pc, pa->pa_tag, PCI_CLASS_REG, PCI_CLASS_MASS_STORAGE << PCI_CLASS_SHIFT | PCI_SUBCLASS_MASS_STORAGE_SATA << PCI_SUBCLASS_SHIFT | - AHCI_PCI_INTERFACE << PCI_INTERFACE_SHIFT | + PCI_INTERFACE_SATA_AHCI10 << PCI_INTERFACE_SHIFT | PCI_REVISION(pa->pa_class) << PCI_REVISION_SHIFT); pci_conf_write(pa->pa_pc, pa->pa_tag, @@ -310,7 +309,7 @@ ahci_pci_match(struct device *parent, vo if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MASS_STORAGE && PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MASS_STORAGE_SATA && - PCI_INTERFACE(pa->pa_class) == AHCI_PCI_INTERFACE) + PCI_INTERFACE(pa->pa_cl
pfctl: zap v4mask and v6mask in host()
These two just complicate the code. Let's defer checks whether an explicit mask has been specified to where it's actually set in host_*(). My motivation is to reduce address family specific code and perhaps even merge `host_v4()' and `host_v6()' eventually. Feedback? OK? Index: pfctl_parser.c === RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v retrieving revision 1.325 diff -u -p -r1.325 pfctl_parser.c --- pfctl_parser.c 30 Jul 2018 08:28:40 - 1.325 +++ pfctl_parser.c 30 Jul 2018 15:53:42 - @@ -76,7 +76,7 @@ struct node_host *ifa_grouplookup(const struct node_host *host_if(const char *, int); struct node_host *host_v4(const char *, int); struct node_host *host_v6(const char *, int); -struct node_host *host_dns(const char *, int, int, int); +struct node_host *host_dns(const char *, int, int); const char *tcpflags = "FSRPAUEW"; @@ -1628,7 +1628,7 @@ struct node_host * host(const char *s, int opts) { struct node_host*h = NULL, *n; - int mask = -1, v4mask = 32, v6mask = 128; + int mask = -1; char*p, *r, *ps, *if_name; const char *errstr; @@ -1643,20 +1643,19 @@ host(const char *s, int opts) if ((p = strrchr(ps, '/')) != NULL) { if ((r = strdup(ps)) == NULL) err(1, "host: strdup"); - mask = strtonum(p+1, 0, v6mask, &errstr); + mask = strtonum(p+1, 0, 128, &errstr); if (errstr) { fprintf(stderr, "netmask is %s: %s\n", errstr, p); goto error; } p[0] = '\0'; - v4mask = v6mask = mask; } else r = ps; if ((h = host_if(ps, mask)) == NULL && (h = host_v4(r, mask)) == NULL && - (h = host_v6(ps, v6mask)) == NULL && - (h = host_dns(ps, v4mask, v6mask, (opts & PF_OPT_NODNS))) == NULL) { + (h = host_v6(ps, mask)) == NULL && + (h = host_dns(ps, mask, (opts & PF_OPT_NODNS))) == NULL) { fprintf(stderr, "no IP address found for %s\n", s); goto error; } @@ -1773,7 +1772,7 @@ host_v6(const char *s, int mask) sizeof(h->addr.v.a.addr)); h->ifindex = ((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id; - set_ipmask(h, mask); + set_ipmask(h, mask > -1 ? mask : 128); freeaddrinfo(res); h->next = NULL; h->tail = h; @@ -1783,7 +1782,7 @@ host_v6(const char *s, int mask) } struct node_host * -host_dns(const char *s, int v4mask, int v6mask, int numeric) +host_dns(const char *s, int mask, int numeric) { struct addrinfo hints, *res0, *res; struct node_host*n, *h = NULL; @@ -1829,7 +1828,7 @@ host_dns(const char *s, int v4mask, int &((struct sockaddr_in *) res->ai_addr)->sin_addr.s_addr, sizeof(struct in_addr)); - set_ipmask(n, v4mask); + set_ipmask(n, mask > -1 ? mask : 32); } else { memcpy(&n->addr.v.a.addr, &((struct sockaddr_in6 *) @@ -1838,7 +1837,7 @@ host_dns(const char *s, int v4mask, int n->ifindex = ((struct sockaddr_in6 *) res->ai_addr)->sin6_scope_id; - set_ipmask(n, v6mask); + set_ipmask(n, mask > -1 ? mask : 128); } n->next = NULL; n->tail = n;
Re: unveil in sndiod
Alexandre Ratchov wrote: > On Mon, Jul 30, 2018 at 07:56:00AM -0600, Theo de Raadt wrote: > > there are a lot of files in /dev ... > > > > can you make this tighter? > > > > Yes. I'm experimenting around this right now. I'm looking at the > following possibilities: > > (1) during initialization, parse device names to determine paths, then > call unveil() for each file. This can work because sndiod knows in > advance all devices it will use. Good enough. > (2) Add a new /dev/snd/ directory and move /dev/audio* and /dev/rmidi* > there. Then call unveil for the /dev/snd/ directory. > > This allows other audio/midi programs to use unveil (some allow > the user to select the device). Please do not go in that direction.
Re: unveil in sndiod
On Mon, Jul 30, 2018 at 07:56:00AM -0600, Theo de Raadt wrote: > there are a lot of files in /dev ... > > can you make this tighter? > Yes. I'm experimenting around this right now. I'm looking at the following possibilities: (1) during initialization, parse device names to determine paths, then call unveil() for each file. This can work because sndiod knows in advance all devices it will use. (2) Add a new /dev/snd/ directory and move /dev/audio* and /dev/rmidi* there. Then call unveil for the /dev/snd/ directory. This allows other audio/midi programs to use unveil (some allow the user to select the device). you see other options? thoughts?
Re: unveil in sndiod
there are a lot of files in /dev ... can you make this tighter? Alexandre Ratchov wrote: > A trivial diff for the sndiod "device helper" process. All this > process does is to open the device and pass it to the main process. So > it can be restricted to /dev. > > The other sndiod process has neither of rpath, wpath, cpath, or exec, > so it doesn't need unveil, right? > > Index: sndiod.c > === > RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v > retrieving revision 1.33 > diff -u -p -r1.33 sndiod.c > --- sndiod.c 26 Jun 2018 07:12:35 - 1.33 > +++ sndiod.c 30 Jul 2018 09:18:32 - > @@ -378,6 +378,8 @@ start_helper(int background) > setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) > err(1, "cannot drop privileges"); > } > + if (unveil("/dev", "rw") < 0) > + err(1, "unveil"); > if (pledge("stdio sendfd rpath wpath", NULL) < 0) > err(1, "pledge"); > while (file_poll()) > >
Re: unveil: incomplete unveil_flagmatch semantic
yeah the latter will be the way to go On Mon, Jul 30, 2018 at 06:02 Sebastien Marie wrote: > Hi, > > I think unveil_flagmatch() isn't complete and/or has not the right > semantic. > > A bit of internals for starting (I will speak about ni_pledge, people > that know what it is and how it works with pledge/unveil could go to > "what is the problem" part). > > unveil(2) works with the syscall annotation introduced for pledge(2). > > When kernel needs to resolv a name to vnode, it used namei() function. > For that it initializes a special structure used as namei() argument: > `struct nameidata`. This struct has a field `ni_pledge` used to let vfs > subsystem know what kind of syscalls called it. This way, underline > subsystem doesn't have to know all syscalls, and could work on them by > "group". > > For example, when you call open(2), depending the flags argument used, > ni_pledge will contain PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. The > subsystem has a quick and accurate view of the intented usage of the > vnode. > > pledge(2) uses it to check that you have the promise of intent to use, > and unveil(2) uses it too in a similar way, in particular in > unveil_flagmatch() to check if the process has the accurate permission > for this particular vnode. > > > > Now, what is the problem with unveil_flagmatch() ? > > If I exclude the PLEDGE_STAT stuff from the equation (I am not still > convinced it is the right way to do it - but it isn't the question for > now), unveil_flagmatch() has a default policy to allow the operation, > and only check for some flags in ni_pledge to deny it. > > For simple syscall like open(2) there is no problem. The possible > value of ni_pledge are composed with only PLEDGE_RPATH, PLEDGE_WPATH, or > PLEDGE_CPATH. > > But for some others syscalls, it isn't the case. > > For example, chmod(2) will set ni_pledge to "PLEDGE_FATTR | > PLEDGE_RPATH". For pledge(2), it means you must have "fattr" (capability > to change attribute on the node) and "rpath" (capability to read the > node) promise to use such syscall. > > As unveil(2) doesn't have the "fattr" concept, and unveil_flagmatch() > will check only for PLEDGE_RPATH, PLEDGE_WPATH, PLEDGE_CPATH and > PLEDGE_EXEC, we end with unsound policy: you could use chmod(2) with just > "r" on unveiled part of the filesystem. > > Some others flags could occurs in ni_pledge: > - PLEDGE_CHOWN: chown(2) family > - PLEDGE_DPATH: mknod(2), mkfifo(2) > - PLEDGE_FATTR: utimes(2) family, chmod(2) family, truncate(2), chflags(2) > - PLEDGE_TTY: revoke(2) > - PLEDGE_UNIX: bind(2), connect(2) > > unveil_flagmatch() has a special case for "" flag: it means deny. > but as soon as you have "r", "w", "x" or "c", you have an allow policy > by default. Check will be done only for a part of ni_pledge. > > > I see basically two possibilities: > - extending unveil(2) permissions to match pledge(2) flags - but I don't > like it, unveil(2) should be kept simple. > > - having a separate namespace for unveil and pledge flags (to avoid > confusion), and translating all pledge flags to unveil flags. > > PLEDGE_RPATH => UNVEIL_READ > PLEDGE_WPATH => UNVEIL_WRITE > PLEDGE_CPATH => UNVEIL_CREATE > PLEDGE_EXEC => UNVEIL_EXEC > PLEDGE_CHOWN => UNVEIL_WRITE > PLEDGE_DPATH => UNVEIL_CREATE > PLEDGE_FATTR => UNVEIL_WRITE > PLEDGE_TTY => UNVEIL_WRITE > PLEDGE_UNIX => UNVEIL_READ|UNVEIL_WRITE (requiring both) > any others => panic(9) > > This way, we could be really exhaustive in unveil_flagmatch() without > having to bother for future PLEDGE flag addition (as we will panic(9) if > some developer doesn't add it where intented). > > Thanks. > -- > Sebastien Marie >
Re: simplefb(4): display color depth when attaching
> Date: Mon, 30 Jul 2018 15:09:58 +0200 > From: Frederic Cambus > > Hi tech@, > > Here is a diff to display color depth alongside resolution when > attaching simplefb(4). > > Compile tested only, the only arm64 device I own is headless. > > Comments? OK? ok kettenis@ > Index: sys/dev/fdt/simplefb.c > === > RCS file: /cvs/src/sys/dev/fdt/simplefb.c,v > retrieving revision 1.3 > diff -u -p -r1.3 simplefb.c > --- sys/dev/fdt/simplefb.c18 Dec 2017 10:13:45 - 1.3 > +++ sys/dev/fdt/simplefb.c29 Jul 2018 21:15:13 - > @@ -154,7 +154,7 @@ simplefb_attach(struct device *parent, s > ri->ri_flg &= ~RI_CLEAR; > } > > - printf(": %dx%d\n", ri->ri_width, ri->ri_height); > + printf(": %dx%d, %dbpp\n", ri->ri_width, ri->ri_height, ri->ri_depth); > > ri->ri_flg |= RI_VCONS; > rasops_init(ri, SIMPLEFB_HEIGHT, SIMPLEFB_WIDTH); > >
simplefb(4): display color depth when attaching
Hi tech@, Here is a diff to display color depth alongside resolution when attaching simplefb(4). Compile tested only, the only arm64 device I own is headless. Comments? OK? Index: sys/dev/fdt/simplefb.c === RCS file: /cvs/src/sys/dev/fdt/simplefb.c,v retrieving revision 1.3 diff -u -p -r1.3 simplefb.c --- sys/dev/fdt/simplefb.c 18 Dec 2017 10:13:45 - 1.3 +++ sys/dev/fdt/simplefb.c 29 Jul 2018 21:15:13 - @@ -154,7 +154,7 @@ simplefb_attach(struct device *parent, s ri->ri_flg &= ~RI_CLEAR; } - printf(": %dx%d\n", ri->ri_width, ri->ri_height); + printf(": %dx%d, %dbpp\n", ri->ri_width, ri->ri_height, ri->ri_depth); ri->ri_flg |= RI_VCONS; rasops_init(ri, SIMPLEFB_HEIGHT, SIMPLEFB_WIDTH);
[diff] acme-client - fix err messages and style in parse.y
[The non-style(9) parts of this were sent previously off-list to a few.] The diff: - fixes error messages - one copy/pasto - two with old(?) syntax "use" rather than "sign with" - applies some style(9) - lots of "return (x);" ==> "return x;" - a few "if (x)" ==> "if (x != something)" Ross Index: parse.y === RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v retrieving revision 1.26 diff -u -p -r1.26 parse.y --- parse.y 29 Jul 2018 12:46:31 - 1.26 +++ parse.y 30 Jul 2018 11:56:47 - @@ -311,7 +311,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altn | DOMAIN FULL CHAIN CERT STRING { char *s; if (domain->fullchain != NULL) { - yyerror("duplicate chain"); + yyerror("duplicate full chain"); YYERROR; } if ((s = strdup($5)) == NULL) @@ -326,13 +326,13 @@ domainoptsl : ALTERNATIVE NAMES '{' altn | SIGN WITH STRING { char *s; if (domain->auth != NULL) { - yyerror("duplicate use"); + yyerror("duplicate sign with"); YYERROR; } if ((s = strdup($3)) == NULL) err(EXIT_FAILURE, "strdup"); if (authority_find(conf, s) == NULL) { - yyerror("use: unknown authority"); + yyerror("sign with: unknown authority"); free(s); YYERROR; } @@ -402,7 +402,7 @@ yyerror(const char *fmt, ...) int kw_cmp(const void *k, const void *e) { - return (strcmp(k, ((const struct keywords *)e)->k_name)); + return strcmp(k, ((const struct keywords *)e)->k_name); } int @@ -431,10 +431,10 @@ lookup(char *s) p = bsearch(s, keywords, sizeof(keywords)/sizeof(keywords[0]), sizeof(keywords[0]), kw_cmp); - if (p) - return (p->k_val); + if (p != NULL) + return p->k_val; else - return (STRING); + return STRING; } #defineSTART_EXPAND1 @@ -460,7 +460,7 @@ igetc(void) else break; } - return (c); + return c; } int @@ -474,9 +474,9 @@ lgetc(int quotec) "quoted string"); if (file == topfile || popfile() == EOF) return (EOF); - return (quotec); + return quotec; } - return (c); + return c; } while ((c = igetc()) == '\\') { @@ -497,7 +497,7 @@ lgetc(int quotec) */ if (file->eof_reached == 0) { file->eof_reached = 1; - return ('\n'); + return '\n'; } while (c == EOF) { if (file == topfile || popfile() == EOF) @@ -505,7 +505,7 @@ lgetc(int quotec) c = igetc(); } } - return (c); + return c; } void @@ -539,7 +539,7 @@ findeol(void) if (c == EOF) break; } - return (ERROR); + return ERROR; } int @@ -562,11 +562,11 @@ top: if (c == '$' && !expanding) { while (1) { if ((c = lgetc(0)) == EOF) - return (0); + return 0; if (p + 1 >= buf + sizeof(buf) - 1) { yyerror("string too long"); - return (findeol()); + return findeol(); } if (isalnum(c) || c == '_') { *p++ = c; @@ -579,7 +579,7 @@ top: val = symget(buf); if (val == NULL) { yyerror("macro '%s' not defined", buf); - return (findeol()); + return findeol(); } p = val + strlen(val) - 1; lungetc(DONE_EXPAND); @@ -597,13 +597,13 @@ top: quotec = c; while (1) { if ((c = lgetc(quotec)) == EOF) - return (0); + return 0; if (c == '\n') { file->lineno++; continue; } else if (c == '\\') {
unveil: incomplete unveil_flagmatch semantic
Hi, I think unveil_flagmatch() isn't complete and/or has not the right semantic. A bit of internals for starting (I will speak about ni_pledge, people that know what it is and how it works with pledge/unveil could go to "what is the problem" part). unveil(2) works with the syscall annotation introduced for pledge(2). When kernel needs to resolv a name to vnode, it used namei() function. For that it initializes a special structure used as namei() argument: `struct nameidata`. This struct has a field `ni_pledge` used to let vfs subsystem know what kind of syscalls called it. This way, underline subsystem doesn't have to know all syscalls, and could work on them by "group". For example, when you call open(2), depending the flags argument used, ni_pledge will contain PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. The subsystem has a quick and accurate view of the intented usage of the vnode. pledge(2) uses it to check that you have the promise of intent to use, and unveil(2) uses it too in a similar way, in particular in unveil_flagmatch() to check if the process has the accurate permission for this particular vnode. Now, what is the problem with unveil_flagmatch() ? If I exclude the PLEDGE_STAT stuff from the equation (I am not still convinced it is the right way to do it - but it isn't the question for now), unveil_flagmatch() has a default policy to allow the operation, and only check for some flags in ni_pledge to deny it. For simple syscall like open(2) there is no problem. The possible value of ni_pledge are composed with only PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. But for some others syscalls, it isn't the case. For example, chmod(2) will set ni_pledge to "PLEDGE_FATTR | PLEDGE_RPATH". For pledge(2), it means you must have "fattr" (capability to change attribute on the node) and "rpath" (capability to read the node) promise to use such syscall. As unveil(2) doesn't have the "fattr" concept, and unveil_flagmatch() will check only for PLEDGE_RPATH, PLEDGE_WPATH, PLEDGE_CPATH and PLEDGE_EXEC, we end with unsound policy: you could use chmod(2) with just "r" on unveiled part of the filesystem. Some others flags could occurs in ni_pledge: - PLEDGE_CHOWN: chown(2) family - PLEDGE_DPATH: mknod(2), mkfifo(2) - PLEDGE_FATTR: utimes(2) family, chmod(2) family, truncate(2), chflags(2) - PLEDGE_TTY: revoke(2) - PLEDGE_UNIX: bind(2), connect(2) unveil_flagmatch() has a special case for "" flag: it means deny. but as soon as you have "r", "w", "x" or "c", you have an allow policy by default. Check will be done only for a part of ni_pledge. I see basically two possibilities: - extending unveil(2) permissions to match pledge(2) flags - but I don't like it, unveil(2) should be kept simple. - having a separate namespace for unveil and pledge flags (to avoid confusion), and translating all pledge flags to unveil flags. PLEDGE_RPATH => UNVEIL_READ PLEDGE_WPATH => UNVEIL_WRITE PLEDGE_CPATH => UNVEIL_CREATE PLEDGE_EXEC => UNVEIL_EXEC PLEDGE_CHOWN => UNVEIL_WRITE PLEDGE_DPATH => UNVEIL_CREATE PLEDGE_FATTR => UNVEIL_WRITE PLEDGE_TTY => UNVEIL_WRITE PLEDGE_UNIX => UNVEIL_READ|UNVEIL_WRITE (requiring both) any others => panic(9) This way, we could be really exhaustive in unveil_flagmatch() without having to bother for future PLEDGE flag addition (as we will panic(9) if some developer doesn't add it where intented). Thanks. -- Sebastien Marie
Re: unveil in sndiod
On Mon, Jul 30, 2018 at 11:26:16AM +0200, Alexandre Ratchov wrote: > > The other sndiod process has neither of rpath, wpath, cpath, or exec, > so it doesn't need unveil, right? I am just replying for this aspect of unveil/pledge. Yes, if the process doesn't have such promises, calling unveil(2) is unnecessary. In fact, if you called unveil(2) previously, when you will call pledge(2), the kernel code will check if you need your unveil configuration or not, and free it if it isn't the case. -- Sebastien Marie
Re: [diff] acme-client - clean up main.c
Hi, thanks for your diff. Ross L Richardson(open...@rlr.id.au) on 2018.07.30 11:25:08 +1000: > > [This diff is very similar to one I sent previously off-list to a few.] > > Just some style(9) and simple cleanup: > - order getopt string and switch cases commited, left f: at the end so its the same as in usage and manpage. > - add space between "if" and "(" > - wrap a long line > - return rather than exit() from main() commited. > - move chngdir == NULL test to where it belongs modified a bit. > - is there any reason >404/* Jail: sandbox, file-system, user. */ >405 >406if (pledge("stdio", NULL) == -1) { >407warn("pledge"); >408exit(EXIT_FAILURE); >409} >410 > shouldn't just use err() ? now its if (pledge("stdio", NULL) == -1) err(EXIT_FAILURE, "pledge"); > If so, then the exit() should still be return, shouldn't it? no, if you use err() there is no need for return. > - should the exit() calls from the various forked child processes > be _exit()? maybe.
unveil in sndiod
A trivial diff for the sndiod "device helper" process. All this process does is to open the device and pass it to the main process. So it can be restricted to /dev. The other sndiod process has neither of rpath, wpath, cpath, or exec, so it doesn't need unveil, right? Index: sndiod.c === RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v retrieving revision 1.33 diff -u -p -r1.33 sndiod.c --- sndiod.c26 Jun 2018 07:12:35 - 1.33 +++ sndiod.c30 Jul 2018 09:18:32 - @@ -378,6 +378,8 @@ start_helper(int background) setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) err(1, "cannot drop privileges"); } + if (unveil("/dev", "rw") < 0) + err(1, "unveil"); if (pledge("stdio sendfd rpath wpath", NULL) < 0) err(1, "pledge"); while (file_poll())
nsd 4.1.23
works for me[tm] OK? NSD versions 4.1.22 and before are vulnerable in comparing TSIG information and this can be used to discover a TSIG secret. NSD uses TSIG to protect zone transfers. The TSIG code uses a secret key to protect the data. The secret key is shared with both sides of the zone transfer connection. The comparison code in NSD was not time insensitive, causing the potential for an attacker to use timing information to discover data about the key contents. NSD versions from 2.2.0 to 4.1.22 are vulnerable. Upgrade to 4.1.23 or newer to get the fix. It was reported by Ondrej Sury (ISC). diff --git configure configure index e034b5441ec..79f500f50fd 100644 --- configure +++ configure @@ -1,6 +1,6 @@ #! /bin/sh # Guess values for system-dependent variables and create Makefiles. -# Generated by GNU Autoconf 2.69 for NSD 4.1.22. +# Generated by GNU Autoconf 2.69 for NSD 4.1.23. # # Report bugs to . # @@ -580,8 +580,8 @@ MAKEFLAGS= # Identity of this package. PACKAGE_NAME='NSD' PACKAGE_TARNAME='nsd' -PACKAGE_VERSION='4.1.22' -PACKAGE_STRING='NSD 4.1.22' +PACKAGE_VERSION='4.1.23' +PACKAGE_STRING='NSD 4.1.23' PACKAGE_BUGREPORT='nsd-b...@nlnetlabs.nl' PACKAGE_URL='' @@ -1286,7 +1286,7 @@ if test "$ac_init_help" = "long"; then # Omit some internal or obsolete options to make the list less imposing. # This message is too long to be a string in the A/UX 3.1 sh. cat <<_ACEOF -\`configure' configures NSD 4.1.22 to adapt to many kinds of systems. +\`configure' configures NSD 4.1.23 to adapt to many kinds of systems. Usage: $0 [OPTION]... [VAR=VALUE]... @@ -1347,7 +1347,7 @@ fi if test -n "$ac_init_help"; then case $ac_init_help in - short | recursive ) echo "Configuration of NSD 4.1.22:";; + short | recursive ) echo "Configuration of NSD 4.1.23:";; esac cat <<\_ACEOF @@ -1496,7 +1496,7 @@ fi test -n "$ac_init_help" && exit $ac_status if $ac_init_version; then cat <<\_ACEOF -NSD configure 4.1.22 +NSD configure 4.1.23 generated by GNU Autoconf 2.69 Copyright (C) 2012 Free Software Foundation, Inc. @@ -2205,7 +2205,7 @@ cat >config.log <<_ACEOF This file contains any messages produced by compilers while running configure, to aid debugging if configure makes a mistake. -It was created by NSD $as_me 4.1.22, which was +It was created by NSD $as_me 4.1.23, which was generated by GNU Autoconf 2.69. Invocation command line was $ $0 $@ @@ -9784,7 +9784,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1 # report actual input values of CONFIG_FILES etc. instead of their # values after options handling. ac_log=" -This file was extended by NSD $as_me 4.1.22, which was +This file was extended by NSD $as_me 4.1.23, which was generated by GNU Autoconf 2.69. Invocation command line was CONFIG_FILES= $CONFIG_FILES @@ -9846,7 +9846,7 @@ _ACEOF cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/&/g'`" ac_cs_version="\\ -NSD config.status 4.1.22 +NSD config.status 4.1.23 configured by $0, generated by GNU Autoconf 2.69, with options \\"\$ac_cs_config\\" diff --git configure.ac configure.ac index 4c6772a47bd..c17501cb5a0 100644 --- configure.ac +++ configure.ac @@ -4,7 +4,7 @@ dnl sinclude(acx_nlnetlabs.m4) -AC_INIT(NSD,4.1.22,nsd-b...@nlnetlabs.nl) +AC_INIT(NSD,4.1.23,nsd-b...@nlnetlabs.nl) AC_CONFIG_HEADER([config.h]) CFLAGS="$CFLAGS" diff --git tsig.c tsig.c index a7cc66ee184..b0e40116f74 100644 --- tsig.c +++ tsig.c @@ -475,7 +475,7 @@ tsig_verify(tsig_record_type *tsig) &tsig->prior_mac_size); if (tsig->mac_size != tsig->prior_mac_size - || memcmp(tsig->mac_data, + || CRYPTO_memcmp(tsig->mac_data, tsig->prior_mac_data, tsig->mac_size) != 0) { -- I'm not entirely sure you are real.