rename pf_state_key and pf_state_item members
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, &pf_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, &cur->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, &cur->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(&pf_state_item_pl, PR_NOWAIT)) == NULL) { - if (TAILQ_EMPTY(&sk->states)) { + if (TAILQ_EMPTY(&sk->sk_states)) { KASSERT(cur == NULL); RB_REMOVE(pf_state_tree, &pf_statetbl, sk); - sk->removed = 1; + sk->sk_removed = 1; pf_state_key_unref(sk); }
Re: netcat: bump BUFSIZE to 64k?
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?
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
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(&d->chunk_info_list[i]); for (j = 0; j < MALLOC_CHUNK_LISTS; j++) LIST_INIT(&d->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 = d->regions
Re: netcat: bump BUFSIZE to 64k?
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?
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?
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()
> 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
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 --- usr.sbin/acme-client
Re: [Patch] Probable error in sh.1
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?
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
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
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
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
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
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
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)]
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;