rename pf_state_key and pf_state_item members

2022-12-18 Thread David Gwynne
this prefixes (some of the) pf_state_key struct members with sk_, and
pf_state_item struct members with si_ (and renames one completely).

this makes searching for and the struct members so much easier, which in
turn makes tweaking code around them a lot easier too. sk_refcnt in
particular would have been a lot nicer to fiddle with than just refcnt.
pf_state structs also have a refcnt, which is annoying.

i hope i didn't mess up the (sk_)states list traversals.

ok?

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1157
diff -u -p -r1.1157 pf.c
--- pf.c16 Dec 2022 02:05:44 -  1.1157
+++ pf.c19 Dec 2022 06:39:45 -
@@ -315,7 +315,7 @@ struct pf_state_tree_id tree_id;
 struct pf_state_list pf_state_list = PF_STATE_LIST_INITIALIZER(pf_state_list);
 
 RB_GENERATE(pf_src_tree, pf_src_node, entry, pf_src_compare);
-RB_GENERATE(pf_state_tree, pf_state_key, entry, pf_state_compare_key);
+RB_GENERATE(pf_state_tree, pf_state_key, sk_entry, pf_state_compare_key);
 RB_GENERATE(pf_state_tree_id, pf_state,
 entry_id, pf_state_compare_id);
 
