Re: pcidump(8): add missing PCI classes
On Fri, Mar 05, 2021 at 09:22:28AM -0700, Theo de Raadt wrote:
> Fix dump() to convert subclass == NULL into something else, or maybe the
> fix should be in pci_subclass() itself.
My initial mistake was to use zero in an empty list. This leads to one
element in the list, which causes wrong list handling in the followup
code path.
So, the following diff remove the zero from the list. Also, add a check
for ps->name is NULL, to prevent dump() to print (null). And fix a
useless line break while here.
OK?
Thanks,
Jan
Index: pcidump.c
===
RCS file: /cvs/src/usr.sbin/pcidump/pcidump.c,v
retrieving revision 1.62
diff -u -p -r1.62 pcidump.c
--- pcidump.c 5 Mar 2021 12:57:20 - 1.62
+++ pcidump.c 5 Mar 2021 17:05:40 -
@@ -1296,8 +1296,8 @@ static const struct pci_subclass pci_sub
{ PCI_SUBCLASS_DASP_MISC, "Miscellaneous" },
};
-static const struct pci_subclass pci_subclass_accelerator[] = {0};
-static const struct pci_subclass pci_subclass_instrumentation[] = {0};
+static const struct pci_subclass pci_subclass_accelerator[] = {};
+static const struct pci_subclass pci_subclass_instrumentation[] = {};
#define CLASS(_c, _n, _s) { \
.class = _c, \
@@ -1389,7 +1389,6 @@ pci_class_name(pci_class_t class)
return (pc->name);
}
-
static const char *
pci_subclass_name(pci_class_t class, pci_subclass_t subclass)
{
@@ -1401,7 +1400,7 @@ pci_subclass_name(pci_class_t class, pci
return ("(unknown)");
ps = pci_subclass(pc, subclass);
- if (ps == NULL)
+ if (ps == NULL || ps->name == NULL)
return ("(unknown)");
return (ps->name);
Re: pcidump(8): add missing PCI classes
On Fri, Mar 05, 2021 at 04:13:53PM +0100, Mark Kettenis wrote: > > Date: Fri, 5 Mar 2021 12:05:38 +0100 > > From: Jan Klemkow > > Content-Type: text/plain; charset=us-ascii > > Content-Disposition: inline > > > > Hi, > > > > this diff adds the missing PCI classes Accelerator and Instrumentation. > > Thus, we can replace a few unknown in its output: > > > > - 0x0008: Class: 13 (unknown), Subclass: 00 (unknown), > > + 0x0008: Class: 13 Instrumentation, Subclass: 00 (null), > > Is this "(null)" the result of printing a null pointer? That would be > not so good. What do you suggest to use instead? Empty String, or "unknown"? It is vendor specific. Thanks, Jan
slaacd(8): various code cleanup & withdraw nameservers when interface goes down
Here are a bunch of diffs on top for Fernando's RFC 8981 work, but is
independent of it, might apply without it, I haven't tried.
I started to cleanup slaacd(8) by applying things I learned while
implementing dhcpleased(8). I'm nowhere near done but it got to the
point where it fixes two bugs:
1) pay better attention to link state
2) withdraw nameserver proposals when an interface goes down (either
due to ifconfig $IF down or because it loses link.).
Tests, OKs?
(I'm sending it split up for easier review, patch(1) can deal with
just applying the full email if you just want to test.)
commit d1f647899f7ee60d326360d3a19bb2f69fe7edc0
Author: Florian Obser
Date: Thu Mar 4 17:54:27 2021 +0100
Introduce engine_update_if().
This was too much code in the imsg handler.
diff --git engine.c engine.c
index 61b8f850d9d..7f867650b13 100644
--- engine.c
+++ engine.c
@@ -277,6 +277,7 @@ void deprecate_all_proposals(struct
slaacd_iface *);
struct slaacd_iface*get_slaacd_iface_by_id(uint32_t);
voidremove_slaacd_iface(uint32_t);
voidfree_ra(struct radv *);
+voidengine_update_iface(struct imsg_ifinfo *);
voidparse_ra(struct slaacd_iface *, struct imsg_ra *);
voidgen_addr(struct slaacd_iface *, struct radv_prefix *,
struct address_proposal *, int);
@@ -684,87 +685,7 @@ engine_dispatch_main(int fd, short event, void *bula)
fatalx("%s: IMSG_UPDATE_IF wrong length: %lu",
__func__, IMSG_DATA_SIZE(imsg));
memcpy(&imsg_ifinfo, imsg.data, sizeof(imsg_ifinfo));
-
- iface = get_slaacd_iface_by_id(imsg_ifinfo.if_index);
- if (iface == NULL) {
- if ((iface = calloc(1, sizeof(*iface))) == NULL)
- fatal("calloc");
- evtimer_set(&iface->timer, iface_timeout,
- iface);
- iface->if_index = imsg_ifinfo.if_index;
- iface->rdomain = imsg_ifinfo.rdomain;
- iface->running = imsg_ifinfo.running;
- if (iface->running)
- start_probe(iface);
- else
- iface->state = IF_DOWN;
- iface->autoconfprivacy =
- imsg_ifinfo.autoconfprivacy;
- iface->soii = imsg_ifinfo.soii;
- memcpy(&iface->hw_address,
- &imsg_ifinfo.hw_address,
- sizeof(struct ether_addr));
- memcpy(&iface->ll_address,
- &imsg_ifinfo.ll_address,
- sizeof(struct sockaddr_in6));
- memcpy(iface->soiikey, imsg_ifinfo.soiikey,
- sizeof(iface->soiikey));
- LIST_INIT(&iface->radvs);
- LIST_INSERT_HEAD(&slaacd_interfaces,
- iface, entries);
- LIST_INIT(&iface->addr_proposals);
- LIST_INIT(&iface->dfr_proposals);
- LIST_INIT(&iface->rdns_proposals);
- } else {
- int need_refresh = 0;
-
- if (iface->autoconfprivacy !=
- imsg_ifinfo.autoconfprivacy) {
- iface->autoconfprivacy =
- imsg_ifinfo.autoconfprivacy;
- need_refresh = 1;
- }
-
- if (iface->soii !=
- imsg_ifinfo.soii) {
- iface->soii =
- imsg_ifinfo.soii;
- need_refresh = 1;
- }
-
- if (memcmp(&iface->hw_address,
- &imsg_ifinfo.hw_address,
- sizeof(struct ether_addr)) != 0) {
- memcpy(&iface->hw_address,
- &imsg_ifinfo.hw_address,
- sizeof(struct ether_addr));
- need_refresh = 1;
- }
- if (memcmp(iface->soiikey,
- imsg_ifi
Re: slacd(8): Implement RFC 8981 (revised RFC 4941, IPv6 Temporary Address Extensions) (revised patch)
Anyone? I'll probably put this in tomorrow. Diffs are piling up...
On Thu, Mar 04, 2021 at 11:47:10AM +0100, Florian Obser wrote:
> Works fine here, OK florian
>
> On Wed, Mar 03, 2021 at 08:50:59PM -0300, Fernando Gont wrote:
> > This revised patch adresses a minor issue pointed out by Florian (avoid
> > floating-point math). At this point this is unnecessary, since the
> > IPv6 temporary address lifetimes are not configurable.
> >
> > P.S.: Patch also available at:
> > https://www.gont.com.ar/files/fgont-patch-rfc8981-v0.3.diff
> >
> > Thanks,
> > Fernando
> >
> >
> >
> >
> > diff --git engine.c engine.c
> > index 4160d798261..3ddf0303dd9 100644
> > --- engine.c
> > +++ engine.c
> > @@ -88,11 +88,15 @@
> > #defineRTR_SOLICITATION_INTERVAL 4
> > #defineMAX_RTR_SOLICITATIONS 3
> >
> > -/* constants for RFC 4941 autoconf privacy extension */
> > -#define PRIV_MAX_DESYNC_FACTOR 600 /* 10 minutes */
> > +/*
> > + * Constants for RFC 8981 autoconf privacy extensions
> > + *
> > + * PRIV_PREFERRED_LIFETIME > (PRIV_MAX_DESYNC_FACTOR + PRIV_REGEN_ADVANCE)
> > + */
> > #define PRIV_VALID_LIFETIME172800 /* 2 days */
> > #define PRIV_PREFERRED_LIFETIME86400 /* 1 day */
> > -#definePRIV_REGEN_ADVANCE 5 /* 5 seconds */
> > +#define PRIV_MAX_DESYNC_FACTOR 34560 /* PRIV_PREFERRED_LIFETIME *
> > 0.4 */
> > +#define PRIV_REGEN_ADVANCE 5 /* 5 seconds */
> >
> > enum if_state {
> > IF_DOWN,
> > @@ -198,6 +202,7 @@ struct address_proposal {
> > uint8_t prefix_len;
> > uint32_t vltime;
> > uint32_t pltime;
> > + uint32_t desync_factor;
> > uint8_t soiikey[SLAACD_SOIIKEY_LEN];
> > uint32_t mtu;
> > };
> > @@ -327,8 +332,6 @@ static struct imsgev*iev_frontend;
> > static struct imsgev *iev_main;
> > int64_t proposal_id;
> >
> > -uint32_tdesync_factor;
> > -
> > void
> > engine_sig_handler(int sig, short event, void *arg)
> > {
> > @@ -399,8 +402,6 @@ engine(int debug, int verbose)
> >
> > LIST_INIT(&slaacd_interfaces);
> >
> > - desync_factor = arc4random_uniform(PRIV_MAX_DESYNC_FACTOR);
> > -
> > event_dispatch();
> >
> > engine_shutdown();
> > @@ -1858,14 +1859,18 @@ update_iface_ra_prefix(struct slaacd_iface *iface,
> > struct radv *ra,
> >
> > if (addr_proposal->privacy) {
> > struct timespec now;
> > - int64_t ltime;
> > + int64_t ltime, mtime;
> >
> > if (clock_gettime(CLOCK_MONOTONIC, &now))
> > fatal("clock_gettime");
> >
> > - ltime = MINIMUM(addr_proposal->created.tv_sec +
> > - PRIV_PREFERRED_LIFETIME - desync_factor,
> > - now.tv_sec + prefix->pltime) - now.tv_sec;
> > + mtime = addr_proposal->created.tv_sec +
> > + PRIV_PREFERRED_LIFETIME -
> > + addr_proposal->desync_factor;
> > +
> > + ltime = MINIMUM(mtime, now.tv_sec + prefix->pltime) -
> > + now.tv_sec;
> > +
> > pltime = ltime > 0 ? ltime : 0;
> >
> > ltime = MINIMUM(addr_proposal->created.tv_sec +
> > @@ -1873,7 +1878,7 @@ update_iface_ra_prefix(struct slaacd_iface *iface,
> > struct radv *ra,
> > now.tv_sec;
> > vltime = ltime > 0 ? ltime : 0;
> >
> > - if (pltime > PRIV_REGEN_ADVANCE)
> > + if ((mtime - now.tv_sec) > PRIV_REGEN_ADVANCE)
> > found_privacy = 1;
> > } else {
> > pltime = prefix->pltime;
> > @@ -1919,11 +1924,11 @@ update_iface_ra_prefix(struct slaacd_iface *iface,
> > struct radv *ra,
> >
> > /* privacy addresses do not depend on eui64 */
> > if (!found_privacy && iface->autoconfprivacy) {
> > - if (prefix->pltime < desync_factor) {
> > + if (prefix->pltime < PRIV_REGEN_ADVANCE) {
> > log_warnx("%s: pltime from %s is too small: %d < %d; "
> > "not generating privacy address", __func__,
> > sin6_to_str(&ra->from), prefix->pltime,
> > - desync_factor);
> > + PRIV_REGEN_ADVANCE);
> > } else
> > /* new privacy proposal */
> > gen_address_proposal(iface, ra, prefix, 1);
> > @@ -2055,8 +2060,11 @@ gen_address_proposal(struct slaacd_iface *iface,
> > struct radv *ra, struct
> > if (privacy) {
> > addr_proposal->vltime = MINIMUM(prefix->vltime,
> > PRIV_VALID_LIFETIME);
> > + addr_proposal->desync_factor =
> >
Re: Ignore /tmp/.Xauth* in daily
чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
>
> Hello all.
>
> Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> /tmp/.Xauth* in daily cleanup, don't we?
>
> Found the hard way a few minutes ago on my X240.
Thanks sthen@, I've realized this happens only when xenodm could not
create ~/.Xauthority. In my case this happens because my laptop starts
with /home mounted read-only, but there may be others. Mattieu, the
xenodm logic itself is correct, right? If yes, anyone brave enough to
okay the diff below then? :-)
> Index: daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.95
> diff -u -p -r1.95 daily
> --- daily 20 Oct 2020 22:42:29 - 1.95
> +++ daily 3 Mar 2021 22:58:28 -
> @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> cd /tmp && {
> find -x . \
> \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> - -o -path './tmux-*' \) \
> + -o -path './tmux-*' -o -path './.Xauth*' \) \
> -prune -o -type f -atime +7 -delete 2>/dev/null
> find -x . -type d -mtime +1 ! -path ./vi.recover ! -path ./.X11-unix \
> ! -path ./.ICE-unix ! -name . \
--
WBR,
Vadim Zhukov
Re: rpki-client validate URI function
On Fri, Mar 05, 2021 at 05:36:53PM +0100, Claudio Jeker wrote: [...] > Here we go. This should be better. ok tb
Re: rpki-client validate URI function
On Fri, Mar 05, 2021 at 04:58:44PM +0100, Claudio Jeker wrote:
> On Fri, Mar 05, 2021 at 04:08:55PM +0100, Theo Buehler wrote:
> > On Fri, Mar 05, 2021 at 01:48:43PM +0100, Claudio Jeker wrote:
> > > Instead of adding similar checks all over the place introduce a
> > > valid_uri() function that checks if a URI is valid enough for rpki-client.
> > > rpki-client does not accept files or directories starting with ., bails on
> > > URI that have strange characters and valid_uri() will also check that the
> > > protocol is the expected one if provided.
> > >
> > > This diff converts the 4 places where URI are handled.
> >
> > I'm ok with this. A few comments inline.
> >
> > > --
> > > :wq Claudio
> > >
> > > Index: cert.c
> > > ===
> > > RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> > > retrieving revision 1.27
> > > diff -u -p -r1.27 cert.c
> > > --- cert.c18 Feb 2021 16:23:17 - 1.27
> > > +++ cert.c5 Mar 2021 12:33:50 -
> > > @@ -19,7 +19,6 @@
> > >
> > > #include
> > > #include
> > > -#include
> > > #include
> > > #include
> > > #include
> > > @@ -140,34 +139,22 @@ sbgp_addr(struct parse *p,
> > > * Returns zero on failure, non-zero on success.
> > > */
> > > static int
> > > -sbgp_sia_resource_notify(struct parse *p,
> > > - const unsigned char *d, size_t dsz)
> > > +sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
> > > {
> > > - size_t i;
> > > -
> > > if (p->res->notify != NULL) {
> > > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > > "Notify location already specified", p->fn);
> > > return 0;
> > > }
> > >
> > > + if ((p->res->notify = strndup(d, dsz)) == NULL)
> >
> > This was slightly more robust before as p->res->notify would only get
> > assigned after all checks passed. That way it's guaranteed that an
> > invalid string is never used erroneously.
> >
> > Perhaps it's not worth the extra gymnastics of strnduping to a local
> > variable and freeing it on error. After all, it will currently get
> > freed in cert_parse_inner() on validation failure...
>
> I'll think about this a bit more. My other use cases use normal strings so
> that's why I decided to switch it here too. Maybe it is better to pass in
> the string length into valid_uri.
>
> > > + err(1, NULL);
> > > +
> > > /* Make sure it's a https:// address. */
> > > - if (dsz <= 8 || strncasecmp(d, "https://";, 8)) {
> > > - warnx("%s: RFC 8182 section 3.2: not using https schema",
> > > - p->fn);
> > > + if (!valid_uri(p->res->notify, "https://";)) {
> > > + warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
> > > return 0;
> > > }
> > > - /* make sure only US-ASCII chars are in the URL */
> > > - for (i = 0; i < dsz; i++) {
> > > - if (isalnum(d[i]) || ispunct(d[i]))
> > > - continue;
> > > - warnx("%s: invalid URI", p->fn);
> > > - return 0;
> > > - }
> > > -
> > > -
> > > - if ((p->res->notify = strndup((const char *)d, dsz)) == NULL)
> > > - err(1, NULL);
> > >
> > > return 1;
> > > }
> > > @@ -177,40 +164,29 @@ sbgp_sia_resource_notify(struct parse *p
> > > * Returns zero on failure, non-zero on success.
> > > */
> > > static int
> > > -sbgp_sia_resource_mft(struct parse *p,
> > > - const unsigned char *d, size_t dsz)
> > > +sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
> > > {
> > > - size_t i;
> > > -
> > > if (p->res->mft != NULL) {
> > > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > > "MFT location already specified", p->fn);
> > > return 0;
> > > }
> > >
> > > - /* Make sure it's an MFT rsync address. */
> > > - if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
> > > - warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
> > > - p->fn);
> > > - return 0;
> > > - }
> > > -
> > > if (strcasecmp(d + dsz - 4, ".mft") != 0) {
> >
> > Since you dropped the above check that ensures that dsz > 8, I think this
> > needs to be
> >
> > if (dsz < 4 || strcasecmp(...))
> >
> > to avoid a potential out-of-bounds access
> >
>
> Yup, your right.
>
> > > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > > - "invalid rsync URI suffix", p->fn);
> > > - return 0;
> > > - }
> > > - /* make sure only US-ASCII chars are in the URL */
> > > - for (i = 0; i < dsz; i++) {
> > > - if (isalnum(d[i]) || ispunct(d[i]))
> > > - continue;
> > > - warnx("%s: invalid URI", p->fn);
> > > + "not an MFT file", p->fn);
> > > return 0;
> > > }
> > >
> > > - if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
> > > + if ((p->res->mft = strndup(d, dsz)) == NULL)
> > > err(1, NULL);
> > >
> > > + /* Make sure it's an MFT rsync address. */
> > > + if (!valid_uri(p->res->mft,
Re: pcidump(8): add missing PCI classes
Fix dump() to convert subclass == NULL into something else, or maybe the fix should be in pci_subclass() itself. Converting NULL to %s into "(null)" is non-standard and has led to sloppier code, there have been efforts improve the whole tree enough that we could one day simply crash on such circumstance instead. We don't need more of these, so fix it. And pass the word around that accepting "(null)" is the wrong mindset. Jan Klemkow wrote: > On Fri, Mar 05, 2021 at 04:13:53PM +0100, Mark Kettenis wrote: > > > Date: Fri, 5 Mar 2021 12:05:38 +0100 > > > From: Jan Klemkow > > > Content-Type: text/plain; charset=us-ascii > > > Content-Disposition: inline > > > > > > Hi, > > > > > > this diff adds the missing PCI classes Accelerator and Instrumentation. > > > Thus, we can replace a few unknown in its output: > > > > > > - 0x0008: Class: 13 (unknown), Subclass: 00 (unknown), > > > + 0x0008: Class: 13 Instrumentation, Subclass: 00 (null), > > > > Is this "(null)" the result of printing a null pointer? That would be > > not so good. > > What do you suggest to use instead? Empty String, or "unknown"? It is > vendor specific. > > Thanks, > Jan >
Re: rpki-client validate URI function
On Fri, Mar 05, 2021 at 04:08:55PM +0100, Theo Buehler wrote:
> On Fri, Mar 05, 2021 at 01:48:43PM +0100, Claudio Jeker wrote:
> > Instead of adding similar checks all over the place introduce a
> > valid_uri() function that checks if a URI is valid enough for rpki-client.
> > rpki-client does not accept files or directories starting with ., bails on
> > URI that have strange characters and valid_uri() will also check that the
> > protocol is the expected one if provided.
> >
> > This diff converts the 4 places where URI are handled.
>
> I'm ok with this. A few comments inline.
>
> > --
> > :wq Claudio
> >
> > Index: cert.c
> > ===
> > RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> > retrieving revision 1.27
> > diff -u -p -r1.27 cert.c
> > --- cert.c 18 Feb 2021 16:23:17 - 1.27
> > +++ cert.c 5 Mar 2021 12:33:50 -
> > @@ -19,7 +19,6 @@
> >
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > @@ -140,34 +139,22 @@ sbgp_addr(struct parse *p,
> > * Returns zero on failure, non-zero on success.
> > */
> > static int
> > -sbgp_sia_resource_notify(struct parse *p,
> > - const unsigned char *d, size_t dsz)
> > +sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
> > {
> > - size_t i;
> > -
> > if (p->res->notify != NULL) {
> > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > "Notify location already specified", p->fn);
> > return 0;
> > }
> >
> > + if ((p->res->notify = strndup(d, dsz)) == NULL)
>
> This was slightly more robust before as p->res->notify would only get
> assigned after all checks passed. That way it's guaranteed that an
> invalid string is never used erroneously.
>
> Perhaps it's not worth the extra gymnastics of strnduping to a local
> variable and freeing it on error. After all, it will currently get
> freed in cert_parse_inner() on validation failure...
I'll think about this a bit more. My other use cases use normal strings so
that's why I decided to switch it here too. Maybe it is better to pass in
the string length into valid_uri.
> > + err(1, NULL);
> > +
> > /* Make sure it's a https:// address. */
> > - if (dsz <= 8 || strncasecmp(d, "https://";, 8)) {
> > - warnx("%s: RFC 8182 section 3.2: not using https schema",
> > - p->fn);
> > + if (!valid_uri(p->res->notify, "https://";)) {
> > + warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
> > return 0;
> > }
> > - /* make sure only US-ASCII chars are in the URL */
> > - for (i = 0; i < dsz; i++) {
> > - if (isalnum(d[i]) || ispunct(d[i]))
> > - continue;
> > - warnx("%s: invalid URI", p->fn);
> > - return 0;
> > - }
> > -
> > -
> > - if ((p->res->notify = strndup((const char *)d, dsz)) == NULL)
> > - err(1, NULL);
> >
> > return 1;
> > }
> > @@ -177,40 +164,29 @@ sbgp_sia_resource_notify(struct parse *p
> > * Returns zero on failure, non-zero on success.
> > */
> > static int
> > -sbgp_sia_resource_mft(struct parse *p,
> > - const unsigned char *d, size_t dsz)
> > +sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
> > {
> > - size_t i;
> > -
> > if (p->res->mft != NULL) {
> > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > "MFT location already specified", p->fn);
> > return 0;
> > }
> >
> > - /* Make sure it's an MFT rsync address. */
> > - if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
> > - warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
> > - p->fn);
> > - return 0;
> > - }
> > -
> > if (strcasecmp(d + dsz - 4, ".mft") != 0) {
>
> Since you dropped the above check that ensures that dsz > 8, I think this
> needs to be
>
> if (dsz < 4 || strcasecmp(...))
>
> to avoid a potential out-of-bounds access
>
Yup, your right.
> > warnx("%s: RFC 6487 section 4.8.8: SIA: "
> > - "invalid rsync URI suffix", p->fn);
> > - return 0;
> > - }
> > - /* make sure only US-ASCII chars are in the URL */
> > - for (i = 0; i < dsz; i++) {
> > - if (isalnum(d[i]) || ispunct(d[i]))
> > - continue;
> > - warnx("%s: invalid URI", p->fn);
> > + "not an MFT file", p->fn);
> > return 0;
> > }
> >
> > - if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
> > + if ((p->res->mft = strndup(d, dsz)) == NULL)
> > err(1, NULL);
> >
> > + /* Make sure it's an MFT rsync address. */
> > + if (!valid_uri(p->res->mft, "rsync://")) {
> > + warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
> > + return 0;
> > + }
> > +
> > return 1;
> > }
> >
> > @@ -219,35 +195,24 @@ sbgp_sia_resource_mft(struct parse *p,
>
Re: rpki-client validate filehash function
On Fri, Mar 05, 2021 at 03:15:48PM +0100, Claudio Jeker wrote:
> RRDP also uses SHA256 hashes to validate files (before withdraws and
> updates). Again move this from the implementation in mft.c to validate.c
> this way it can be reused.
>
> OK?
ok tb
> --
> :wq Claudio
>
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.51
> diff -u -p -r1.51 extern.h
> --- extern.h 5 Mar 2021 12:33:19 - 1.51
> +++ extern.h 5 Mar 2021 14:00:29 -
> @@ -354,6 +354,7 @@ intvalid_ta(const char *, struct auth
> int valid_cert(const char *, struct auth_tree *,
> const struct cert *);
> int valid_roa(const char *, struct auth_tree *, struct roa *);
> +int valid_filehash(const char *, const char *, size_t);
>
> /* Working with CMS files. */
>
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 mft.c
> --- mft.c 4 Mar 2021 14:24:17 - 1.28
> +++ mft.c 5 Mar 2021 12:33:50 -
> @@ -434,61 +434,32 @@ out:
> }
>
> /*
> - * Check the hash value of a file.
> + * Check all files and their hashes in a MFT structure.
> * Return zero on failure, non-zero on success.
> */
> -static int
> -mft_validfilehash(const char *fn, const struct mftfile *m)
> +int
> +mft_check(const char *fn, struct mft *p)
> {
> - charfilehash[SHA256_DIGEST_LENGTH];
> - charbuffer[8192];
> + size_t i;
> + int rc = 1;
> char*cp, *path = NULL;
> - SHA256_CTX ctx;
> - ssize_t nr;
> - int fd;
>
> /* Check hash of file now, but first build path for it */
> cp = strrchr(fn, '/');
> assert(cp != NULL);
> assert(cp - fn < INT_MAX);
> - if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn, m->file) == -1)
> - err(1, NULL);
>
> - if ((fd = open(path, O_RDONLY)) == -1) {
> - warn("%s: referenced file %s", fn, m->file);
> + for (i = 0; i < p->filesz; i++) {
> + const struct mftfile *m = &p->files[i];
> + if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn,
> + m->file) == -1)
> + err(1, NULL);
> + if (!valid_filehash(path, m->hash, sizeof(m->hash))) {
> + warnx("%s: bad message digest for %s", fn, m->file);
> + rc = 0;
> + }
> free(path);
> - return 0;
> }
> - free(path);
> -
> - SHA256_Init(&ctx);
> - while ((nr = read(fd, buffer, sizeof(buffer))) > 0) {
> - SHA256_Update(&ctx, buffer, nr);
> - }
> - close(fd);
> -
> - SHA256_Final(filehash, &ctx);
> - if (memcmp(m->hash, filehash, SHA256_DIGEST_LENGTH) != 0) {
> - warnx("%s: bad message digest for %s", fn, m->file);
> - return 0;
> - }
> -
> - return 1;
> -}
> -
> -/*
> - * Check all files and their hashes in a MFT structure.
> - * Return zero on failure, non-zero on success.
> - */
> -int
> -mft_check(const char *fn, struct mft *p)
> -{
> - size_t i;
> - int rc = 1;
> -
> - for (i = 0; i < p->filesz; i++)
> - if (!mft_validfilehash(fn, &p->files[i]))
> - rc = 0;
>
> return rc;
> }
> Index: validate.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 validate.c
> --- validate.c12 Sep 2020 15:46:48 - 1.11
> +++ validate.c5 Mar 2021 13:59:46 -
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -237,6 +238,37 @@ valid_roa(const char *fn, struct auth_tr
> tracewarn(a);
> return 0;
> }
> +
> + return 1;
> +}
> +
> +/*
> + * Validate a file by verifying the SHA256 hash of that file.
> + * Returns 1 if valid, 0 otherwise.
> + */
> +int
> +valid_filehash(const char *fn, const char *hash, size_t hlen)
> +{
> + SHA256_CTX ctx;
> + charfilehash[SHA256_DIGEST_LENGTH];
> + charbuffer[8192];
> + ssize_t nr;
> + int fd;
> +
> + if (hlen != sizeof(filehash))
> + errx(1, "bad hash size");
> +
> + if ((fd = open(fn, O_RDONLY)) == -1)
> + return 0;
> +
> + SHA256_Init(&ctx);
> + while ((nr = read(fd, buffer, sizeof(buffer))) > 0)
> + SHA256_Update(&ctx, buffer, nr);
> + close(fd);
> +
> + SHA256_Final(filehash, &ctx);
> + if (memcmp(hash, filehash, sizeof(filehash)) != 0)
> + return 0;
>
> return 1;
> }
>
Re: Fix assigning array variables in ksh
пт, 5 мар. 2021 г. в 18:14, Theo Buehler :
>
> On Sun, Feb 21, 2021 at 10:04:07PM +0300, Vadim Zhukov wrote:
> > Hello all.
> >
> > This continues the 'Another potential ksh bug?' thread on misc@:
> > https://marc.info/?l=openbsd-misc&m=160736700220621&w=2
> > This present is a bit too late for Christmas, but at least the Day of
> > Red Army is coming soon. I'm sure noone is against this patch then.
> >
> > The code in typeset() function, which is responsible for almost all
> > shell variable tweaking, contains a bug: in case of array it passes
> > "foo=var" instead of only variable name ("foo"), if the value was
> > ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
> > Generally speaking, we had creating a (temporary) variable via global()
> > in vpbase, and of course it didn't get the read-only marker.
> >
> > This fix is to use 'tvar' variable, which always contains shell
> > variable name, without value.
>
> ok tb
>
> > As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
> > I'm too lazy to check why. :-)
>
> I'm confused. I have 11 failing tests before and after your patch.
> Also IFS-null-1 has been passing for a long time.
Hm-m-m. Probably something in my environment... I'll experiment separately then.
Thanks!
Re: Fix assigning array variables in ksh
On Sun, Feb 21, 2021 at 10:04:07PM +0300, Vadim Zhukov wrote:
> Hello all.
>
> This continues the 'Another potential ksh bug?' thread on misc@:
> https://marc.info/?l=openbsd-misc&m=160736700220621&w=2
> This present is a bit too late for Christmas, but at least the Day of
> Red Army is coming soon. I'm sure noone is against this patch then.
>
> The code in typeset() function, which is responsible for almost all
> shell variable tweaking, contains a bug: in case of array it passes
> "foo=var" instead of only variable name ("foo"), if the value was
> ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
> Generally speaking, we had creating a (temporary) variable via global()
> in vpbase, and of course it didn't get the read-only marker.
>
> This fix is to use 'tvar' variable, which always contains shell
> variable name, without value.
ok tb
> As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
> I'm too lazy to check why. :-)
I'm confused. I have 11 failing tests before and after your patch.
Also IFS-null-1 has been passing for a long time.
>
> Okay anyone?
>
> --
> WBR,
> Vadim Zhukov
>
>
> Index: regress/bin/ksh/obsd-regress.t
> ===
> RCS file: /cvs/src/regress/bin/ksh/obsd-regress.t,v
> retrieving revision 1.10
> diff -u -p -r1.10 obsd-regress.t
> --- regress/bin/ksh/obsd-regress.t8 Dec 2018 21:03:51 - 1.10
> +++ regress/bin/ksh/obsd-regress.t21 Feb 2021 18:51:54 -
> @@ -503,3 +503,16 @@ description:
> stdin:
> kill -s SIGINFO $$
> ---
> +
> +name: overwrite-ro-array
> +description:
> + do not allow to override first element of a read-only array
> + via the non-array access.
> +stdin:
> + arr[0]=foo
> + readonly arr
> + arr=bar
> +expected-exit: e == 1
> +expected-stderr-pattern:
> + /: arr: is read only$/
> +---
> Index: bin/ksh/var.c
> ===
> RCS file: /cvs/src/bin/ksh/var.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 var.c
> --- bin/ksh/var.c 21 Feb 2020 18:21:23 - 1.71
> +++ bin/ksh/var.c 21 Feb 2021 18:51:54 -
> @@ -644,7 +644,7 @@ typeset(const char *var, int set, int cl
> global(tvar);
> set &= ~(LOCAL|LOCAL_COPY);
>
> - vpbase = (vp->flag & ARRAY) ? global(arrayname(var)) : vp;
> + vpbase = (vp->flag & ARRAY) ? global(arrayname(tvar)) : vp;
>
> /* only allow export flag to be set. at&t ksh allows any attribute to
>* be changed, which means it can be truncated or modified
>
Re: pcidump(8): add missing PCI classes
> Date: Fri, 5 Mar 2021 12:05:38 +0100
> From: Jan Klemkow
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> Hi,
>
> this diff adds the missing PCI classes Accelerator and Instrumentation.
> Thus, we can replace a few unknown in its output:
>
> - 0x0008: Class: 13 (unknown), Subclass: 00 (unknown),
> + 0x0008: Class: 13 Instrumentation, Subclass: 00 (null),
Is this "(null)" the result of printing a null pointer? That would be
not so good.
> Both Classes have vendor specific APIs. So, there are no predefined
> subclasses.
>
> OK?
>
> bye,
> Jan
>
> Index: pcidump.c
> ===
> RCS file: /cvs/src/usr.sbin/pcidump/pcidump.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 pcidump.c
> --- pcidump.c 17 Jan 2021 11:54:15 - 1.61
> +++ pcidump.c 5 Mar 2021 10:57:27 -
> @@ -1296,6 +1296,9 @@ static const struct pci_subclass pci_sub
> { PCI_SUBCLASS_DASP_MISC, "Miscellaneous" },
> };
>
> +static const struct pci_subclass pci_subclass_accelerator[] = {0};
> +static const struct pci_subclass pci_subclass_instrumentation[] = {0};
> +
> #define CLASS(_c, _n, _s) { \
> .class = _c, \
> .name = _n, \
> @@ -1338,6 +1341,10 @@ static const struct pci_class pci_classe
> pci_subclass_crypto),
> CLASS(PCI_CLASS_DASP, "DASP",
> pci_subclass_dasp),
> + CLASS(PCI_CLASS_ACCELERATOR,"Accelerator",
> + pci_subclass_accelerator),
> + CLASS(PCI_CLASS_INSTRUMENTATION, "Instrumentation",
> + pci_subclass_instrumentation),
> };
>
> static const struct pci_class *
>
>
Re: rpki-client validate URI function
On Fri, Mar 05, 2021 at 01:48:43PM +0100, Claudio Jeker wrote:
> Instead of adding similar checks all over the place introduce a
> valid_uri() function that checks if a URI is valid enough for rpki-client.
> rpki-client does not accept files or directories starting with ., bails on
> URI that have strange characters and valid_uri() will also check that the
> protocol is the expected one if provided.
>
> This diff converts the 4 places where URI are handled.
I'm ok with this. A few comments inline.
> --
> :wq Claudio
>
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 cert.c
> --- cert.c18 Feb 2021 16:23:17 - 1.27
> +++ cert.c5 Mar 2021 12:33:50 -
> @@ -19,7 +19,6 @@
>
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -140,34 +139,22 @@ sbgp_addr(struct parse *p,
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_sia_resource_notify(struct parse *p,
> - const unsigned char *d, size_t dsz)
> +sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
> {
> - size_t i;
> -
> if (p->res->notify != NULL) {
> warnx("%s: RFC 6487 section 4.8.8: SIA: "
> "Notify location already specified", p->fn);
> return 0;
> }
>
> + if ((p->res->notify = strndup(d, dsz)) == NULL)
This was slightly more robust before as p->res->notify would only get
assigned after all checks passed. That way it's guaranteed that an
invalid string is never used erroneously.
Perhaps it's not worth the extra gymnastics of strnduping to a local
variable and freeing it on error. After all, it will currently get
freed in cert_parse_inner() on validation failure...
> + err(1, NULL);
> +
> /* Make sure it's a https:// address. */
> - if (dsz <= 8 || strncasecmp(d, "https://";, 8)) {
> - warnx("%s: RFC 8182 section 3.2: not using https schema",
> - p->fn);
> + if (!valid_uri(p->res->notify, "https://";)) {
> + warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
> return 0;
> }
> - /* make sure only US-ASCII chars are in the URL */
> - for (i = 0; i < dsz; i++) {
> - if (isalnum(d[i]) || ispunct(d[i]))
> - continue;
> - warnx("%s: invalid URI", p->fn);
> - return 0;
> - }
> -
> -
> - if ((p->res->notify = strndup((const char *)d, dsz)) == NULL)
> - err(1, NULL);
>
> return 1;
> }
> @@ -177,40 +164,29 @@ sbgp_sia_resource_notify(struct parse *p
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_sia_resource_mft(struct parse *p,
> - const unsigned char *d, size_t dsz)
> +sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
> {
> - size_t i;
> -
> if (p->res->mft != NULL) {
> warnx("%s: RFC 6487 section 4.8.8: SIA: "
> "MFT location already specified", p->fn);
> return 0;
> }
>
> - /* Make sure it's an MFT rsync address. */
> - if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
> - warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
> - p->fn);
> - return 0;
> - }
> -
> if (strcasecmp(d + dsz - 4, ".mft") != 0) {
Since you dropped the above check that ensures that dsz > 8, I think this
needs to be
if (dsz < 4 || strcasecmp(...))
to avoid a potential out-of-bounds access
> warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "invalid rsync URI suffix", p->fn);
> - return 0;
> - }
> - /* make sure only US-ASCII chars are in the URL */
> - for (i = 0; i < dsz; i++) {
> - if (isalnum(d[i]) || ispunct(d[i]))
> - continue;
> - warnx("%s: invalid URI", p->fn);
> + "not an MFT file", p->fn);
> return 0;
> }
>
> - if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
> + if ((p->res->mft = strndup(d, dsz)) == NULL)
> err(1, NULL);
>
> + /* Make sure it's an MFT rsync address. */
> + if (!valid_uri(p->res->mft, "rsync://")) {
> + warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
> + return 0;
> + }
> +
> return 1;
> }
>
> @@ -219,35 +195,24 @@ sbgp_sia_resource_mft(struct parse *p,
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_sia_resource_carepo(struct parse *p,
> - const unsigned char *d, size_t dsz)
> +sbgp_sia_resource_carepo(struct parse *p, const char *d, size_t dsz)
> {
> - size_t i;
> -
> if (p->res->repo != NULL) {
> warnx("%s: RFC 6487 section 4.8.8: SIA: "
> "CA repository already specified", p
pcidump(8): add missing PCI classes
Hi,
this diff adds the missing PCI classes Accelerator and Instrumentation.
Thus, we can replace a few unknown in its output:
- 0x0008: Class: 13 (unknown), Subclass: 00 (unknown),
+ 0x0008: Class: 13 Instrumentation, Subclass: 00 (null),
Both Classes have vendor specific APIs. So, there are no predefined
subclasses.
OK?
bye,
Jan
Index: pcidump.c
===
RCS file: /cvs/src/usr.sbin/pcidump/pcidump.c,v
retrieving revision 1.61
diff -u -p -r1.61 pcidump.c
--- pcidump.c 17 Jan 2021 11:54:15 - 1.61
+++ pcidump.c 5 Mar 2021 10:57:27 -
@@ -1296,6 +1296,9 @@ static const struct pci_subclass pci_sub
{ PCI_SUBCLASS_DASP_MISC, "Miscellaneous" },
};
+static const struct pci_subclass pci_subclass_accelerator[] = {0};
+static const struct pci_subclass pci_subclass_instrumentation[] = {0};
+
#define CLASS(_c, _n, _s) { \
.class = _c, \
.name = _n, \
@@ -1338,6 +1341,10 @@ static const struct pci_class pci_classe
pci_subclass_crypto),
CLASS(PCI_CLASS_DASP, "DASP",
pci_subclass_dasp),
+ CLASS(PCI_CLASS_ACCELERATOR,"Accelerator",
+ pci_subclass_accelerator),
+ CLASS(PCI_CLASS_INSTRUMENTATION, "Instrumentation",
+ pci_subclass_instrumentation),
};
static const struct pci_class *
uhidev: allow devices to match specific multiple reports
uhidev allows a child device to claim all reports by calling *_match
functions with the report id set to UHIDEV_CLAIM_ALLREPORTID.
umt needs this because it has to access 3 reports which has worked
okay up until now because devices with umt and a ukbd have usually
presented them on separate uhidev devices. However, on a new
Surface Type Cover device, someone reported that these devices are
on the same uhidev meaning if umt attaches, the ukbd device can't.
To remedy this, probe devices with UHIDEV_CLAIM_MULTIPLE_REPORTID
instead and include an array in the uhidev_attach_arg the size of
the available reports. Devices wanting to claim multiple reports
just set the indexes in the array to 1 that it wants to claim and
uhidev will reserve those, but still probe other devices with each
specific unclaimed id.
umt is modified to do its report finding in its match function so it
can claim just the specific reports it needs.
Index: dev/usb/uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.89
diff -u -p -u -p -r1.89 uhidev.c
--- dev/usb/uhidev.c15 Feb 2021 11:26:00 - 1.89
+++ dev/usb/uhidev.c5 Mar 2021 14:51:09 -
@@ -250,21 +250,27 @@ uhidev_attach(struct device *parent, str
uha.uaa = uaa;
uha.parent = sc;
- uha.reportid = UHIDEV_CLAIM_ALLREPORTID;
+ uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
+ uha.nreports = nrepid;
+ uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
- /* Look for a driver claiming all report IDs first. */
+ /* Look for a driver claiming multiple report IDs first. */
dev = config_found_sm(self, &uha, NULL, uhidevsubmatch);
if (dev != NULL) {
for (repid = 0; repid < nrepid; repid++) {
/*
* Could already be assigned by uhidev_set_report_dev().
*/
- if (sc->sc_subdevs[repid] == NULL)
+ if (sc->sc_subdevs[repid] != NULL)
+ continue;
+
+ if (uha.claimed[repid])
sc->sc_subdevs[repid] = (struct uhidev *)dev;
}
- return;
}
+ free(uha.claimed, M_TEMP, nrepid);
+
for (repid = 0; repid < nrepid; repid++) {
DPRINTF(("%s: try repid=%d\n", __func__, repid));
if (hid_report_size(desc, size, hid_input, repid) == 0 &&
@@ -355,7 +361,7 @@ uhidevprint(void *aux, const char *pnp)
if (pnp)
printf("uhid at %s", pnp);
- if (uha->reportid != 0 && uha->reportid != UHIDEV_CLAIM_ALLREPORTID)
+ if (uha->reportid != 0 && uha->reportid !=
UHIDEV_CLAIM_MULTIPLE_REPORTID)
printf(" reportid %d", uha->reportid);
return (UNCONF);
}
Index: dev/usb/umt.c
===
RCS file: /cvs/src/sys/dev/usb/umt.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 umt.c
--- dev/usb/umt.c 23 Aug 2020 11:08:02 - 1.2
+++ dev/usb/umt.c 5 Mar 2021 14:51:09 -
@@ -61,8 +61,8 @@ const struct wsmouse_accessops umt_acces
};
intumt_match(struct device *, void *, void *);
-intumt_find_winptp_reports(struct uhidev_softc *, void *, int,
- struct umt_softc *);
+intumt_find_winptp_reports(struct uhidev_softc *, void *, int, int *,
+ int *, int *);
void umt_attach(struct device *, struct device *, void *);
intumt_hidev_get_report(struct device *, int, int, void *, int);
intumt_hidev_set_report(struct device *, int, int, void *, int);
@@ -83,13 +83,19 @@ int
umt_match(struct device *parent, void *match, void *aux)
{
struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
+ int input = 0, conf = 0, cap = 0;
int size;
void *desc;
- if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID) {
+ if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID) {
uhidev_get_report_desc(uha->parent, &desc, &size);
- if (umt_find_winptp_reports(uha->parent, desc, size, NULL))
+ if (umt_find_winptp_reports(uha->parent, desc, size, &input,
+ &conf, &cap)) {
+ uha->claimed[input] = 1;
+ uha->claimed[conf] = 1;
+ uha->claimed[cap] = 1;
return (UMATCH_DEVCLASS_DEVSUBCLASS);
+ }
}
return (UMATCH_NONE);
@@ -97,16 +103,17 @@ umt_match(struct device *parent, void *m
int
umt_find_winptp_reports(struct uhidev_softc *parent, void *desc, int size,
-struct umt_softc *sc)
+int *input, int *config, int *cap)
{
int repid;
- int input = 0, conf = 0, cap = 0;
+ int finput = 0, fconf = 0, fcap = 0;
- if (sc != NULL) {
- sc->sc_rep_inpu
rpki-client validate filehash function
RRDP also uses SHA256 hashes to validate files (before withdraws and
updates). Again move this from the implementation in mft.c to validate.c
this way it can be reused.
OK?
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.51
diff -u -p -r1.51 extern.h
--- extern.h5 Mar 2021 12:33:19 - 1.51
+++ extern.h5 Mar 2021 14:00:29 -
@@ -354,6 +354,7 @@ int valid_ta(const char *, struct auth
int valid_cert(const char *, struct auth_tree *,
const struct cert *);
int valid_roa(const char *, struct auth_tree *, struct roa *);
+int valid_filehash(const char *, const char *, size_t);
/* Working with CMS files. */
Index: mft.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
retrieving revision 1.28
diff -u -p -r1.28 mft.c
--- mft.c 4 Mar 2021 14:24:17 - 1.28
+++ mft.c 5 Mar 2021 12:33:50 -
@@ -434,61 +434,32 @@ out:
}
/*
- * Check the hash value of a file.
+ * Check all files and their hashes in a MFT structure.
* Return zero on failure, non-zero on success.
*/
-static int
-mft_validfilehash(const char *fn, const struct mftfile *m)
+int
+mft_check(const char *fn, struct mft *p)
{
- charfilehash[SHA256_DIGEST_LENGTH];
- charbuffer[8192];
+ size_t i;
+ int rc = 1;
char*cp, *path = NULL;
- SHA256_CTX ctx;
- ssize_t nr;
- int fd;
/* Check hash of file now, but first build path for it */
cp = strrchr(fn, '/');
assert(cp != NULL);
assert(cp - fn < INT_MAX);
- if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn, m->file) == -1)
- err(1, NULL);
- if ((fd = open(path, O_RDONLY)) == -1) {
- warn("%s: referenced file %s", fn, m->file);
+ for (i = 0; i < p->filesz; i++) {
+ const struct mftfile *m = &p->files[i];
+ if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn,
+ m->file) == -1)
+ err(1, NULL);
+ if (!valid_filehash(path, m->hash, sizeof(m->hash))) {
+ warnx("%s: bad message digest for %s", fn, m->file);
+ rc = 0;
+ }
free(path);
- return 0;
}
- free(path);
-
- SHA256_Init(&ctx);
- while ((nr = read(fd, buffer, sizeof(buffer))) > 0) {
- SHA256_Update(&ctx, buffer, nr);
- }
- close(fd);
-
- SHA256_Final(filehash, &ctx);
- if (memcmp(m->hash, filehash, SHA256_DIGEST_LENGTH) != 0) {
- warnx("%s: bad message digest for %s", fn, m->file);
- return 0;
- }
-
- return 1;
-}
-
-/*
- * Check all files and their hashes in a MFT structure.
- * Return zero on failure, non-zero on success.
- */
-int
-mft_check(const char *fn, struct mft *p)
-{
- size_t i;
- int rc = 1;
-
- for (i = 0; i < p->filesz; i++)
- if (!mft_validfilehash(fn, &p->files[i]))
- rc = 0;
return rc;
}
Index: validate.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.11
diff -u -p -r1.11 validate.c
--- validate.c 12 Sep 2020 15:46:48 - 1.11
+++ validate.c 5 Mar 2021 13:59:46 -
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -237,6 +238,37 @@ valid_roa(const char *fn, struct auth_tr
tracewarn(a);
return 0;
}
+
+ return 1;
+}
+
+/*
+ * Validate a file by verifying the SHA256 hash of that file.
+ * Returns 1 if valid, 0 otherwise.
+ */
+int
+valid_filehash(const char *fn, const char *hash, size_t hlen)
+{
+ SHA256_CTX ctx;
+ charfilehash[SHA256_DIGEST_LENGTH];
+ charbuffer[8192];
+ ssize_t nr;
+ int fd;
+
+ if (hlen != sizeof(filehash))
+ errx(1, "bad hash size");
+
+ if ((fd = open(fn, O_RDONLY)) == -1)
+ return 0;
+
+ SHA256_Init(&ctx);
+ while ((nr = read(fd, buffer, sizeof(buffer))) > 0)
+ SHA256_Update(&ctx, buffer, nr);
+ close(fd);
+
+ SHA256_Final(filehash, &ctx);
+ if (memcmp(hash, filehash, sizeof(filehash)) != 0)
+ return 0;
return 1;
}
rpki-client validate URI function
Instead of adding similar checks all over the place introduce a
valid_uri() function that checks if a URI is valid enough for rpki-client.
rpki-client does not accept files or directories starting with ., bails on
URI that have strange characters and valid_uri() will also check that the
protocol is the expected one if provided.
This diff converts the 4 places where URI are handled.
--
:wq Claudio
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.27
diff -u -p -r1.27 cert.c
--- cert.c 18 Feb 2021 16:23:17 - 1.27
+++ cert.c 5 Mar 2021 12:33:50 -
@@ -19,7 +19,6 @@
#include
#include
-#include
#include
#include
#include
@@ -140,34 +139,22 @@ sbgp_addr(struct parse *p,
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_sia_resource_notify(struct parse *p,
- const unsigned char *d, size_t dsz)
+sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
{
- size_t i;
-
if (p->res->notify != NULL) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
"Notify location already specified", p->fn);
return 0;
}
+ if ((p->res->notify = strndup(d, dsz)) == NULL)
+ err(1, NULL);
+
/* Make sure it's a https:// address. */
- if (dsz <= 8 || strncasecmp(d, "https://";, 8)) {
- warnx("%s: RFC 8182 section 3.2: not using https schema",
- p->fn);
+ if (!valid_uri(p->res->notify, "https://";)) {
+ warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
return 0;
}
- /* make sure only US-ASCII chars are in the URL */
- for (i = 0; i < dsz; i++) {
- if (isalnum(d[i]) || ispunct(d[i]))
- continue;
- warnx("%s: invalid URI", p->fn);
- return 0;
- }
-
-
- if ((p->res->notify = strndup((const char *)d, dsz)) == NULL)
- err(1, NULL);
return 1;
}
@@ -177,40 +164,29 @@ sbgp_sia_resource_notify(struct parse *p
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_sia_resource_mft(struct parse *p,
- const unsigned char *d, size_t dsz)
+sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
{
- size_t i;
-
if (p->res->mft != NULL) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
"MFT location already specified", p->fn);
return 0;
}
- /* Make sure it's an MFT rsync address. */
- if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
- warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
- p->fn);
- return 0;
- }
-
if (strcasecmp(d + dsz - 4, ".mft") != 0) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
- "invalid rsync URI suffix", p->fn);
- return 0;
- }
- /* make sure only US-ASCII chars are in the URL */
- for (i = 0; i < dsz; i++) {
- if (isalnum(d[i]) || ispunct(d[i]))
- continue;
- warnx("%s: invalid URI", p->fn);
+ "not an MFT file", p->fn);
return 0;
}
- if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
+ if ((p->res->mft = strndup(d, dsz)) == NULL)
err(1, NULL);
+ /* Make sure it's an MFT rsync address. */
+ if (!valid_uri(p->res->mft, "rsync://")) {
+ warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
+ return 0;
+ }
+
return 1;
}
@@ -219,35 +195,24 @@ sbgp_sia_resource_mft(struct parse *p,
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_sia_resource_carepo(struct parse *p,
- const unsigned char *d, size_t dsz)
+sbgp_sia_resource_carepo(struct parse *p, const char *d, size_t dsz)
{
- size_t i;
-
if (p->res->repo != NULL) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
"CA repository already specified", p->fn);
return 0;
}
+ if ((p->res->repo = strndup(d, dsz)) == NULL)
+ err(1, NULL);
+
/* Make sure it's an rsync:// address. */
- if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
- warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
+ if (!valid_uri(p->res->repo, "rsync://")) {
+ warnx("%s: RFC 6487 section 4.8.8: bad CA repository URI",
p->fn);
return 0;
}
- /* make sure only US-ASCII chars are in the URL */
- for (i = 0; i < dsz; i++) {
- if (isalnum(d[i]) || ispunct(d[i]))
- continue;
- warnx("%s: invalid URI", p->fn);
- return 0;
-
Re: pcidump(8): add missing PCI classes
ok.
> On 5 Mar 2021, at 9:05 pm, Jan Klemkow wrote:
>
> Hi,
>
> this diff adds the missing PCI classes Accelerator and Instrumentation.
> Thus, we can replace a few unknown in its output:
>
> - 0x0008: Class: 13 (unknown), Subclass: 00 (unknown),
> + 0x0008: Class: 13 Instrumentation, Subclass: 00 (null),
>
> Both Classes have vendor specific APIs. So, there are no predefined
> subclasses.
>
> OK?
>
> bye,
> Jan
>
> Index: pcidump.c
> ===
> RCS file: /cvs/src/usr.sbin/pcidump/pcidump.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 pcidump.c
> --- pcidump.c 17 Jan 2021 11:54:15 - 1.61
> +++ pcidump.c 5 Mar 2021 10:57:27 -
> @@ -1296,6 +1296,9 @@ static const struct pci_subclass pci_sub
> { PCI_SUBCLASS_DASP_MISC, "Miscellaneous" },
> };
>
> +static const struct pci_subclass pci_subclass_accelerator[] = {0};
> +static const struct pci_subclass pci_subclass_instrumentation[] = {0};
> +
> #define CLASS(_c, _n, _s) { \
> .class = _c, \
> .name = _n, \
> @@ -1338,6 +1341,10 @@ static const struct pci_class pci_classe
> pci_subclass_crypto),
> CLASS(PCI_CLASS_DASP, "DASP",
> pci_subclass_dasp),
> + CLASS(PCI_CLASS_ACCELERATOR,"Accelerator",
> + pci_subclass_accelerator),
> + CLASS(PCI_CLASS_INSTRUMENTATION, "Instrumentation",
> + pci_subclass_instrumentation),
> };
>
> static const struct pci_class *
>
Re: use 64bit ethernet addresses in carp(4)
On 5.3.2021. 6:11, David Gwynne wrote: > this passes the destination ethernet address from the network packet > as a uint64_t from ether_input into carp_input, so it can use it > to see if a carp interface should take the packet. > > it's been working on amd64 and sparc64. anyone else want to try? Hi, i'm hitting this diff on routers and doing ifconfig carp1 destroy && ifconfig carp0 destroy && sleep 3 && sh /etc/netstart && sleep 3 on both routers while sending traffic over carp interfaces .. both routers seems stable ..
Re: Kill SINGLE_PTRACE
On 04/03/21(Thu) 12:25, Claudio Jeker wrote: > On Thu, Mar 04, 2021 at 11:06:21AM +0100, Martin Pieuchot wrote: > > [...] > > The comment documents what sibling threads are supposed to do once the > > current one has called single_thread_set() with a given SINGLE_* option. > > > > Sibling threads will continue to execute until the next parking point > > where single_thread_check() are. Parking points are divided in two > > categories. In the "deep" ones unwinding is preferred for UNWIND and > > EXIT, in the others only context switching occurs. > > > > Every single_thread_set() call is in itself a parking point to prevent > > races. The only "deep" parking point is the one in sys_execve() for > > obvious reasons. > > Actually this is where I got confused. This is the place where "deep" is > the wrong word. The fact that SINGLE_UNWIND will abort the > single_thread_set() if another thread is in the process to run single > threaded should probably be added as a comment here. In the end this is > here to prevent a race between two threads calling execve() at the same > time. I'd appreciate if you could add the comment yourself if you're ok with the diff as is. I'm not sure to understand what you're suggesting, so it seems simpler to me if you can pick the words yourself.
Re: Read `ps_single' once
On 04/03/21(Thu) 11:45, Mark Kettenis wrote:
> > Date: Thu, 4 Mar 2021 11:19:23 +0100
> > From: Martin Pieuchot
> >
> > On 04/03/21(Thu) 11:01, Mark Kettenis wrote:
> > > > Date: Thu, 4 Mar 2021 10:54:48 +0100
> > > > From: Patrick Wildt
> > > >
> > > > Am Thu, Mar 04, 2021 at 10:42:24AM +0100 schrieb Mark Kettenis:
> > > > > > Date: Thu, 4 Mar 2021 10:34:24 +0100
> > > > > > From: Martin Pieuchot
> > > > > >
> > > > > > Running t/rw/msleep(9) w/o KERNEL_LOCK() implies that a thread can
> > > > > > change the value of `ps_single' while one of its siblings might be
> > > > > > dereferencing it.
> > > > > >
> > > > > > To prevent inconsistencies in the code executed by sibling thread,
> > > > > > the
> > > > > > diff below makes sure `ps_single' is dereferenced only once in
> > > > > > various
> > > > > > parts of the kernel.
> > > > > >
> > > > > > ok?
> > > > >
> > > > > I think that means that ps_single has to be declared "volatile".
> > > >
> > > > Isn't there the READ_ONCE(x) macro, that does exactly that?
> > >
> > > Not a big fan of READ_ONCE() and WRITE_ONCE(), but apparently those
> > > are needed to comply with the alpha memory model. At least in some
> > > cases...
> >
> > Updated diff using READ_ONCE(), ok?
>
> If you use READ_ONCE() you shoul also use WRITE_ONCE() everywhere
> where you modify ps_single isn't it?
I don't know, I'm learning how to do it. I'd appreciate if somebody could
come with a READ_ONCE(9) manual explaining how this API should be used.
Updated diff including the WRITE_ONCE().
Index: kern/kern_exit.c
===
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.196
diff -u -p -r1.196 kern_exit.c
--- kern/kern_exit.c15 Feb 2021 09:35:59 - 1.196
+++ kern/kern_exit.c5 Mar 2021 10:28:05 -
@@ -274,6 +274,8 @@ exit1(struct proc *p, int xexit, int xsi
*/
if (qr->ps_flags & PS_TRACED &&
!(qr->ps_flags & PS_EXITING)) {
+ struct proc *st;
+
process_untrace(qr);
/*
@@ -281,9 +283,9 @@ exit1(struct proc *p, int xexit, int xsi
* direct the signal to the active
* thread to avoid deadlock.
*/
- if (qr->ps_single)
- ptsignal(qr->ps_single, SIGKILL,
- STHREAD);
+ st = READ_ONCE(qr->ps_single);
+ if (st != NULL)
+ ptsignal(st, SIGKILL, STHREAD);
else
prsignal(qr, SIGKILL);
} else {
@@ -510,7 +512,7 @@ dowait4(struct proc *q, pid_t pid, int *
{
int nfound;
struct process *pr;
- struct proc *p;
+ struct proc *p, *st;
int error;
if (pid == 0)
@@ -541,10 +543,11 @@ loop:
proc_finish_wait(q, p);
return (0);
}
+
+ st = READ_ONCE(pr->ps_single);
if (pr->ps_flags & PS_TRACED &&
- (pr->ps_flags & PS_WAITED) == 0 && pr->ps_single &&
- pr->ps_single->p_stat == SSTOP &&
- (pr->ps_single->p_flag & P_SUSPSINGLE) == 0) {
+ (pr->ps_flags & PS_WAITED) == 0 && st != NULL &&
+ st->p_stat == SSTOP && (st->p_flag & P_SUSPSINGLE) == 0) {
if (single_thread_wait(pr, 0))
goto loop;
Index: kern/kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.274
diff -u -p -r1.274 kern_sig.c
--- kern/kern_sig.c 4 Mar 2021 09:02:37 - 1.274
+++ kern/kern_sig.c 5 Mar 2021 10:28:05 -
@@ -2040,7 +2040,7 @@ single_thread_set(struct proc *p, enum s
}
pr->ps_singlecount = 0;
membar_producer();
- pr->ps_single = p;
+ WRITE_ONCE(pr->ps_single, p);
TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) {
if (q == p)
continue;
@@ -2131,7 +2131,7 @@ single_thread_clear(struct proc *p, int
KERNEL_ASSERT_LOCKED();
SCHED_LOCK(s);
- pr->ps_single = NULL;
+ WRITE_ONCE(pr->ps_single, NULL);
atomic_clearbits_int(&pr->ps_flags, PS_SINGLEUNWIND | PS_SINGLEEXIT);
TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) {
if (q == p || (q->p_flag & P_SUSPSINGLE) == 0)
Index: kern/sys_process.c
===
RCS file: /cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.86
diff -u -p -r1.86 sys_process.c
--
