Re: if calloc() needs nmemb and size, why doesn't freezero()?

2021-02-18 Thread Otto Moerbeek
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

2021-02-18 Thread Greg Steuck
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()?

2021-02-18 Thread Luke Small
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

2021-02-18 Thread Scott Cheloha
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

2021-02-18 Thread Theo de Raadt
This diff will land in snapshots.



Re: if calloc() needs nmemb and size, why doesn't freezero()?

2021-02-18 Thread Theo de Raadt
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'

2021-02-18 Thread Vitaliy Makkoveev
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()?

2021-02-18 Thread Luke Small
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

2021-02-18 Thread Stuart Henderson
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

2021-02-18 Thread Otto Moerbeek
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

2021-02-18 Thread Theo Buehler
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

2021-02-18 Thread Theo Buehler
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

2021-02-18 Thread Matthieu Herrb
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

2021-02-18 Thread Damien Couderc

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

2021-02-18 Thread Landry Breuil
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

2021-02-18 Thread Claudio Jeker
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

2021-02-18 Thread Jonathan Gray
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

2021-02-18 Thread Jonathan Gray
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

2021-02-18 Thread Stuart Henderson
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

2021-02-18 Thread Mark Kettenis
> 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

2021-02-18 Thread 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.

> 
> > 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

2021-02-18 Thread Mark Kettenis
> 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

2021-02-18 Thread Claudio Jeker
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

2021-02-18 Thread Jonathan Gray
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()?

2021-02-18 Thread Otto Moerbeek
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.
>