@@ -736,24 +736,25 @@ pf_state_key_attach(struct pf_state_key 
PF_ASSERT_LOCKED();
 
KASSERT(s->key[idx] == NULL);
-   sk->removed = 0;
+   sk->sk_removed = 0;
cur = RB_INSERT(pf_state_tree, _statetbl, sk);
if (cur != NULL) {
-   sk->removed = 1;
+   sk->sk_removed = 1;
/* key exists. check for same kif, if none, add to key */
-   TAILQ_FOREACH(si, >states, entry) {
-   if (si->s->kif == s->kif &&
-   ((si->s->key[PF_SK_WIRE]->af == sk->af &&
-si->s->direction == s->direction) ||
-   (si->s->key[PF_SK_WIRE]->af !=
-si->s->key[PF_SK_STACK]->af &&
-sk->af == si->s->key[PF_SK_STACK]->af &&
-si->s->direction != s->direction))) {
+   TAILQ_FOREACH(si, >sk_states, si_entry) {
+   struct pf_state *tst = si->si_st;
+   if (tst->kif == s->kif &&
+   ((tst->key[PF_SK_WIRE]->af == sk->af &&
+tst->direction == s->direction) ||
+   (tst->key[PF_SK_WIRE]->af !=
+tst->key[PF_SK_STACK]->af &&
+sk->af == tst->key[PF_SK_STACK]->af &&
+tst->direction != s->direction))) {
int reuse = 0;
 
if (sk->proto == IPPROTO_TCP &&
-   si->s->src.state >= TCPS_FIN_WAIT_2 &&
-   si->s->dst.state >= TCPS_FIN_WAIT_2)
+   tst->src.state >= TCPS_FIN_WAIT_2 &&
+   tst->dst.state >= TCPS_FIN_WAIT_2)
reuse = 1;
if (pf_status.debug >= LOG_NOTICE) {
log(LOG_NOTICE,
@@ -766,16 +767,16 @@ pf_state_key_attach(struct pf_state_key 
(idx == PF_SK_WIRE) ?  sk : NULL,
(idx == PF_SK_STACK) ?  sk : NULL);
addlog(", existing: ");
-   pf_print_state_parts(si->s,
+   pf_print_state_parts(tst,
(idx == PF_SK_WIRE) ?  sk : NULL,
(idx == PF_SK_STACK) ?  sk : NULL);
addlog("\n");
}
if (reuse) {
-   pf_set_protostate(si->s, PF_PEER_BOTH,
+   pf_set_protostate(tst, PF_PEER_BOTH,
TCPS_CLOSED);
/* remove late or sks can go away */
-   olds = si->s;
+   olds = tst;
} else {
pf_state_key_unref(sk);
return (NULL);  /* collision! */
@@ -789,10 +790,10 @@ pf_state_key_attach(struct pf_state_key 
}
 
if ((si = pool_get(_state_item_pl, PR_NOWAIT)) == NULL) {
-   if (TAILQ_EMPTY(>states)) {
+   if (TAILQ_EMPTY(>sk_states)) {
KASSERT(cur == NULL);
RB_REMOVE(pf_state_tree, _statetbl, sk);
-   sk->removed = 1;
+   sk->sk_removed = 1;
pf_state_key_unref(sk);
}
 
@@ -800,13 +801,13 @@ 

Re: netcat: bump BUFSIZE to 64k?

2022-12-18 Thread Loganaden Velvindron
On Sun, 18 Dec 2022 at 17:01, Theo Buehler  wrote:
>
> This is the remaining bit of mpf's recent netcat diff. The commit log
> shows that it was bumped to 64k in the past, but that was promptly
> reverted due to concerns of buffer bloat caused by atomicio blocking
> traffic in the other direction.
>
> I don't know if things are different enough 8 years later that this can
> be reconsidered. Not my area, just throwing it out there so it doesn't
> get lost.
>

If there's a push to reduce bufferbloat, then maybe have a look at this too:

https://datatracker.ietf.org/doc/id/draft-gettys-iw10-considered-harmful-00.html

Maybe revert IW from 10 to 4 :-) ?

> Index: netcat.c
> ===
> RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.224
> diff -u -p -r1.224 netcat.c
> --- netcat.c18 Dec 2022 12:53:18 -  1.224
> +++ netcat.c18 Dec 2022 12:54:58 -
> @@ -66,7 +66,7 @@
>  #define POLL_NETOUT1
>  #define POLL_NETIN 2
>  #define POLL_STDOUT3
> -#define BUFSIZE16384
> +#define BUFSIZE65536
>
>  #define TLS_NOVERIFY   (1 << 1)
>  #define TLS_NONAME (1 << 2)
>



is this rge crash known?

2022-12-18 Thread Geoff Steckel

nc of 0's from one rge to another at full speed crashes
in the input interrupt path with corruption of the memory
pool used for the mbufs
It's 100% reproduceable.
Probably race condition & use-after-free or some such
since it takes 200,000+ packets to happen.
I suspect that the crash happens when the corruption is detected
some time after it actually occurs.

This is a ---very--- abbreviated description.
If this crash hasn't been seen before I'll submit a full bug report.

Is there any more info from sysctls, ddb, etc. that would help?
I can put in breakpoints & dump (small) memory areas.
If running the most recent snapshot would give better info I can do that.
A serial console to get an exact transcript is possible but not easy.

Any suggestions of something I can do to help beyond a standard bug
report welcomed. I can run test patches easily.

This is with the standard 1500 mtu.
Setting mtu to 8000 trashes memory enough to cause a kernel protection 
fault.


OpenBSD 7.2 (GENERIC.MP) #758: Tue Sep 27 11:57:54 MDT 2022
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 67997949952 (64847MB)
avail mem = 65919754240 (62865MB)

bios0 at mainbus0: SMBIOS rev. 3.3 @ 0xe6cc0 (33 entries)
bios0: vendor American Megatrends International, LLC. version "P2.10" 
date 08/02/2021

bios0: ASRock B550 Phantom Gaming 4
...
cpu0: AMD Ryzen 5 5600G with Radeon Graphics, 3892.77 MHz, 19-50-00
...
rge0 at pci3 dev 0 function 0 "Realtek RTL8125" rev 0x00: msi, address 
78:2d:7e:12:5a:d6


panic hand copied:
sched_idle
apicpu_idle
Xintr_ioapic_edge22_???
intr_handler
rge_intr
rge_rxeof
rge_newbuf
mclget(0, 2, 2400)
pool_get
pool_cache_get
panic: pool cache item mcl9k cp freelist modified
(panic on test 1)
fd800b076880 + 16 0x64000403b8f3400 != 0x46b8689556980a7
(panic on test 2 - all previous data identical)
fd800b118d40 + 16 0x64000409da03400 != 0x5be8fd0cf17429b

thanks for any input,
Geoff Steckel



malloc and immutable

2022-12-18 Thread Otto Moerbeek
Hi,

This is the latest verion of a diff I sent around previously, but now
it's time this gets wider testing for real.

The main purpose is to rearrange malloc init in such a way that a few
pages containing mallic internal data structures can be made
immutable. In addtion to that, each pools is inited on-demand instead
of always.

So please test! I did not get a lot of feedback on the earlier versions.

-Otto

Index: stdlib/malloc.c
===
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.275
diff -u -p -r1.275 malloc.c
--- stdlib/malloc.c 14 Oct 2022 04:38:39 -  1.275
+++ stdlib/malloc.c 18 Dec 2022 18:57:16 -
@@ -142,6 +142,7 @@ struct dir_info {
int malloc_junk;/* junk fill? */
int mmap_flag;  /* extra flag for mmap */
int mutex;
+   int malloc_mt;  /* multi-threaded mode? */
/* lists of free chunk info structs */
struct chunk_head chunk_info_list[MALLOC_MAXSHIFT + 1];
/* lists of chunks with free slots */
@@ -181,8 +182,6 @@ struct dir_info {
 #endif /* MALLOC_STATS */
u_int32_t canary2;
 };
-#define DIR_INFO_RSZ   ((sizeof(struct dir_info) + MALLOC_PAGEMASK) & \
-   ~MALLOC_PAGEMASK)
 
 static void unmap(struct dir_info *d, void *p, size_t sz, size_t clear);
 
@@ -208,7 +207,6 @@ struct malloc_readonly {
/* Main bookkeeping information */
struct dir_info *malloc_pool[_MALLOC_MUTEXES];
u_int   malloc_mutexes; /* how much in actual use? */
-   int malloc_mt;  /* multi-threaded mode? */
int malloc_freecheck;   /* Extensive double free check */
int malloc_freeunmap;   /* mprotect free pages PROT_NONE? */
int def_malloc_junk;/* junk fill? */
@@ -258,7 +256,7 @@ static void malloc_exit(void);
 static inline void
 _MALLOC_LEAVE(struct dir_info *d)
 {
-   if (mopts.malloc_mt) {
+   if (d->malloc_mt) {
d->active--;
_MALLOC_UNLOCK(d->mutex);
}
@@ -267,7 +265,7 @@ _MALLOC_LEAVE(struct dir_info *d)
 static inline void
 _MALLOC_ENTER(struct dir_info *d)
 {
-   if (mopts.malloc_mt) {
+   if (d->malloc_mt) {
_MALLOC_LOCK(d->mutex);
d->active++;
}
@@ -292,7 +290,7 @@ hash(void *p)
 static inline struct dir_info *
 getpool(void)
 {
-   if (!mopts.malloc_mt)
+   if (mopts.malloc_pool[1] == NULL || !mopts.malloc_pool[1]->malloc_mt)
return mopts.malloc_pool[1];
else/* first one reserved for special pool */
return mopts.malloc_pool[1 + TIB_GET()->tib_tid %
@@ -497,46 +495,22 @@ omalloc_init(void)
 }
 
 static void
-omalloc_poolinit(struct dir_info **dp, int mmap_flag)
+omalloc_poolinit(struct dir_info *d, int mmap_flag)
 {
-   char *p;
-   size_t d_avail, regioninfo_size;
-   struct dir_info *d;
int i, j;
 
-   /*
-* Allocate dir_info with a guard page on either side. Also
-* randomise offset inside the page at which the dir_info
-* lies (subject to alignment by 1 << MALLOC_MINSHIFT)
-*/
-   if ((p = MMAPNONE(DIR_INFO_RSZ + (MALLOC_PAGESIZE * 2), mmap_flag)) ==
-   MAP_FAILED)
-   wrterror(NULL, "malloc init mmap failed");
-   mprotect(p + MALLOC_PAGESIZE, DIR_INFO_RSZ, PROT_READ | PROT_WRITE);
-   d_avail = (DIR_INFO_RSZ - sizeof(*d)) >> MALLOC_MINSHIFT;
-   d = (struct dir_info *)(p + MALLOC_PAGESIZE +
-   (arc4random_uniform(d_avail) << MALLOC_MINSHIFT));
-
-   rbytes_init(d);
-   d->regions_free = d->regions_total = MALLOC_INITIAL_REGIONS;
-   regioninfo_size = d->regions_total * sizeof(struct region_info);
-   d->r = MMAP(regioninfo_size, mmap_flag);
-   if (d->r == MAP_FAILED) {
-   d->regions_total = 0;
-   wrterror(NULL, "malloc init mmap failed");
-   }
+   d->r = NULL;
+   d->rbytesused = sizeof(d->rbytes);
+   d->regions_free = d->regions_total = 0;
for (i = 0; i <= MALLOC_MAXSHIFT; i++) {
LIST_INIT(>chunk_info_list[i]);
for (j = 0; j < MALLOC_CHUNK_LISTS; j++)
LIST_INIT(>chunk_dir[i][j]);
}
-   STATS_ADD(d->malloc_used, regioninfo_size + 3 * MALLOC_PAGESIZE);
d->mmap_flag = mmap_flag;
d->malloc_junk = mopts.def_malloc_junk;
d->canary1 = mopts.malloc_canary ^ (u_int32_t)(uintptr_t)d;
d->canary2 = ~d->canary1;
-
-   *dp = d;
 }
 
 static int
@@ -551,7 +525,8 @@ omalloc_grow(struct dir_info *d)
if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2)
return 1;
 
-   newtotal = d->regions_total * 2;
+   newtotal = 

Re: netcat: bump BUFSIZE to 64k?

2022-12-18 Thread Claudio Jeker
On Sun, Dec 18, 2022 at 05:53:25PM +0100, Marco Pfatschbacher wrote:
> 
> On Sun, Dec 18, 2022 at 02:00:24PM +0100, Theo Buehler wrote:
> > This is the remaining bit of mpf's recent netcat diff. The commit log
> > shows that it was bumped to 64k in the past, but that was promptly
> > reverted due to concerns of buffer bloat caused by atomicio blocking
> > traffic in the other direction.
> 
> Thanks for taking care of this.
> I should've done some research myself before suggesting this.
> 
> For reference:
> 
> revision 1.121
> date: 2014/06/10 16:35:42;  author: tedu;  state: Exp;  lines: +2 -2;
> stick with 16k buffers for a little while to avoid bufferbloat.
> atomicio writing out 64k in one direction will cause traffic in the
> other direction to stall until it's complete. discussion with deraadt
> 
> revision 1.120
> date: 2014/06/10 16:23:07;  author: tedu;  state: Exp;  lines: +3 -3;
> increase buffer size to 64k, and actually use it. ok deraadt
> from John-Mark Gurney
> 
> > I don't know if things are different enough 8 years later that this can
> > be reconsidered. Not my area, just throwing it out there so it doesn't
> > get lost.
> 
> I can only assume that the concern was about talking to a server
> that would block while trying to send a response after only reading a part of
> the data.
> I can't think of a real world scenario where this is a problem right now.
> 
> But since my use case (sending a huge UDP packet) also depends on bumping
> the socket buffer size with -O, we could also make the BUFSIZE conditional on
> the provided socket buffer size, or give this a new option.
> 

The problem described  is that atomicio() stalls until all of 64k is written.
With a small socketbuffer size this requires the receiver to read the data
and so calling atomicio() with a big buffer can stall nc.

What confuses me is that atomicio() is not used in the main readwrite()
loop. There nc polls on both fds and then read/write depending on return
values.  atomicio() is only used by atelnet() and socks_connect() which do
not depend on BUFSIZE.

I assume this was modified when TLS support was added but I did
not investigate further.

-- 
:wq Claudio



Re: netcat: bump BUFSIZE to 64k?

2022-12-18 Thread Theo de Raadt
you say

"I can't think of a real world scenario where this is a problem right now."

Wait, has something changed?

I don't understand what you think has changed that undoes the justification
for tedu's backout.

Are there perhaps modes that should use larger buffer sizes, and modes that
should not, and can this be automatically determined inside nc?

Marco Pfatschbacher  wrote:

> 
> 
> On Sun, Dec 18, 2022 at 02:00:24PM +0100, Theo Buehler wrote:
> > This is the remaining bit of mpf's recent netcat diff. The commit log
> > shows that it was bumped to 64k in the past, but that was promptly
> > reverted due to concerns of buffer bloat caused by atomicio blocking
> > traffic in the other direction.
> 
> Thanks for taking care of this.
> I should've done some research myself before suggesting this.
> 
> For reference:
> 
> revision 1.121
> date: 2014/06/10 16:35:42;  author: tedu;  state: Exp;  lines: +2 -2;
> stick with 16k buffers for a little while to avoid bufferbloat.
> atomicio writing out 64k in one direction will cause traffic in the
> other direction to stall until it's complete. discussion with deraadt
> 
> revision 1.120
> date: 2014/06/10 16:23:07;  author: tedu;  state: Exp;  lines: +3 -3;
> increase buffer size to 64k, and actually use it. ok deraadt
> from John-Mark Gurney
> 
> > I don't know if things are different enough 8 years later that this can
> > be reconsidered. Not my area, just throwing it out there so it doesn't
> > get lost.
> 
> I can only assume that the concern was about talking to a server
> that would block while trying to send a response after only reading a part of
> the data.
> I can't think of a real world scenario where this is a problem right now.
> 
> But since my use case (sending a huge UDP packet) also depends on bumping
> the socket buffer size with -O, we could also make the BUFSIZE conditional on
> the provided socket buffer size, or give this a new option.
> 
> 



Re: netcat: bump BUFSIZE to 64k?

2022-12-18 Thread Marco Pfatschbacher


On Sun, Dec 18, 2022 at 02:00:24PM +0100, Theo Buehler wrote:
> This is the remaining bit of mpf's recent netcat diff. The commit log
> shows that it was bumped to 64k in the past, but that was promptly
> reverted due to concerns of buffer bloat caused by atomicio blocking
> traffic in the other direction.

Thanks for taking care of this.
I should've done some research myself before suggesting this.

For reference:

revision 1.121
date: 2014/06/10 16:35:42;  author: tedu;  state: Exp;  lines: +2 -2;
stick with 16k buffers for a little while to avoid bufferbloat.
atomicio writing out 64k in one direction will cause traffic in the
other direction to stall until it's complete. discussion with deraadt

revision 1.120
date: 2014/06/10 16:23:07;  author: tedu;  state: Exp;  lines: +3 -3;
increase buffer size to 64k, and actually use it. ok deraadt
from John-Mark Gurney

> I don't know if things are different enough 8 years later that this can
> be reconsidered. Not my area, just throwing it out there so it doesn't
> get lost.

I can only assume that the concern was about talking to a server
that would block while trying to send a response after only reading a part of
the data.
I can't think of a real world scenario where this is a problem right now.

But since my use case (sending a huge UDP packet) also depends on bumping
the socket buffer size with -O, we could also make the BUFSIZE conditional on
the provided socket buffer size, or give this a new option.




Re: sparc64: pull retry logic out of stickcmpr_set()

2022-12-18 Thread Mark Kettenis
> Date: Fri, 16 Dec 2022 16:28:03 -0600
> From: Scott Cheloha 
> 
> miod@'s UltraSPARC IIe machine really, really hates the following
> code:
> 
>   stickcmpr_set(stick());
> 
> When we try that code, invariably the clock interrupt does not fire
> and the machine hangs.
> 
> I think stickcmpr_set() fails to ensure that %STICK_CMPR is larger
> than %STICK before returning to the caller.  That, or miod@'s machine
> is weird.  In either case, we've been stuck for a few weeks trying to
> figure it out.
> 
> After some back and forth, we eventually tried pulling the retry logic
> out of stickcmpr_set() into a C function.  It worked!  His UltraSPARC
> IIe machine (probably a Sun Blade 100?) finally booted.  That patch is
> attached below.
> 
> If this change is accepted I intend to change tickcmpr_set() and
> sys_tickcmpr_set() in identical ways to keep the three clocks' code
> similar.
> 
> I am including this bit:
> 
> > @@ -920,9 +922,24 @@ stick_start(void)
> >  */
> > 
> > s = intr_disable();
> > -   ci->ci_tick = roundup(stick(), tick_increment);
> > -   stickcmpr_set(ci->ci_tick);
> > +   ci->ci_tick = stick();
> > +   stick_rearm(ci->ci_tick);
> 
> ... to demonstrate that the new logic actually works.  I suspect that
> the retry logic in stickcmpr_set() has a bug that has remained hidden
> by roundup() since it was committed in 2008.
> 
> kettenis: You wrote stickcmpr_set().  Am I nuts?  Can you spot a bug
> in in stickcmpr_set()?  I cannot, but OTOH I was born yesterday.

No, I can't spot the bug.  Unless reading STICK_REG_LOW has some of
the bits in the upper half of the 64-bit register set...

> miod: Can you confirm that this patch still works?  I tweaked it to
> match the existing assembly like you asked.
> 
> ok?

ok kettenis@

> Index: clock.c
> ===
> RCS file: /cvs/src/sys/arch/sparc64/sparc64/clock.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 clock.c
> --- clock.c   10 Nov 2022 07:08:01 -  1.72
> +++ clock.c   16 Dec 2022 22:25:06 -
> @@ -150,6 +150,8 @@ void  tick_start(void);
>  void sys_tick_start(void);
>  void stick_start(void);
>  
> +void stick_rearm(uint64_t);
> +
>  int  tickintr(void *);
>  int  sys_tickintr(void *);
>  int  stickintr(void *);
> @@ -810,7 +812,7 @@ stickintr(void *cap)
>  
>   /* Reset the interrupt. */
>   s = intr_disable();
> - stickcmpr_set(ci->ci_tick);
> + stick_rearm(ci->ci_tick);
>   intr_restore(s);
>  
>   return (1);
> @@ -920,9 +922,24 @@ stick_start(void)
>*/
>  
>   s = intr_disable();
> - ci->ci_tick = roundup(stick(), tick_increment);
> - stickcmpr_set(ci->ci_tick);
> + ci->ci_tick = stick();
> + stick_rearm(ci->ci_tick);
>   intr_restore(s);
> +}
> +
> +void
> +stick_rearm(uint64_t cmp)
> +{
> + uint64_t now, off = 8;
> +
> + stickcmpr_set(cmp);
> + now = stick();
> + while (cmp <= now) {
> + cmp += off;
> + stickcmpr_set(cmp);
> + now = stick();
> + off *= 2;
> + }
>  }
>  
>  u_int
> Index: locore.s
> ===
> RCS file: /cvs/src/sys/arch/sparc64/sparc64/locore.s,v
> retrieving revision 1.194
> diff -u -p -r1.194 locore.s
> --- locore.s  8 Dec 2022 01:25:45 -   1.194
> +++ locore.s  16 Dec 2022 22:25:07 -
> @@ -7568,26 +7568,10 @@ END(stick)
>  
>  ENTRY(stickcmpr_set)
>   setxSTICK_CMP_HIGH, %o1, %o3
> - mov 8, %o2  ! Initial step size
> -1:
>   srlx%o0, 32, %o1
>   stxa%o1, [%o3] ASI_PHYS_NON_CACHED
>   add %o3, (STICK_CMP_LOW - STICK_CMP_HIGH), %o4
>   stxa%o0, [%o4] ASI_PHYS_NON_CACHED
> -
> - add %o3, (STICK_REG_LOW - STICK_CMP_HIGH), %o4
> - ldxa[%o4] ASI_PHYS_NON_CACHED, %o1
> - add %o3, (STICK_REG_HIGH - STICK_CMP_HIGH), %o4
> - ldxa[%o4] ASI_PHYS_NON_CACHED, %o5
> - sllx%o5, 32, %o5
> - or  %o1, %o5, %o1
> -
> - cmp %o0, %o1! Make sure the value we wrote
> - bg,pt   %xcc, 2f!   was in the future
> -  add%o0, %o2, %o0   ! If not, add the step size, double
> - ba,pt   %xcc, 1b!   the step size and try again.
> -  sllx   %o2, 1, %o2
> -2:
>   retl
>nop
>  END(stickcmpr_set)
> 
> 



Re: acme-client: allow configuring key and cert owner

2022-12-18 Thread Lucas
Stuart Henderson  wrote:
> On 2022/12/18 03:06, Lucas wrote:
> > The following patch expands acme-client config file `domain` blocks to
> > allow for a `owner user:group` directive, which allows to get rid of
> > customs scripts that "fix" permissions for issued certs, mostly needed
> > in ports land. I don't find it too invasive, so I thought it could be
> > merged. Most of the code and manpage bits were taken from vmd.
> 
> Why do you need to chown a certificate? It is published to the world
> anyway in Certificate Transparency logs, what's the problem with
> root-owned and world-readable?

You're absolutely right. And even without CT, the certificate is public,
otherwise you won't be able to establish a TLS session. I guess the
obsessive in me wanted matching ownership for key and cert.

> (There would be more reason to do this for a key, but the existing
> handling seems good enough for that).

Yes, and that partly makes it even less of a case for this patch. There
is still the need to chown to the right user, and currently the handling
of that is manually by the operator. Arguably, currently that only
happens when issuing a certificate for the very first time, or on manual
secret key rotation.

Nevertheless, dropping the cert part is easy, with the upside of not
needing chown pledge anymore in the patch. It was a fun learning
experience. I'll stop pursuing the patch, but leave the final version
anyways.

-Lucas


diff /usr/src
commit - 93aad84f8cf14cfaff5b9cdb67494e561810ddc4
path + /usr/src
blob - eb5f19eb298c117c3957faa0ed6ced14972ffaca
file + usr.sbin/acme-client/acme-client.conf.5
--- usr.sbin/acme-client/acme-client.conf.5
+++ usr.sbin/acme-client/acme-client.conf.5
@@ -131,7 +131,7 @@ plain domain name forms.
 The common name is included automatically if this option is present,
 but there is no automatic conversion/inclusion between "www." and
 plain domain name forms.
-.It Ic domain key Ar file Op Ar keytype
+.It Ic domain key Ar file Oo Ar keytype Oc Oo Ic owner Ar user : Ns Ar group Oc
 The private key file for which the certificate will be obtained.
 .Ar keytype
 can be
@@ -146,6 +146,18 @@ or secp384r1 for
 .Cm rsa
 or secp384r1 for
 .Cm ecdsa ) .
+If
+.Cm owner
+is given, set the key owner to
+.Ar user
+and
+.Ar group .
+If only
+.Ar user
+is given, only the user is set.
+If only
+.Pf : Ar group
+is given, only the group is set.
 .It Ic domain certificate Ar file
 The filename of the certificate that will be issued.
 This is optional if
blob - 4b43b6ef4ac302706859bf14b681cf8052aa55c8
file + usr.sbin/acme-client/extern.h
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -209,7 +209,7 @@ int  keyproc(int, const char *, const char **, 
size_t
 int fileproc(int, const char *, const char *, const char *,
const char *);
 int keyproc(int, const char *, const char **, size_t,
-   enum keytype);
+   enum keytype, uid_t, gid_t);
 int netproc(int, int, int, int, int, int, int,
struct authority_c *, const char *const *,
size_t);
blob - a3bc2796c30b97c5628e5f3d350a765c87cb
file + usr.sbin/acme-client/keyproc.c
--- usr.sbin/acme-client/keyproc.c
+++ usr.sbin/acme-client/keyproc.c
@@ -75,7 +75,7 @@ keyproc(int netsock, const char *keyfile, const char *
  */
 int
 keyproc(int netsock, const char *keyfile, const char **alts, size_t altsz,
-enum keytype keytype)
+enum keytype keytype, uid_t uid, gid_t gid)
 {
char*der64 = NULL, *der = NULL, *dercp;
char*sans = NULL, *san = NULL;
@@ -97,7 +97,18 @@ keyproc(int netsock, const char *keyfile, const char *
 
prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO);
if ((f = fopen(keyfile, "r")) == NULL && errno == ENOENT) {
+   int fd;
+
f = fopen(keyfile, "wx");
+   if (f != NULL) {
+   fd = fileno(f);
+   if (fd >= 0) {
+   if (fchown(fd, uid, gid) == -1) {
+   warn("chown %s", keyfile);
+   goto out;
+   }
+   }
+   }
newkey = 1;
}
umask(prev);
blob - bec17254297fb1e770411c1cb8ddb718e150ee0f
file + usr.sbin/acme-client/main.c
--- usr.sbin/acme-client/main.c
+++ usr.sbin/acme-client/main.c
@@ -248,7 +248,7 @@ main(int argc, char *argv[])
close(file_fds[1]);
c = keyproc(key_fds[0], domain->key,
(const char **)alts, altsz,
-   domain->keytype);
+   domain->keytype, domain->owner.uid, domain->owner.gid);
exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
}
 
blob - 3954f62a0d0894d8fd243944e8393c2e9dc2e70e
file + usr.sbin/acme-client/parse.h
--- 

Re: [Patch] Probable error in sh.1

2022-12-18 Thread Jason McIntyre
On Sun, Dec 18, 2022 at 12:56:59PM +, Klemens Nanni wrote:
> 12/18/22 15:53, Stefan Hagen ??:
> > Ross L Richardson wrote (2022-12-18 10:55 CET):
> >> The word "array" occurs only once in sh.1.  Therefore, either it deserves
> >> more explanation, or removal with something like the patch below.
> > 
> > I think you're right. This looks like an oversight from the works when
> > sh(1) was rewritten / split from ksh(1) in 2015.
> > 
> > OK sdk@
> 
> Agreed, I'd say go ahead.
> 

ok jmc

> > 
> >> Ross
> >> ==
> >>
> >> --- sh.1.orig  Thu Sep  1 10:07:22 2022
> >> +++ sh.1   Sun Dec 18 20:47:53 2022
> >> @@ -1390,7 +1390,7 @@
> >>   .Pp
> >>   Where
> >>   .Ar expression
> >> -is an integer, parameter name, or array reference,
> >> +is an integer or parameter name,
> >>   optionally combined with any of the operators described below,
> >>   listed and grouped according to precedence:
> >>   .Bl -tag -width Ds
> >>
> > 
> 



netcat: bump BUFSIZE to 64k?

2022-12-18 Thread Theo Buehler
This is the remaining bit of mpf's recent netcat diff. The commit log
shows that it was bumped to 64k in the past, but that was promptly
reverted due to concerns of buffer bloat caused by atomicio blocking
traffic in the other direction.

I don't know if things are different enough 8 years later that this can
be reconsidered. Not my area, just throwing it out there so it doesn't
get lost.

Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.224
diff -u -p -r1.224 netcat.c
--- netcat.c18 Dec 2022 12:53:18 -  1.224
+++ netcat.c18 Dec 2022 12:54:58 -
@@ -66,7 +66,7 @@
 #define POLL_NETOUT1
 #define POLL_NETIN 2
 #define POLL_STDOUT3
-#define BUFSIZE16384
+#define BUFSIZE65536
 
 #define TLS_NOVERIFY   (1 << 1)
 #define TLS_NONAME (1 << 2)



Re: [Patch] Probable error in sh.1

2022-12-18 Thread Klemens Nanni

12/18/22 15:53, Stefan Hagen пишет:

Ross L Richardson wrote (2022-12-18 10:55 CET):

The word "array" occurs only once in sh.1.  Therefore, either it deserves
more explanation, or removal with something like the patch below.


I think you're right. This looks like an oversight from the works when
sh(1) was rewritten / split from ksh(1) in 2015.

OK sdk@


Agreed, I'd say go ahead.




Ross
==

--- sh.1.orig   Thu Sep  1 10:07:22 2022
+++ sh.1Sun Dec 18 20:47:53 2022
@@ -1390,7 +1390,7 @@
  .Pp
  Where
  .Ar expression
-is an integer, parameter name, or array reference,
+is an integer or parameter name,
  optionally combined with any of the operators described below,
  listed and grouped according to precedence:
  .Bl -tag -width Ds







Re: acme-client: allow configuring key and cert owner

2022-12-18 Thread Stuart Henderson
On 2022/12/18 03:06, Lucas wrote:
> The following patch expands acme-client config file `domain` blocks to
> allow for a `owner user:group` directive, which allows to get rid of
> customs scripts that "fix" permissions for issued certs, mostly needed
> in ports land. I don't find it too invasive, so I thought it could be
> merged. Most of the code and manpage bits were taken from vmd.

Why do you need to chown a certificate? It is published to the world
anyway in Certificate Transparency logs, what's the problem with
root-owned and world-readable?

(There would be more reason to do this for a key, but the existing
handling seems good enough for that).



Re: acme-client: allow configuring key and cert owner

2022-12-18 Thread Lucas
Lucas  wrote:
> Hi tech@,
> 
> The following patch expands acme-client config file `domain` blocks to
> allow for a `owner user:group` directive, which allows to get rid of
> customs scripts that "fix" permissions for issued certs, mostly needed
> in ports land. I don't find it too invasive, so I thought it could be
> merged. Most of the code and manpage bits were taken from vmd.
> 
> Noteworthy details:
> 
> 1. fileproc.c pledge is expanded with chown. In particular, I don't
>understand pledge manpage: I was under the impression that wpath and
>fattr would allow for fchown, but I got an "Operation not supported"
>while testing. I guess this is related to the paragraph stating that
>some syscalls be allowed with limitations under some categories.
> 2. if the private key already exists, keyproc.c won't interfere with its
>ownership, the same way it does now. The fchown call can be moved if
>"fixing" the ownership is desirable.

After checking chown(2), -1 seems more sensible as the default values
for [gu]id, instead of 0.

-Lucas


diff /usr/src
commit - 93aad84f8cf14cfaff5b9cdb67494e561810ddc4
path + /usr/src
blob - eb5f19eb298c117c3957faa0ed6ced14972ffaca
file + usr.sbin/acme-client/acme-client.conf.5
--- usr.sbin/acme-client/acme-client.conf.5
+++ usr.sbin/acme-client/acme-client.conf.5
@@ -197,6 +197,17 @@ will be used.
 If it is not specified, a default of
 .Pa /var/www/acme
 will be used.
+.It Ic owner Ar user : Ns Ar group
+Set the owner of the key and certificate files create to
+.Ar user
+and
+.Ar group .
+If only
+.Ar user
+is given, only the user is set.
+If only
+.Pf : Ar group
+is given, only the group is set.
 .El
 .Sh FILES
 .Bl -tag -width /etc/examples/acme-client.conf -compact
blob - 4b43b6ef4ac302706859bf14b681cf8052aa55c8
file + usr.sbin/acme-client/extern.h
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -207,9 +207,9 @@ int  fileproc(int, const char *, const char *, 
const 
 int revokeproc(int, const char *, int, int, const char *const *,
size_t);
 int fileproc(int, const char *, const char *, const char *,
-   const char *);
+   const char *, uid_t, gid_t);
 int keyproc(int, const char *, const char **, size_t,
-   enum keytype);
+   enum keytype, uid_t, gid_t);
 int netproc(int, int, int, int, int, int, int,
struct authority_c *, const char *const *,
size_t);
blob - b8b8651c31907c6d37147c779f302481a1cb3c86
file + usr.sbin/acme-client/fileproc.c
--- usr.sbin/acme-client/fileproc.c
+++ usr.sbin/acme-client/fileproc.c
@@ -29,7 +29,8 @@ serialise(const char *real, const char *v, size_t vsz,
 #include "extern.h"
 
 static int
-serialise(const char *real, const char *v, size_t vsz, const char *v2, size_t 
v2sz)
+serialise(const char *real, const char *v, size_t vsz, const char *v2,
+size_t v2sz, uid_t uid, gid_t gid)
 {
int   fd;
char *tmp;
@@ -64,6 +65,10 @@ serialise(const char *real, const char *v, size_t vsz,
warn("fchmod");
goto out;
}
+   if (fchown(fd, uid, gid) == -1) {
+   warn("fchown");
+   goto out;
+   }
if ((ssize_t)vsz != write(fd, v, vsz)) {
warnx("write");
goto out;
@@ -91,7 +96,7 @@ fileproc(int certsock, const char *certdir, const char
 
 int
 fileproc(int certsock, const char *certdir, const char *certfile, const char
-*chainfile, const char *fullchainfile)
+*chainfile, const char *fullchainfile, uid_t uid, gid_t gid)
 {
char*csr = NULL, *ch = NULL;
size_t   chsz, csz;
@@ -108,7 +113,7 @@ fileproc(int certsock, const char *certdir, const char
 * rpath and cpath for rename, wpath and cpath for
 * writing to the temporary. fattr for fchmod.
 */
-   if (pledge("stdio cpath wpath rpath fattr", NULL) == -1) {
+   if (pledge("stdio cpath wpath rpath fattr chown", NULL) == -1) {
warn("pledge");
goto out;
}
@@ -173,7 +178,7 @@ fileproc(int certsock, const char *certdir, const char
goto out;
 
if (chainfile) {
-   if (!serialise(chainfile, ch, chsz, NULL, 0))
+   if (!serialise(chainfile, ch, chsz, NULL, 0, uid, gid))
goto out;
 
dodbg("%s: created", chainfile);
@@ -190,7 +195,7 @@ fileproc(int certsock, const char *certdir, const char
goto out;
 
if (certfile) {
-   if (!serialise(certfile, csr, csz, NULL, 0))
+   if (!serialise(certfile, csr, csz, NULL, 0, uid, gid))
goto out;
 
dodbg("%s: created", certfile);
@@ -204,7 +209,7 @@ fileproc(int certsock, const char *certdir, const char
 */
   

Re: [Patch] Probable error in sh.1

2022-12-18 Thread Stefan Hagen
Ross L Richardson wrote (2022-12-18 10:55 CET):
> The word "array" occurs only once in sh.1.  Therefore, either it deserves
> more explanation, or removal with something like the patch below.

I think you're right. This looks like an oversight from the works when 
sh(1) was rewritten / split from ksh(1) in 2015.

OK sdk@

> Ross
> ==
> 
> --- sh.1.orig Thu Sep  1 10:07:22 2022
> +++ sh.1  Sun Dec 18 20:47:53 2022
> @@ -1390,7 +1390,7 @@
>  .Pp
>  Where
>  .Ar expression
> -is an integer, parameter name, or array reference,
> +is an integer or parameter name,
>  optionally combined with any of the operators described below,
>  listed and grouped according to precedence:
>  .Bl -tag -width Ds
> 



Re: add 2 transmeta devices to pcidevs

2022-12-18 Thread Jonathan Gray
On Sun, Dec 11, 2022 at 01:57:13PM -0500, Daniel Dickman wrote:
> I have a laptop with these Transmeta devices:
> 
> pchb0 at pci0 dev 0 function 0 vendor "Transmeta", unknown product 0x0060 rev 
> 0x00
> ppb0 at pci0 dev 1 function 0 vendor "Transmeta", unknown product 0x0061 rev 
> 0x00
> 
> NetBSD describes device 0061 as the integrated North Bridge, but I think 
> this is incorrect as this is actually an AGP bridge:
> 
> product TRANSMETA TM8000NB0x0061  TM8000 Integrated North Bridge
> 
> The PCI repository has these entries, which I feel is more correct than 
> NetBSD:
> 
> 0060  TM8000 Northbridge  
> 0061  TM8000 AGP bridge
> 
> However, reading the "Efficeon BIOS Programmers Guide", I chose to 
> describe device as 0060 as HyperTransport instead.
> 
> The Efficeon processor has a virtual north bridge that can communicate 
> with the south bridge over HyperTransport and can communicate with the 
> graphics controller over an AGP bridge. For reference, see Chapter 2, 
> Figure 1 showing the system block diagram.
> 
> Some contemporaneous news coverage on HyperTransport is available here for 
> reference: 
> https://www.eetimes.com/transmeta-links-next-generation-processor-to-hypertransport/
> 
> ok?

ok jsg@

> 
> Index: pcidevs
> ===
> RCS file: /cvs/src/sys/dev/pci/pcidevs,v
> retrieving revision 1.2014
> diff -u -p -u -r1.2014 pcidevs
> --- pcidevs   4 Dec 2022 03:13:52 -   1.2014
> +++ pcidevs   11 Dec 2022 18:51:04 -
> @@ -9030,6 +9030,8 @@ product TOSHIBA2 TFIRO  0x0701  Infrared
>  product TOSHIBA2 SDCARD  0x0805  SD
>  
>  /* Transmeta products */
> +product TRANSMETA TM8000_HT  0x0060  TM8000 HyperTransport
> +product TRANSMETA TM8000_AGP 0x0061  TM8000 AGP
>  product TRANSMETA NB 0x0295  Northbridge
>  product TRANSMETA LONGRUN_NB 0x0395  LongRun Northbridge
>  product TRANSMETA SDRAM  0x0396  SDRAM
> 
> 



[Patch] Probable error in sh.1

2022-12-18 Thread Ross L Richardson
The word "array" occurs only once in sh.1.  Therefore, either it deserves
more explanation, or removal with something like the patch below.

Ross
==

--- sh.1.orig   Thu Sep  1 10:07:22 2022
+++ sh.1Sun Dec 18 20:47:53 2022
@@ -1390,7 +1390,7 @@
 .Pp
 Where
 .Ar expression
-is an integer, parameter name, or array reference,
+is an integer or parameter name,
 optionally combined with any of the operators described below,
 listed and grouped according to precedence:
 .Bl -tag -width Ds



Re: Support Wacom One M (CTL-672) [Was: Support Wacom One S (CTL-472)]

2022-12-18 Thread Stefan Hagen
Sven M. Hallberg wrote (2022-12-08 14:12 CET):
> Marcus Glocker on Sat, Sep 03 2022:
> > I have an Wacom One CTL-672, never used it on OpenBSD.
> 
> This is the "Wacom One M", which I also own...
> 
> > Currently it attaches to ums(4). Works fine with that.
> 
> It seems to expose two HIDs, one which reports as a regular "mouse" and
> makes it work like a touchpad (relative mode), and a second one (with a
> nonsense report descriptor) that can be used for absolute positioning.
> 
> > It also works fine when attaching to uwacom(4), without and with your
> > diff.  It doesn't seem to require specific 'tsscale' nor
> > 'loc_tip_press' settings.
> 
> This does not match my experience; the second device (which uwacom
> attaches to) did not produce any events. It appears it needs the call to
> uhidev_set_report() of the "One S" code path to switch on.
> 
> Trivial patch below that made it produce events for me.
> 
> I did have to fiddle with xinput(1) to get the scale right. I ended up
> putting the following in its InputDevice section in /etc/X11/xorg.conf:
> 
> Option  "TransformationMatrix" "0.09 0 0 0 0.08 0 0 0 1"
> 
> I wonder what the correct way is to avoid having to do this. Is it those
> tsscale parameters? If so, what's the best way to determine the correct
> values?

I don't think the scale values can be read from the device.
But you can get them from the linux wacom driver here:
https://github.com/linuxwacom/input-wacom/blob/master/4.5/wacom_wac.c#L11

Untested patch below.

Best Regards,
Stefan

diff --git a/sys/dev/usb/uwacom.c b/sys/dev/usb/uwacom.c
index f9af276a641..2c4e51b7522 100644
--- a/sys/dev/usb/uwacom.c
+++ b/sys/dev/usb/uwacom.c
@@ -149,6 +149,11 @@ uwacom_attach(struct device *parent, struct device *self, 
void *aux)
ms->sc_tsscale.maxy = 9500;
}
 
+   if (uha->uaa->product == USB_PRODUCT_WACOM_ONE_M) {
+   ms->sc_tsscale.maxx = 21600;
+   ms->sc_tsscale.maxy = 13500;
+   }
+
if (uha->uaa->product == USB_PRODUCT_WACOM_INTUOS_DRAW) {
sc->sc_flags = UWACOM_USE_PRESSURE | UWACOM_BIG_ENDIAN;
sc->sc_loc_tip_press.pos = 43;