Re: if calloc() needs nmemb and size, why doesn't freezero()?
On Thu, Feb 18, 2021 at 03:24:36PM -0600, Luke Small wrote: > However, calloc(ptr, nmemb, size) may have been called using smaller int > variable types which would overflow when multiplied. Where if the variables > storing the values passed to nmemb and size are less than or especially > equal to their original values, I think it’d be good to state that: > > freezero(ptr, (size_t)nmemb * (size_t)size); > is guaranteed to work, but > freezero(ptr, nmemb * size); > does not have that guarantee. Lets try to make things explicit. The function c() does the overflowe check like calloc does. The function f() takes a size_t. #include #include #define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 4)) void c(size_t nmemb, size_t size) { if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) && nmemb > 0 && SIZE_T_MAX / nmemb < size) printf("Overflow\n"); else printf("%zu\n", nmemb * size); } void f(size_t m) { printf("%zu\n", m); } int main() { int a = INT_MAX; int b = INT_MAX; c(a, b); f(a * b); } Now the issues is that the multiplication in the last line of main() overflows: $ ./a.out 4611686014132420609 1 because this is an int multiplication only after that the promotion to size_t is done. So you are right that this can happen, *if you are using the wrong types*. But I would argue that feeding anything other than either size_t or constants to calloc() is already wrong. You *have* to consider the argument conversion rules when feeding values to calloc() (or any function). To avoid having to think about those, start with size_t already for everything that is a size or count of a memory object. -Otto
Re: timecounting: use C99-style initialization for all timecounter structs
Scott Cheloha writes: > Hi, > > If the timecounter struct changes again in the future it will be > easier to make the change if we are using C99-style initialization > everywhere. In general I think C99-style initialization is easier to > read for larger structs. The timecounter struct definitely qualifies > as "larger". We probably should already be doing this but nobody has > bothered yet. > > So I will bother. This patch changes every timecounter struct to use > C99-style initialization. Some are already using it but most are not. > > Yes, I am aware that this is tedious to review. I'm sorry. I think > suffering this now will pay off in the future. Nah, not bad at all. Could you check if you get identical object files by chance? It would be a nice double check. > > Speaking of the future: in a subsequent patch I would like to remove > several of the the zero and NULL members, as C99 guarantees that > omission of a member at initialization causes it to be implicitly > zeroed. For instance, there is no reason to set .tc_user if the > timecounter has no corresponding driver in libc. There are also no > drivers setting the .tc_poll_pps function pointer, so we can just let > it implicitly be NULL. And if the timecounter needs no private cookie > we don't need to explicitly set .tc_priv to NULL. Et cetera. > > I suppose if people prefer it we _could_ do such changes in this > patch. I'm leaning toward not doing that. Switching to the C99 style > *and* dropping members will make review more difficult and increase > the likelihood of a mistake, i.e. I will accidentally break the build > on some platform and people will yell at me, which I want to avoid. > > Thoughts? Preferences? ok? If others don't mind on some grounds, I'm fairly convinced of this being a functional no-op and so: OK gnezdo@ > > Index: ./arch/alpha/alpha/clock.c > === > RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v > retrieving revision 1.24 > diff -u -p -r1.24 clock.c > --- ./arch/alpha/alpha/clock.c6 Jul 2020 13:33:06 - 1.24 > +++ ./arch/alpha/alpha/clock.c19 Feb 2021 02:57:55 - > @@ -64,7 +64,14 @@ int clk_irq = 0; > > u_int rpcc_get_timecount(struct timecounter *); > struct timecounter rpcc_timecounter = { > - rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, 0 > + .tc_get_timecount = rpcc_get_timecount, > + .tc_poll_pps = NULL, > + .tc_counter_mask = ~0u, > + .tc_frequency = 0, > + .tc_name = "rpcc", > + .tc_quality = 0, > + .tc_priv = NULL, > + .tc_user = 0, > }; > > extern todr_chip_handle_t todr_handle; > Index: ./arch/amd64/amd64/tsc.c > === > RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v > retrieving revision 1.22 > diff -u -p -r1.22 tsc.c > --- ./arch/amd64/amd64/tsc.c 24 Dec 2020 04:20:48 - 1.22 > +++ ./arch/amd64/amd64/tsc.c 19 Feb 2021 02:57:55 - > @@ -52,7 +52,14 @@ extern u_int32_t lapic_per_second; > #endif > > struct timecounter tsc_timecounter = { > - tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL, TC_TSC > + .tc_get_timecount = tsc_get_timecount, > + .tc_poll_pps = NULL, > + .tc_counter_mask = ~0u, > + .tc_frequency = 0, > + .tc_name = "tsc", > + .tc_quality = -1000, > + .tc_priv = NULL, > + .tc_user = TC_TSC, > }; > > uint64_t > Index: ./arch/amd64/isa/clock.c > === > RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v > retrieving revision 1.34 > diff -u -p -r1.34 clock.c > --- ./arch/amd64/isa/clock.c 6 Jul 2020 13:33:06 - 1.34 > +++ ./arch/amd64/isa/clock.c 19 Feb 2021 02:57:55 - > @@ -116,7 +116,14 @@ u_int i8254_get_timecount(struct timecou > u_int i8254_simple_get_timecount(struct timecounter *tc); > > static struct timecounter i8254_timecounter = { > - i8254_get_timecount, NULL, ~0u, TIMER_FREQ, "i8254", 0, NULL, 0 > + .tc_get_timecount = i8254_get_timecount, > + .tc_poll_pps = NULL, > + .tc_counter_mask = ~0u, > + .tc_frequency = TIMER_FREQ, > + .tc_name = "i8254", > + .tc_quality = 0, > + .tc_priv = NULL, > + .tc_user = 0, > }; > > int clockintr(void *); > Index: ./arch/armv7/omap/dmtimer.c > === > RCS file: /cvs/src/sys/arch/armv7/omap/dmtimer.c,v > retrieving revision 1.9 > diff -u -p -r1.9 dmtimer.c > --- ./arch/armv7/omap/dmtimer.c 19 Jan 2021 18:04:43 - 1.9 > +++ ./arch/armv7/omap/dmtimer.c 19 Feb 2021 02:57:55 - > @@ -111,7 +111,13 @@ void dmtimer_setstatclockrate(int newhz) > u_int dmtimer_get_timecount(struct timecounter *); > > static struct timecounter dmtimer_timecounter = { > - dmtimer_get_timecount, NULL, 0x, 0, "dmtimer", 0, NULL > + .tc_get_timecount = dmtimer_get_timecount, > +
Re: if calloc() needs nmemb and size, why doesn't freezero()?
I had a drawn out email email describing passing by value and the function’s need to only perform size_t multiplication overload checking but not only do you not care I don’t think it’s worth my time to merely succeed in angering you. I love your work! On Thu, Feb 18, 2021 at 7:10 PM Theo de Raadt wrote: > Luke Small wrote: > > > However, calloc(ptr, nmemb, size) may have been called using smaller int > > variable types which would overflow when multiplied. > > In which case the allocation would not have succeeded. > > Where if the variables > > storing the values passed to nmemb and size are less than or especially > > equal to their original values, I think it’d be good to state that: > > Huh? > > > freezero(ptr, (size_t)nmemb * (size_t)size); > > is guaranteed to work, but > > freezero(ptr, nmemb * size); > > does not have that guarantee. > > I hope I never run any software by you. > -- -Luke
timecounting: use C99-style initialization for all timecounter structs
Hi, If the timecounter struct changes again in the future it will be easier to make the change if we are using C99-style initialization everywhere. In general I think C99-style initialization is easier to read for larger structs. The timecounter struct definitely qualifies as "larger". We probably should already be doing this but nobody has bothered yet. So I will bother. This patch changes every timecounter struct to use C99-style initialization. Some are already using it but most are not. Yes, I am aware that this is tedious to review. I'm sorry. I think suffering this now will pay off in the future. Speaking of the future: in a subsequent patch I would like to remove several of the the zero and NULL members, as C99 guarantees that omission of a member at initialization causes it to be implicitly zeroed. For instance, there is no reason to set .tc_user if the timecounter has no corresponding driver in libc. There are also no drivers setting the .tc_poll_pps function pointer, so we can just let it implicitly be NULL. And if the timecounter needs no private cookie we don't need to explicitly set .tc_priv to NULL. Et cetera. I suppose if people prefer it we _could_ do such changes in this patch. I'm leaning toward not doing that. Switching to the C99 style *and* dropping members will make review more difficult and increase the likelihood of a mistake, i.e. I will accidentally break the build on some platform and people will yell at me, which I want to avoid. Thoughts? Preferences? ok? Index: ./arch/alpha/alpha/clock.c === RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v retrieving revision 1.24 diff -u -p -r1.24 clock.c --- ./arch/alpha/alpha/clock.c 6 Jul 2020 13:33:06 - 1.24 +++ ./arch/alpha/alpha/clock.c 19 Feb 2021 02:57:55 - @@ -64,7 +64,14 @@ int clk_irq = 0; u_int rpcc_get_timecount(struct timecounter *); struct timecounter rpcc_timecounter = { - rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, 0 + .tc_get_timecount = rpcc_get_timecount, + .tc_poll_pps = NULL, + .tc_counter_mask = ~0u, + .tc_frequency = 0, + .tc_name = "rpcc", + .tc_quality = 0, + .tc_priv = NULL, + .tc_user = 0, }; extern todr_chip_handle_t todr_handle; Index: ./arch/amd64/amd64/tsc.c === RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v retrieving revision 1.22 diff -u -p -r1.22 tsc.c --- ./arch/amd64/amd64/tsc.c24 Dec 2020 04:20:48 - 1.22 +++ ./arch/amd64/amd64/tsc.c19 Feb 2021 02:57:55 - @@ -52,7 +52,14 @@ extern u_int32_t lapic_per_second; #endif struct timecounter tsc_timecounter = { - tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL, TC_TSC + .tc_get_timecount = tsc_get_timecount, + .tc_poll_pps = NULL, + .tc_counter_mask = ~0u, + .tc_frequency = 0, + .tc_name = "tsc", + .tc_quality = -1000, + .tc_priv = NULL, + .tc_user = TC_TSC, }; uint64_t Index: ./arch/amd64/isa/clock.c === RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v retrieving revision 1.34 diff -u -p -r1.34 clock.c --- ./arch/amd64/isa/clock.c6 Jul 2020 13:33:06 - 1.34 +++ ./arch/amd64/isa/clock.c19 Feb 2021 02:57:55 - @@ -116,7 +116,14 @@ u_int i8254_get_timecount(struct timecou u_int i8254_simple_get_timecount(struct timecounter *tc); static struct timecounter i8254_timecounter = { - i8254_get_timecount, NULL, ~0u, TIMER_FREQ, "i8254", 0, NULL, 0 + .tc_get_timecount = i8254_get_timecount, + .tc_poll_pps = NULL, + .tc_counter_mask = ~0u, + .tc_frequency = TIMER_FREQ, + .tc_name = "i8254", + .tc_quality = 0, + .tc_priv = NULL, + .tc_user = 0, }; intclockintr(void *); Index: ./arch/armv7/omap/dmtimer.c === RCS file: /cvs/src/sys/arch/armv7/omap/dmtimer.c,v retrieving revision 1.9 diff -u -p -r1.9 dmtimer.c --- ./arch/armv7/omap/dmtimer.c 19 Jan 2021 18:04:43 - 1.9 +++ ./arch/armv7/omap/dmtimer.c 19 Feb 2021 02:57:55 - @@ -111,7 +111,13 @@ void dmtimer_setstatclockrate(int newhz) u_int dmtimer_get_timecount(struct timecounter *); static struct timecounter dmtimer_timecounter = { - dmtimer_get_timecount, NULL, 0x, 0, "dmtimer", 0, NULL + .tc_get_timecount = dmtimer_get_timecount, + .tc_poll_pps = NULL, + .tc_counter_mask = 0x, + .tc_frequency = 0, + .tc_name = "dmtimer", + .tc_quality = 0, + .tc_priv = NULL, }; bus_space_handle_t dmtimer_ioh0; Index: ./arch/armv7/omap/gptimer.c === RCS file: /cvs/src/sys/arch/armv7/omap/gptimer.c,v retrieving revision 1.11 diff -u -p -r1.11 gptimer.c ---
Re: malloc junking tweaks
This diff will land in snapshots.
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Luke Small wrote: > However, calloc(ptr, nmemb, size) may have been called using smaller int > variable types which would overflow when multiplied. In which case the allocation would not have succeeded. > Where if the variables > storing the values passed to nmemb and size are less than or especially > equal to their original values, I think it’d be good to state that: Huh? > freezero(ptr, (size_t)nmemb * (size_t)size); > is guaranteed to work, but > freezero(ptr, nmemb * size); > does not have that guarantee. I hope I never run any software by you.
Re: UNIX domain sockets: move garbage collector to `systqmp'
ping > On 16 Feb 2021, at 16:13, Vitaliy Makkoveev wrote: > > There are no fallout reports after UNIX sockets unlocking, so I propose > to move moves garbage collector to `systqmp'. unp_gc() touches nothing > which requires kernel lock to be held. > > Index: sys/kern/uipc_usrreq.c > === > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v > retrieving revision 1.143 > diff -u -p -r1.143 uipc_usrreq.c > --- sys/kern/uipc_usrreq.c10 Feb 2021 08:20:09 - 1.143 > +++ sys/kern/uipc_usrreq.c16 Feb 2021 09:44:26 - > @@ -444,7 +444,7 @@ unp_detach(struct unpcb *unp) > m_freem(unp->unp_addr); > pool_put(_pool, unp); > if (unp_rights) > - task_add(systq, _gc_task); > + task_add(systqmp, _gc_task); > > if (vp != NULL) { > /* > @@ -1197,7 +1197,7 @@ unp_discard(struct fdpass *rp, int nfds) > memset(rp, 0, sizeof(*rp) * nfds); > SLIST_INSERT_HEAD(_deferred, defer, ud_link); > > - task_add(systq, _gc_task); > + task_add(systqmp, _gc_task); > } > > int >
Re: if calloc() needs nmemb and size, why doesn't freezero()?
However, calloc(ptr, nmemb, size) may have been called using smaller int variable types which would overflow when multiplied. Where if the variables storing the values passed to nmemb and size are less than or especially equal to their original values, I think it’d be good to state that: freezero(ptr, (size_t)nmemb * (size_t)size); is guaranteed to work, but freezero(ptr, nmemb * size); does not have that guarantee. On Thu, Feb 18, 2021 at 3:42 AM Otto Moerbeek wrote: > On Wed, Feb 17, 2021 at 11:05:49AM -0700, Theo de Raadt wrote: > > > Luke Small wrote: > > > > > I guess I always thought there'd be some more substantial overflow > mitigation. > > > > You have to free with the exact same size as allocation. > > Small correction: the size may be smaller than the original. In that > case, only a partial clear is guaranteed, the deallocation will still > be for the full allocation. Originally we were more strict, but iirc > that was causing to much headaches for some. See > https://cvsweb.openbsd.org/src/lib/libc/stdlib/malloc.c?rev=1.221 > > But the point stands: nmemb * size does not overflow, since the > original allocation would have overflowed and thus failed. > > -Otto > > > > > nmemb and size did not change. > > > > The math has already been checked, and regular codeflows will store the > > multiple in a single variable after successful checking, for > > reuse. > > > > > Would it be too much hand-holding to put in the manpage that to avoid > potential > > > freeezero() integer overflow, > > > it may be useful to run freezero() as freezero((size_t)nmemb * > (size_t)size); > > > > Wow, Those casts make it very clear you don't understand C, if you do > > that kind of stuff elsewhere you are introducing problems. > > > > Sorry no you are wrong. > > > -- -Luke
Re: malloc junking tweaks
On 2021/02/18 19:52, Otto Moerbeek wrote: > Any feedback? I've been doing i386 ports builds with this in, no problems seen there. The changes seem sane, and there's an easy way for people to disable the changes if they do run into problems. Does it make sense to put it into snaps?
Re: malloc junking tweaks
On Fri, Feb 12, 2021 at 02:48:34PM +0100, Otto Moerbeek wrote: > On Fri, Feb 12, 2021 at 02:18:08PM +0100, Otto Moerbeek wrote: > > > Hi, > > > > Curently, junking is done like this: > > > > - for small chunk, the whole chunk junked on free > > > > - for pages sized and larger, the first half a page is filled > > > > - after a delayed free, the first 32 bytes of small chunks are > > validated to not be overwritten > > > > - page sized and larger allocations are not validated at all, even if > > they end up in the cache. > > > > This diff changes the following: > > > > - I make use of the fact that I know how the chunks are aligned, and > > write 8 bytes at the time by using a uint64_t pointer. For an > > allocation a max of 4 such uint64_t's are written spread over the > > allocation. For pages sized and larger, the first page is junked in > > such a way. > > > > - Delayed free of a small chunk checks the corresponiding way. > > > > - Pages ending up in the cache are validated upon unmapping or re-use. > > > > The last point is the real gain: we also check for write-after-free > > for large allocations, which we did not do before. > > > > So we are catching more writes-after-frees. A price to pay is that > > larger chunks are not completely junked, but only a total of 32 bytes > > are. I chose this number after comparing performance with the current > > code: we still gain a bit in speed. > > > > Junk mode 0 (j) and junk mode 2 (J) are not changed. > > > > Please test and review, > > > > -Otto > > > > And now with correct version of diff Any feedback? -Otto > > > Index: stdlib/malloc.3 > === > RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v > retrieving revision 1.126 > diff -u -p -r1.126 malloc.3 > --- stdlib/malloc.3 14 Sep 2019 13:16:50 - 1.126 > +++ stdlib/malloc.3 12 Feb 2021 08:14:54 - > @@ -619,7 +619,7 @@ or > reallocate an unallocated pointer was made. > .It Dq chunk is already free > There was an attempt to free a chunk that had already been freed. > -.It Dq use after free > +.It Dq write after free > A chunk has been modified after it was freed. > .It Dq modified chunk-pointer > The pointer passed to > Index: stdlib/malloc.c > === > RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v > retrieving revision 1.267 > diff -u -p -r1.267 malloc.c > --- stdlib/malloc.c 23 Nov 2020 15:42:11 - 1.267 > +++ stdlib/malloc.c 12 Feb 2021 08:14:54 - > @@ -89,6 +89,7 @@ > */ > #define SOME_JUNK0xdb/* deadbeef */ > #define SOME_FREEJUNK0xdf/* dead, free */ > +#define SOME_FREEJUNK_ULL0xdfdfdfdfdfdfdfdfULL > > #define MMAP(sz,f) mmap(NULL, (sz), PROT_READ | PROT_WRITE, \ > MAP_ANON | MAP_PRIVATE | (f), -1, 0) > @@ -655,6 +656,49 @@ delete(struct dir_info *d, struct region > } > } > > +static inline void > +junk_free(int junk, void *p, size_t sz) > +{ > + size_t i, step = 1; > + uint64_t *lp = p; > + > + if (junk == 0 || sz == 0) > + return; > + sz /= sizeof(uint64_t); > + if (junk == 1) { > + if (sz > MALLOC_PAGESIZE / sizeof(uint64_t)) > + sz = MALLOC_PAGESIZE / sizeof(uint64_t); > + step = sz / 4; > + if (step == 0) > + step = 1; > + } > + for (i = 0; i < sz; i += step) > + lp[i] = SOME_FREEJUNK_ULL; > +} > + > +static inline void > +validate_junk(struct dir_info *pool, void *p, size_t sz) > +{ > + size_t i, step = 1; > + uint64_t *lp = p; > + > + if (pool->malloc_junk == 0 || sz == 0) > + return; > + sz /= sizeof(uint64_t); > + if (pool->malloc_junk == 1) { > + if (sz > MALLOC_PAGESIZE / sizeof(uint64_t)) > + sz = MALLOC_PAGESIZE / sizeof(uint64_t); > + step = sz / 4; > + if (step == 0) > + step = 1; > + } > + for (i = 0; i < sz; i += step) { > + if (lp[i] != SOME_FREEJUNK_ULL) > + wrterror(pool, "write after free %p", p); > + } > +} > + > + > /* > * Cache maintenance. We keep at most malloc_cache pages cached. > * If the cache is becoming full, unmap pages in the cache for real, > @@ -663,7 +707,7 @@ delete(struct dir_info *d, struct region > * cache are in MALLOC_PAGESIZE units. > */ > static void > -unmap(struct dir_info *d, void *p, size_t sz, size_t clear, int junk) > +unmap(struct dir_info *d, void *p, size_t sz, size_t clear) > { > size_t psz = sz >> MALLOC_PAGESHIFT; > size_t rsz; > @@ -695,6 +739,8 @@ unmap(struct dir_info *d, void *p, size_ > r = >free_regions[(i + offset) & mask]; > if (r->p != NULL) { > rsz = r->size << MALLOC_PAGESHIFT; > + if
Re: further x509 cleanup in rpki-client
On Thu, Feb 18, 2021 at 02:41:39PM +0100, Claudio Jeker wrote: > Instead of iterating over all x509 extension and look for SKI and AKI use > X509_get_ext_d2i(). This reduces the complexity a fair bit. Also add > additional checks (e.g. make sure the extensions are non-critical). > More cleanup in cert.c should follow but one step at a time. ok tb
Re: rpki-client, create repo dir in parent process
On Thu, Feb 18, 2021 at 11:57:52AM +0100, Claudio Jeker wrote: > This diff moves the mkpath() call from the rsync child to the parent. > As a result the rsync process no longer needs cpath. It will also simplify > integration of RRDP since that will be another process. ok tb > > -- > :wq Claudio > > ? obj > Index: extern.h > === > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > retrieving revision 1.44 > diff -u -p -r1.44 extern.h > --- extern.h 16 Feb 2021 08:52:00 - 1.44 > +++ extern.h 18 Feb 2021 10:51:17 - > @@ -449,7 +449,7 @@ intoutput_json(FILE *, struct vrp_tre > void logx(const char *fmt, ...) > __attribute__((format(printf, 1, 2))); > > -int mkpath(const char *); > +int mkpath(int, const char *); > > #define RPKI_PATH_OUT_DIR "/var/db/rpki-client" > #define RPKI_PATH_BASE_DIR "/var/cache/rpki-client" > Index: main.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > retrieving revision 1.101 > diff -u -p -r1.101 main.c > --- main.c18 Feb 2021 10:10:20 - 1.101 > +++ main.c18 Feb 2021 10:51:17 - > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -91,6 +92,7 @@ RB_PROTOTYPE(filepath_tree, filepath, en > > static struct filepath_tree fpt = RB_INITIALIZER(); > static struct msgbuf procq, rsyncq; > +static int cachefd; > > const char *bird_tablename = "ROAS"; > > @@ -289,6 +291,15 @@ repo_fetch(struct repo *rp) > return; > } > > + /* > + * Create destination location. > + * Build up the tree to this point because GPL rsync(1) > + * will not build the destination for us. > + */ > + > + if (mkpath(cachefd, rp->local) == -1) > + err(1, "%s", rp->local); > + > logx("%s: pulling from network", rp->local); > if ((b = ibuf_dynamic(256, UINT_MAX)) == NULL) > err(1, NULL); > @@ -684,7 +695,7 @@ add_to_del(char **del, size_t *dsz, char > } > > static size_t > -repo_cleanup(const char *cachedir) > +repo_cleanup(int dirfd) > { > size_t i, delsz = 0; > char *argv[2], **del = NULL; > @@ -692,8 +703,8 @@ repo_cleanup(const char *cachedir) > FTSENT *e; > > /* change working directory to the cache directory */ > - if (chdir(cachedir) == -1) > - err(1, "%s: chdir", cachedir); > + if (fchdir(dirfd) == -1) > + err(1, "fchdir"); > > for (i = 0; i < rt.reposz; i++) { > if (asprintf([0], "%s", rt.repos[i].local) == -1) > @@ -866,6 +877,9 @@ main(int argc, char *argv[]) > goto usage; > } > > + if ((cachefd = open(cachedir, O_RDONLY, 0)) == -1) > + err(1, "cache directory %s", cachedir); > + > if (outformats == 0) > outformats = FORMAT_OPENBGPD; > > @@ -891,8 +905,8 @@ main(int argc, char *argv[]) > close(fd[1]); > > /* change working directory to the cache directory */ > - if (chdir(cachedir) == -1) > - err(1, "%s: chdir", cachedir); > + if (fchdir(cachefd) == -1) > + err(1, "fchdir"); > > /* Only allow access to the cache directory. */ > if (unveil(cachedir, "r") == -1) > @@ -924,8 +938,8 @@ main(int argc, char *argv[]) > close(fd[1]); > > /* change working directory to the cache directory */ > - if (chdir(cachedir) == -1) > - err(1, "%s: chdir", cachedir); > + if (fchdir(cachefd) == -1) > + err(1, "fchdir"); > > if (pledge("stdio rpath cpath proc exec unveil", NULL) > == -1) > @@ -1088,7 +1102,7 @@ main(int argc, char *argv[]) > if (outputfiles(, )) > rc = 1; > > - stats.del_files = repo_cleanup(cachedir); > + stats.del_files = repo_cleanup(cachefd); > > logx("Route Origin Authorizations: %zu (%zu failed parse, %zu invalid)", > stats.roas, stats.roas_fail, stats.roas_invalid); > Index: mkdir.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/mkdir.c,v > retrieving revision 1.1 > diff -u -p -r1.1 mkdir.c > --- mkdir.c 2 Feb 2021 18:33:11 - 1.1 > +++ mkdir.c 18 Feb 2021 10:51:17 - > @@ -43,7 +43,7 @@ > * dir_mode - file mode of intermediate directories > */ > int > -mkpath(const char *dir) > +mkpath(int dirfd, const char *dir) > { > char *path, *slash; > int done; > @@ -59,7 +59,7 @@ mkpath(const char *dir) > done = (*slash == '\0'); > *slash = '\0'; >
Re: use /dev/dri/ in xenocara
On Thu, Feb 18, 2021 at 09:18:51PM +1100, Jonathan Gray wrote: Ok matthieu@. > Index: lib/libdrm/xf86drm.h > === > RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v > retrieving revision 1.21 > diff -u -p -r1.21 xf86drm.h > --- lib/libdrm/xf86drm.h 11 Feb 2021 10:27:08 - 1.21 > +++ lib/libdrm/xf86drm.h 18 Feb 2021 10:02:03 - > @@ -76,18 +76,11 @@ extern "C" { > (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) > #define DRM_DEV_MODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) > > -#ifdef __OpenBSD__ > -#define DRM_DIR_NAME "/dev" > -#define DRM_PRIMARY_MINOR_NAME "drm" > -#define DRM_CONTROL_MINOR_NAME "drmC" > -#define DRM_RENDER_MINOR_NAME "drmR" > -#else > #define DRM_DIR_NAME "/dev/dri" > #define DRM_PRIMARY_MINOR_NAME "card" > #define DRM_CONTROL_MINOR_NAME "controlD" > #define DRM_RENDER_MINOR_NAME "renderD" > #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */ > -#endif > > #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" > #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" > Index: xserver/hw/xfree86/drivers/modesetting/driver.c > === > RCS file: /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v > retrieving revision 1.9 > diff -u -p -r1.9 driver.c > --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 09:30:54 > - 1.9 > +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 10:02:03 > - > @@ -226,7 +226,7 @@ open_hw(const char *dev) > else { > dev = getenv("KMSDEVICE"); > if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { > -dev = "/dev/drm0"; > +dev = "/dev/dri/card0"; > fd = priv_open_device(dev); > } > } -- Matthieu Herrb
Re: use /dev/dri/ in xenocara
Le 18/02/2021 à 13:11, Jonathan Gray a écrit : On Thu, Feb 18, 2021 at 11:34:19AM +, Stuart Henderson wrote: On 2021/02/18 22:24, Jonathan Gray wrote: On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: Date: Thu, 18 Feb 2021 21:18:51 +1100 From: Jonathan Gray I suspect that there are some ports that need to get their unveils updated if we do this. firefox ports were updated. Not aware of anything else in ports that unveils /dev/drm. unveils: not afaik others: gdm already handled it, some other ports will need patches changing: graphics/clutter/cogl/patches/patch-cogl_winsys_cogl-winsys-egl-kms_c graphics/waffle/patches/patch-src_waffle_gbm_wgbm_display_c x11/compton/patches/patch-src_compton_c x11/slim/patches/patch-slim_conf This is a display manager like xdm/gdm. The last upstream release was in 2013. I can patch it after the xenocara changes go in or perhaps we remove it as landry suggested in I am using slim on my children computers since many years now and it works very well. This could explain why there is nothing done upstream: no bug, no fix and features are already there. revision 1.55 date: 2020/07/24 05:41:37; author: landry; state: Exp; lines: +2 -3; commitid: yb1SZfwKZuXceEq0; Drop maintainership, i havent used slim in ages. Anyway it's more or less dead upstream since 7 years, which doesnt look good for a login manager.. candidate for removal ? xenodm works ootb, and is customizable once you grok X properties.. x11/picom/patches/patch-src_vsync_c /dev/drm will stay for now so the others can be updated later.
Re: use /dev/dri/ in xenocara
On Thu, Feb 18, 2021 at 11:11:15PM +1100, Jonathan Gray wrote: > On Thu, Feb 18, 2021 at 11:34:19AM +, Stuart Henderson wrote: > > On 2021/02/18 22:24, Jonathan Gray wrote: > > > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > > > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > > > > From: Jonathan Gray > > > > > > > > I suspect that there are some ports that need to get their unveils > > > > updated if we do this. > > > > > > firefox ports were updated. Not aware of anything else in ports that > > > unveils /dev/drm. > > > > unveils: not afaik > > > > others: gdm already handled it, some other ports will need patches changing: > > > > graphics/clutter/cogl/patches/patch-cogl_winsys_cogl-winsys-egl-kms_c > > graphics/waffle/patches/patch-src_waffle_gbm_wgbm_display_c > > x11/compton/patches/patch-src_compton_c > > x11/slim/patches/patch-slim_conf > > This is a display manager like xdm/gdm. The last upstream release was > in 2013. I can patch it after the xenocara changes go in or perhaps we > remove it as landry suggested in there's a less dead upstream at https://github.com/PeteGozz/slim. FreeBSD and debian still ship a package for slim :) I *might* have a look at updating the port to this github fork. Landry
further x509 cleanup in rpki-client
Instead of iterating over all x509 extension and look for SKI and AKI use X509_get_ext_d2i(). This reduces the complexity a fair bit. Also add additional checks (e.g. make sure the extensions are non-critical). More cleanup in cert.c should follow but one step at a time. -- :wq Claudio Index: cert.c === RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v retrieving revision 1.26 diff -u -p -r1.26 cert.c --- cert.c 16 Feb 2021 07:58:30 - 1.26 +++ cert.c 16 Feb 2021 20:04:54 - @@ -1080,19 +1080,10 @@ cert_parse_inner(X509 **xp, const char * /* ignored here, handled later */ break; case NID_info_access: - free(p.res->aia); - p.res->aia = x509_get_aia(x, p.fn); - c = (p.res->aia != NULL); break; case NID_authority_key_identifier: - free(p.res->aki); - p.res->aki = x509_get_aki_ext(ext, p.fn); - c = (p.res->aki != NULL); break; case NID_subject_key_identifier: - free(p.res->ski); - p.res->ski = x509_get_ski_ext(ext, p.fn); - c = (p.res->ski != NULL); break; default: /* { @@ -1107,8 +1098,12 @@ cert_parse_inner(X509 **xp, const char * goto out; } - if (!ta) + p.res->aki = x509_get_aki(x, ta, p.fn); + p.res->ski = x509_get_ski(x, p.fn); + if (!ta) { + p.res->aia = x509_get_aia(x, p.fn); p.res->crl = x509_get_crl(x, p.fn); + } /* Validation on required fields. */ @@ -1131,6 +1126,16 @@ cert_parse_inner(X509 **xp, const char * } else if (!ta && strcmp(p.res->aki, p.res->ski) == 0) { warnx("%s: RFC 6487 section 8.4.2: " "non-trust anchor AKI may not match SKI", p.fn); + goto out; + } + + if (!ta && p.res->aia == NULL) { + warnx("%s: RFC 6487 section 8.4.7: " + "non-trust anchor missing AIA", p.fn); + goto out; + } else if (ta && p.res->aia != NULL) { + warnx("%s: RFC 6487 section 8.4.7: " + "trust anchor must not have AIA", p.fn); goto out; } Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.44 diff -u -p -r1.44 extern.h --- extern.h16 Feb 2021 08:52:00 - 1.44 +++ extern.h16 Feb 2021 20:03:40 - @@ -421,12 +421,12 @@ void io_str_read(int, char **); /* X509 helpers. */ char *x509_get_aia(X509 *, const char *); -char *x509_get_aki_ext(X509_EXTENSION *, const char *); -char *x509_get_ski_ext(X509_EXTENSION *, const char *); +char *x509_get_aki(X509 *, int, const char *); +char *x509_get_ski(X509 *, const char *); int x509_get_extensions(X509 *, const char *, char **, char **, char **); char *x509_get_crl(X509 *, const char *); -char *x509_crl_get_aki(X509_CRL *); +char *x509_crl_get_aki(X509_CRL *, const char *); /* Output! */ Index: parser.c === RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v retrieving revision 1.4 diff -u -p -r1.4 parser.c --- parser.c4 Feb 2021 14:32:01 - 1.4 +++ parser.c16 Feb 2021 17:46:25 - @@ -361,7 +361,8 @@ proc_parser_crl(struct entity *entp, X50 if ((x509_crl = crl_parse(entp->file)) != NULL) { if ((crl = malloc(sizeof(*crl))) == NULL) err(1, NULL); - if ((crl->aki = x509_crl_get_aki(x509_crl)) == NULL) + if ((crl->aki = x509_crl_get_aki(x509_crl, entp->file)) == + NULL) errx(1, "x509_crl_get_aki failed"); crl->x509_crl = x509_crl; Index: x509.c === RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v retrieving revision 1.15 diff -u -p -r1.15 x509.c --- x509.c 16 Feb 2021 07:58:30 - 1.15 +++ x509.c 18 Feb 2021 13:28:41 - @@ -30,88 +30,67 @@ #include "extern.h" /* - * Wrapper around ASN1_get_object() that preserves the current start - * state and returns a more meaningful value. - * Return zero on failure, non-zero on success. - */ -static int -ASN1_frame(const char *fn, size_t sz, -const unsigned char **cnt, long *cntsz, int *tag) -{ - int ret, pcls; - - assert(cnt != NULL && *cnt != NULL); - assert(sz > 0); - ret =
Re: use /dev/dri/ in xenocara
On Thu, Feb 18, 2021 at 11:34:19AM +, Stuart Henderson wrote: > On 2021/02/18 22:24, Jonathan Gray wrote: > > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > > > From: Jonathan Gray > > > > > > I suspect that there are some ports that need to get their unveils > > > updated if we do this. > > > > firefox ports were updated. Not aware of anything else in ports that > > unveils /dev/drm. > > unveils: not afaik > > others: gdm already handled it, some other ports will need patches changing: > > graphics/clutter/cogl/patches/patch-cogl_winsys_cogl-winsys-egl-kms_c > graphics/waffle/patches/patch-src_waffle_gbm_wgbm_display_c > x11/compton/patches/patch-src_compton_c > x11/slim/patches/patch-slim_conf This is a display manager like xdm/gdm. The last upstream release was in 2013. I can patch it after the xenocara changes go in or perhaps we remove it as landry suggested in revision 1.55 date: 2020/07/24 05:41:37; author: landry; state: Exp; lines: +2 -3; commitid: yb1SZfwKZuXceEq0; Drop maintainership, i havent used slim in ages. Anyway it's more or less dead upstream since 7 years, which doesnt look good for a login manager.. candidate for removal ? xenodm works ootb, and is customizable once you grok X properties.. > x11/picom/patches/patch-src_vsync_c /dev/drm will stay for now so the others can be updated later.
Re: use /dev/dri/ in xenocara
On Thu, Feb 18, 2021 at 12:29:29PM +0100, Mark Kettenis wrote: > > Date: Thu, 18 Feb 2021 22:24:10 +1100 > > From: Jonathan Gray > > > > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > > > From: Jonathan Gray > > > > > > I suspect that there are some ports that need to get their unveils > > > updated if we do this. > > > > firefox ports were updated. Not aware of anything else in ports that > > unveils /dev/drm. > > Doesn't chromium use unveil? chromium opens the device outside of the sandbox > > Anyway, this should make unveiling easier and if you are coordinating > with the ports folks, this is ok kettenis@ > > > > > Index: lib/libdrm/xf86drm.h > > > > === > > > > RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v > > > > retrieving revision 1.21 > > > > diff -u -p -r1.21 xf86drm.h > > > > --- lib/libdrm/xf86drm.h11 Feb 2021 10:27:08 - 1.21 > > > > +++ lib/libdrm/xf86drm.h18 Feb 2021 10:02:03 - > > > > @@ -76,18 +76,11 @@ extern "C" { > > > > (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) > > > > #define DRM_DEV_MODE(S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) > > > > > > > > -#ifdef __OpenBSD__ > > > > -#define DRM_DIR_NAME "/dev" > > > > -#define DRM_PRIMARY_MINOR_NAME "drm" > > > > -#define DRM_CONTROL_MINOR_NAME "drmC" > > > > -#define DRM_RENDER_MINOR_NAME "drmR" > > > > -#else > > > > #define DRM_DIR_NAME "/dev/dri" > > > > #define DRM_PRIMARY_MINOR_NAME "card" > > > > #define DRM_CONTROL_MINOR_NAME "controlD" > > > > #define DRM_RENDER_MINOR_NAME "renderD" > > > > #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility > > > > */ > > > > -#endif > > > > > > > > #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" > > > > #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" > > > > Index: xserver/hw/xfree86/drivers/modesetting/driver.c > > > > === > > > > RCS file: > > > > /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v > > > > retrieving revision 1.9 > > > > diff -u -p -r1.9 driver.c > > > > --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 > > > > 09:30:54 - 1.9 > > > > +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 > > > > 10:02:03 - > > > > @@ -226,7 +226,7 @@ open_hw(const char *dev) > > > > else { > > > > dev = getenv("KMSDEVICE"); > > > > if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { > > > > -dev = "/dev/drm0"; > > > > +dev = "/dev/dri/card0"; > > > > fd = priv_open_device(dev); > > > > } > > > > } > > > > > > > > > > > > > >
Re: use /dev/dri/ in xenocara
On 2021/02/18 22:24, Jonathan Gray wrote: > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > > From: Jonathan Gray > > > > I suspect that there are some ports that need to get their unveils > > updated if we do this. > > firefox ports were updated. Not aware of anything else in ports that > unveils /dev/drm. unveils: not afaik others: gdm already handled it, some other ports will need patches changing: graphics/clutter/cogl/patches/patch-cogl_winsys_cogl-winsys-egl-kms_c graphics/waffle/patches/patch-src_waffle_gbm_wgbm_display_c x11/compton/patches/patch-src_compton_c x11/slim/patches/patch-slim_conf x11/picom/patches/patch-src_vsync_c
Re: use /dev/dri/ in xenocara
> Date: Thu, 18 Feb 2021 22:24:10 +1100 > From: Jonathan Gray > > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > > From: Jonathan Gray > > > > I suspect that there are some ports that need to get their unveils > > updated if we do this. > > firefox ports were updated. Not aware of anything else in ports that > unveils /dev/drm. Doesn't chromium use unveil? Anyway, this should make unveiling easier and if you are coordinating with the ports folks, this is ok kettenis@ > > > Index: lib/libdrm/xf86drm.h > > > === > > > RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v > > > retrieving revision 1.21 > > > diff -u -p -r1.21 xf86drm.h > > > --- lib/libdrm/xf86drm.h 11 Feb 2021 10:27:08 - 1.21 > > > +++ lib/libdrm/xf86drm.h 18 Feb 2021 10:02:03 - > > > @@ -76,18 +76,11 @@ extern "C" { > > > (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) > > > #define DRM_DEV_MODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) > > > > > > -#ifdef __OpenBSD__ > > > -#define DRM_DIR_NAME "/dev" > > > -#define DRM_PRIMARY_MINOR_NAME "drm" > > > -#define DRM_CONTROL_MINOR_NAME "drmC" > > > -#define DRM_RENDER_MINOR_NAME "drmR" > > > -#else > > > #define DRM_DIR_NAME "/dev/dri" > > > #define DRM_PRIMARY_MINOR_NAME "card" > > > #define DRM_CONTROL_MINOR_NAME "controlD" > > > #define DRM_RENDER_MINOR_NAME "renderD" > > > #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */ > > > -#endif > > > > > > #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" > > > #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" > > > Index: xserver/hw/xfree86/drivers/modesetting/driver.c > > > === > > > RCS file: /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v > > > retrieving revision 1.9 > > > diff -u -p -r1.9 driver.c > > > --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 > > > 09:30:54 - 1.9 > > > +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 > > > 10:02:03 - > > > @@ -226,7 +226,7 @@ open_hw(const char *dev) > > > else { > > > dev = getenv("KMSDEVICE"); > > > if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { > > > -dev = "/dev/drm0"; > > > +dev = "/dev/dri/card0"; > > > fd = priv_open_device(dev); > > > } > > > } > > > > > > > > >
Re: use /dev/dri/ in xenocara
On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote: > > Date: Thu, 18 Feb 2021 21:18:51 +1100 > > From: Jonathan Gray > > I suspect that there are some ports that need to get their unveils > updated if we do this. firefox ports were updated. Not aware of anything else in ports that unveils /dev/drm. > > > Index: lib/libdrm/xf86drm.h > > === > > RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v > > retrieving revision 1.21 > > diff -u -p -r1.21 xf86drm.h > > --- lib/libdrm/xf86drm.h11 Feb 2021 10:27:08 - 1.21 > > +++ lib/libdrm/xf86drm.h18 Feb 2021 10:02:03 - > > @@ -76,18 +76,11 @@ extern "C" { > > (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) > > #define DRM_DEV_MODE(S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) > > > > -#ifdef __OpenBSD__ > > -#define DRM_DIR_NAME "/dev" > > -#define DRM_PRIMARY_MINOR_NAME "drm" > > -#define DRM_CONTROL_MINOR_NAME "drmC" > > -#define DRM_RENDER_MINOR_NAME "drmR" > > -#else > > #define DRM_DIR_NAME "/dev/dri" > > #define DRM_PRIMARY_MINOR_NAME "card" > > #define DRM_CONTROL_MINOR_NAME "controlD" > > #define DRM_RENDER_MINOR_NAME "renderD" > > #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */ > > -#endif > > > > #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" > > #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" > > Index: xserver/hw/xfree86/drivers/modesetting/driver.c > > === > > RCS file: /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v > > retrieving revision 1.9 > > diff -u -p -r1.9 driver.c > > --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 09:30:54 > > - 1.9 > > +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 10:02:03 > > - > > @@ -226,7 +226,7 @@ open_hw(const char *dev) > > else { > > dev = getenv("KMSDEVICE"); > > if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { > > -dev = "/dev/drm0"; > > +dev = "/dev/dri/card0"; > > fd = priv_open_device(dev); > > } > > } > > > > >
Re: use /dev/dri/ in xenocara
> Date: Thu, 18 Feb 2021 21:18:51 +1100 > From: Jonathan Gray I suspect that there are some ports that need to get their unveils updated if we do this. > Index: lib/libdrm/xf86drm.h > === > RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v > retrieving revision 1.21 > diff -u -p -r1.21 xf86drm.h > --- lib/libdrm/xf86drm.h 11 Feb 2021 10:27:08 - 1.21 > +++ lib/libdrm/xf86drm.h 18 Feb 2021 10:02:03 - > @@ -76,18 +76,11 @@ extern "C" { > (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) > #define DRM_DEV_MODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) > > -#ifdef __OpenBSD__ > -#define DRM_DIR_NAME "/dev" > -#define DRM_PRIMARY_MINOR_NAME "drm" > -#define DRM_CONTROL_MINOR_NAME "drmC" > -#define DRM_RENDER_MINOR_NAME "drmR" > -#else > #define DRM_DIR_NAME "/dev/dri" > #define DRM_PRIMARY_MINOR_NAME "card" > #define DRM_CONTROL_MINOR_NAME "controlD" > #define DRM_RENDER_MINOR_NAME "renderD" > #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */ > -#endif > > #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" > #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" > Index: xserver/hw/xfree86/drivers/modesetting/driver.c > === > RCS file: /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v > retrieving revision 1.9 > diff -u -p -r1.9 driver.c > --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 09:30:54 > - 1.9 > +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 10:02:03 > - > @@ -226,7 +226,7 @@ open_hw(const char *dev) > else { > dev = getenv("KMSDEVICE"); > if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { > -dev = "/dev/drm0"; > +dev = "/dev/dri/card0"; > fd = priv_open_device(dev); > } > } > >
rpki-client, create repo dir in parent process
This diff moves the mkpath() call from the rsync child to the parent. As a result the rsync process no longer needs cpath. It will also simplify integration of RRDP since that will be another process. -- :wq Claudio ? obj Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.44 diff -u -p -r1.44 extern.h --- extern.h16 Feb 2021 08:52:00 - 1.44 +++ extern.h18 Feb 2021 10:51:17 - @@ -449,7 +449,7 @@ int output_json(FILE *, struct vrp_tre void logx(const char *fmt, ...) __attribute__((format(printf, 1, 2))); -intmkpath(const char *); +intmkpath(int, const char *); #defineRPKI_PATH_OUT_DIR "/var/db/rpki-client" #defineRPKI_PATH_BASE_DIR "/var/cache/rpki-client" Index: main.c === RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v retrieving revision 1.101 diff -u -p -r1.101 main.c --- main.c 18 Feb 2021 10:10:20 - 1.101 +++ main.c 18 Feb 2021 10:51:17 - @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -91,6 +92,7 @@ RB_PROTOTYPE(filepath_tree, filepath, en static struct filepath_treefpt = RB_INITIALIZER(); static struct msgbuf procq, rsyncq; +static int cachefd; const char *bird_tablename = "ROAS"; @@ -289,6 +291,15 @@ repo_fetch(struct repo *rp) return; } + /* +* Create destination location. +* Build up the tree to this point because GPL rsync(1) +* will not build the destination for us. +*/ + + if (mkpath(cachefd, rp->local) == -1) + err(1, "%s", rp->local); + logx("%s: pulling from network", rp->local); if ((b = ibuf_dynamic(256, UINT_MAX)) == NULL) err(1, NULL); @@ -684,7 +695,7 @@ add_to_del(char **del, size_t *dsz, char } static size_t -repo_cleanup(const char *cachedir) +repo_cleanup(int dirfd) { size_t i, delsz = 0; char *argv[2], **del = NULL; @@ -692,8 +703,8 @@ repo_cleanup(const char *cachedir) FTSENT *e; /* change working directory to the cache directory */ - if (chdir(cachedir) == -1) - err(1, "%s: chdir", cachedir); + if (fchdir(dirfd) == -1) + err(1, "fchdir"); for (i = 0; i < rt.reposz; i++) { if (asprintf([0], "%s", rt.repos[i].local) == -1) @@ -866,6 +877,9 @@ main(int argc, char *argv[]) goto usage; } + if ((cachefd = open(cachedir, O_RDONLY, 0)) == -1) + err(1, "cache directory %s", cachedir); + if (outformats == 0) outformats = FORMAT_OPENBGPD; @@ -891,8 +905,8 @@ main(int argc, char *argv[]) close(fd[1]); /* change working directory to the cache directory */ - if (chdir(cachedir) == -1) - err(1, "%s: chdir", cachedir); + if (fchdir(cachefd) == -1) + err(1, "fchdir"); /* Only allow access to the cache directory. */ if (unveil(cachedir, "r") == -1) @@ -924,8 +938,8 @@ main(int argc, char *argv[]) close(fd[1]); /* change working directory to the cache directory */ - if (chdir(cachedir) == -1) - err(1, "%s: chdir", cachedir); + if (fchdir(cachefd) == -1) + err(1, "fchdir"); if (pledge("stdio rpath cpath proc exec unveil", NULL) == -1) @@ -1088,7 +1102,7 @@ main(int argc, char *argv[]) if (outputfiles(, )) rc = 1; - stats.del_files = repo_cleanup(cachedir); + stats.del_files = repo_cleanup(cachefd); logx("Route Origin Authorizations: %zu (%zu failed parse, %zu invalid)", stats.roas, stats.roas_fail, stats.roas_invalid); Index: mkdir.c === RCS file: /cvs/src/usr.sbin/rpki-client/mkdir.c,v retrieving revision 1.1 diff -u -p -r1.1 mkdir.c --- mkdir.c 2 Feb 2021 18:33:11 - 1.1 +++ mkdir.c 18 Feb 2021 10:51:17 - @@ -43,7 +43,7 @@ * dir_mode - file mode of intermediate directories */ int -mkpath(const char *dir) +mkpath(int dirfd, const char *dir) { char *path, *slash; int done; @@ -59,7 +59,7 @@ mkpath(const char *dir) done = (*slash == '\0'); *slash = '\0'; - if (mkdir(path, 0700) == -1 && errno != EEXIST) { + if (mkdirat(dirfd, path, 0700) == -1 && errno != EEXIST) { free(path); return (-1);
use /dev/dri/ in xenocara
Index: lib/libdrm/xf86drm.h === RCS file: /cvs/xenocara/lib/libdrm/xf86drm.h,v retrieving revision 1.21 diff -u -p -r1.21 xf86drm.h --- lib/libdrm/xf86drm.h11 Feb 2021 10:27:08 - 1.21 +++ lib/libdrm/xf86drm.h18 Feb 2021 10:02:03 - @@ -76,18 +76,11 @@ extern "C" { (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) #define DRM_DEV_MODE(S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) -#ifdef __OpenBSD__ -#define DRM_DIR_NAME "/dev" -#define DRM_PRIMARY_MINOR_NAME "drm" -#define DRM_CONTROL_MINOR_NAME "drmC" -#define DRM_RENDER_MINOR_NAME "drmR" -#else #define DRM_DIR_NAME "/dev/dri" #define DRM_PRIMARY_MINOR_NAME "card" #define DRM_CONTROL_MINOR_NAME "controlD" #define DRM_RENDER_MINOR_NAME "renderD" #define DRM_PROC_NAME "/proc/dri/" /* For backward Linux compatibility */ -#endif #define DRM_DEV_NAME "%s/" DRM_PRIMARY_MINOR_NAME "%d" #define DRM_CONTROL_DEV_NAME "%s/" DRM_CONTROL_MINOR_NAME "%d" Index: xserver/hw/xfree86/drivers/modesetting/driver.c === RCS file: /cvs/xenocara/xserver/hw/xfree86/drivers/modesetting/driver.c,v retrieving revision 1.9 diff -u -p -r1.9 driver.c --- xserver/hw/xfree86/drivers/modesetting/driver.c 12 Dec 2020 09:30:54 - 1.9 +++ xserver/hw/xfree86/drivers/modesetting/driver.c 18 Feb 2021 10:02:03 - @@ -226,7 +226,7 @@ open_hw(const char *dev) else { dev = getenv("KMSDEVICE"); if ((NULL == dev) || ((fd = priv_open_device(dev)) == -1)) { -dev = "/dev/drm0"; +dev = "/dev/dri/card0"; fd = priv_open_device(dev); } }
Re: if calloc() needs nmemb and size, why doesn't freezero()?
On Wed, Feb 17, 2021 at 11:05:49AM -0700, Theo de Raadt wrote: > Luke Small wrote: > > > I guess I always thought there'd be some more substantial overflow > > mitigation. > > You have to free with the exact same size as allocation. Small correction: the size may be smaller than the original. In that case, only a partial clear is guaranteed, the deallocation will still be for the full allocation. Originally we were more strict, but iirc that was causing to much headaches for some. See https://cvsweb.openbsd.org/src/lib/libc/stdlib/malloc.c?rev=1.221 But the point stands: nmemb * size does not overflow, since the original allocation would have overflowed and thus failed. -Otto > > nmemb and size did not change. > > The math has already been checked, and regular codeflows will store the > multiple in a single variable after successful checking, for > reuse. > > > Would it be too much hand-holding to put in the manpage that to avoid > > potential > > freeezero() integer overflow, > > it may be useful to run freezero() as freezero((size_t)nmemb * > > (size_t)size); > > Wow, Those casts make it very clear you don't understand C, if you do > that kind of stuff elsewhere you are introducing problems. > > Sorry no you are wrong. >