Re: CVS: cvs.openbsd.org: src
On Thu, Jun 01, 2023 at 12:57:54PM -0600, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/06/01 12:57:54 > > Modified files: > sys/net: if_wg.c if_wg.h > sbin/ifconfig : ifconfig.8 ifconfig.c > > Log message: > Add support for wireguard peer descriptions > > "wgdescr[iption] foo" to label one peer (amongst many) on a wg(4) interface, > "-wgdescr[iption]" or "wgdescr ''" to remove the label, completely analogous > to existing interface discriptions. > > Idea/initial diff from Mikolaj Kucharski (OK sthen) > Tests/prodded by Hrvoje Popovski > Tweaks/manual bits from me > Feedback deraadt sthen mvs claudio > OK claudio > I would like to point out, that initial version was written by Noah Meier https://marc.info/?l=openbsd-tech=163478285129091=2 -- Regards, Mikolaj
Re: CVS: cvs.openbsd.org: src
On 2023/06/01 03:47, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2023/06/01 03:47:35 > > Modified files: > usr.sbin/bgpd : kroute.c > > Log message: > Check the F_NEXTHOP flag on the right kroute6 object. > > On multipath routes the check ended up checking the wrong route for the > nexthop update. This resulted in a use-after-free in kroute_detach_nexthop(). > This only affects IPv6 in the IPv4 code path the right object was already > used. > > Thanks to sthen@ for providing the debug information to track this down. > OK sthen@ tb@ > I think this is one where we can definitely say "found the hard way", thank you Claudio for staring at the code..
Re: CVS: cvs.openbsd.org: src
On 28.4.2023. 17:50, Alexandr Nedvedicky wrote: > CVSROOT: /cvs > Module name: src > Changes by: sas...@cvs.openbsd.org 2023/04/28 09:50:05 > > Modified files: > sys/net: if_pfsync.c > > Log message: > remove superfluous/invalid KASSERT() in pfsync_q_del(). > > pointed and OK bluhm@ > > I thought that this diff would repair this panic r620-2# uvm_fault(0x825c8000, 0x17, 0, 2) -> e kernel: page fault trap, code=2 Stopped at pfsync_q_del+0x8d: movq%rdx,0x8(%rax) TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *461341 27834 0 0x14000 0x2005K softnet pfsync_q_del(fd832ba5d320) at pfsync_q_del+0x8d pf_remove_state(fd832ba5d320) at pf_remove_state+0x156 pfsync_in_del_c(fd8002ce6738,c,79,0) at pfsync_in_del_c+0x6f pfsync_input(800022d60a98,800022d60aa4,f0,2) at pfsync_input+0x34c ip_deliver(800022d60a98,800022d60aa4,f0,2) at ip_deliver+0x113 ipintr() at ipintr+0x69 if_netisr(0) at if_netisr+0xe0 taskq_thread(8003) at taskq_thread+0x100 end trace frame: 0x0, count: 7 https://www.openbsd.org/ddb.html describes the minimum info required in bug reports. Insufficient info makes it difficult to find and fix bugs. but it didn't :)
Re: CVS: cvs.openbsd.org: src
Or as it should have said: "Make 'bsd' before 'mr.fs' so that 'mr.fs' can use the actual space available in the kernel image rather than relying on third parties like disktab to be correct. No intentional functional change." Sigh. Kenneth R Westerback writes: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/04/28 02:45:26 > > Modified files: > distrib/alpha/miniroot: Makefile > distrib/amd64/ramdiskA: Makefile > distrib/amd64/ramdisk_cd: Makefile > distrib/arm64/ramdisk: Makefile > distrib/armv7/ramdisk: Makefile > distrib/hppa/ramdisk: Makefile > distrib/i386/ramdisk: Makefile > distrib/i386/ramdisk_cd: Makefile > distrib/landisk/ramdisk: Makefile > distrib/loongson/ramdisk: Makefile > distrib/luna88k/ramdisk: Makefile > distrib/macppc/ramdisk: Makefile > distrib/octeon/ramdisk: Makefile > distrib/powerpc64/ramdisk: Makefile > distrib/riscv64/ramdisk: Makefile > distrib/sparc64/miniroot: Makefile > distrib/sparc64/ramdisk: Makefile > distrib/sparc64/ramdiskB: Makefile > sys/arch/octeon/stand/boot: Makefile > sys/arch/powerpc64/stand/boot: Makefile > > Log message: > Retire -E's "expert" mode. Introduced 23 years ago to avoid > confusing users with FFS attributes that only experts should > fiddle with. Actual use has withered away with functionality > rendered moot or moved elsewhere. > > '-e' remains for the truly obscure corner cases. > > Simply excise the code for now to see if hidden users/uses are > exposed. Further simplifications are possible if no such > users/uses surface. > > ok with sthen@ millert@ kn@ otto@ -- Ken
Re: CVS: cvs.openbsd.org: src
Sigh. Greg Steuck wrote: > "Theo de Raadt" writes: > > > Can you dig deeper into why? > > The easiest reproducer is to compile a file with a single > #include > > > Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? > > Neither of them seems to have the magic define, unlike the 13 > unobviously chosen places in /usr/include which have: > > #ifndef _SIZE_T_DEFINED_ > #define _SIZE_T_DEFINED_ > typedef __size_t size_t; > #endif > > Should I replicate this block into sys/syslog.h? That fixes the problem > but clearly one can never tell what's gonna break with system include > files. > > Thanks > Greg
Re: CVS: cvs.openbsd.org: src
"Todd C. Miller" writes: >> Should I replicate this block into sys/syslog.h? That fixes the >> problem >> but clearly one can never tell what's gonna break with system >> include >> files. > > That should be OK but you could also just use __size_t in the > function prototype. We use __va_list in syslog.h instead of va_list > for a similar reason. I'm running this through make build just in case, but it seems to be fixing the original problem. OK? diff --git a/sys/sys/syslog.h b/sys/sys/syslog.h index 063105952b1..c5f24339ecc 100644 --- a/sys/sys/syslog.h +++ b/sys/sys/syslog.h @@ -204,7 +204,7 @@ int setlogmask_r(int, struct syslog_data *); void syslog_r(int, struct syslog_data *, const char *, ...) __attribute__((__format__(__syslog__,3,4))); void vsyslog_r(int, struct syslog_data *, const char *, __va_list); -intsendsyslog(const char *, size_t, int); +intsendsyslog(const char *, __size_t, int); __END_DECLS #else /* !_KERNEL */
Re: CVS: cvs.openbsd.org: src
On Thu, 27 Apr 2023 15:40:10 +0100, Greg Steuck wrote: > Neither of them seems to have the magic define, unlike the 13 > unobviously chosen places in /usr/include which have: > > #ifndef _SIZE_T_DEFINED_ > #define _SIZE_T_DEFINED_ > typedef __size_t size_t; > #endif > > Should I replicate this block into sys/syslog.h? That fixes the problem > but clearly one can never tell what's gonna break with system include > files. That should be OK but you could also just use __size_t in the function prototype. We use __va_list in syslog.h instead of va_list for a similar reason. - todd
Re: CVS: cvs.openbsd.org: src
"Theo de Raadt" writes: > Can you dig deeper into why? The easiest reproducer is to compile a file with a single #include > Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? Neither of them seems to have the magic define, unlike the 13 unobviously chosen places in /usr/include which have: #ifndef _SIZE_T_DEFINED_ #define _SIZE_T_DEFINED_ typedef __size_tsize_t; #endif Should I replicate this block into sys/syslog.h? That fixes the problem but clearly one can never tell what's gonna break with system include files. Thanks Greg
Re: CVS: cvs.openbsd.org: src
Can you dig deeper into why? Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? Antoine Jacoutot wrote: > On Wed, Apr 26, 2023 at 12:52:45AM -0600, Greg Steuck wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: gne...@cvs.openbsd.org 2023/04/26 00:52:45 > > > > Modified files: > > lib/libc/hidden: syslog.h > > sys/sys: syslog.h > > > > Log message: > > Declare sendsyslog to match its man page > > > > This simplifies syzkaller revival after the removal of __syscall. > > > > OK bluhm, millert, deraadt > > This broke (at least): > > www/e2guardian > www/dansguardian > > /usr/include/syslog.h:207:30: error: unknown type name 'size_t'; did you mean > '__size_t'? > > > -- > Antoine >
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 12:52:45AM -0600, Greg Steuck wrote: > CVSROOT: /cvs > Module name: src > Changes by: gne...@cvs.openbsd.org 2023/04/26 00:52:45 > > Modified files: > lib/libc/hidden: syslog.h > sys/sys: syslog.h > > Log message: > Declare sendsyslog to match its man page > > This simplifies syzkaller revival after the removal of __syscall. > > OK bluhm, millert, deraadt This broke (at least): www/e2guardian www/dansguardian /usr/include/syslog.h:207:30: error: unknown type name 'size_t'; did you mean '__size_t'? -- Antoine
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 11:39:20AM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2023/04/26 11:39:20 > > Modified files: > usr.sbin/bgpctl: json.c > > Log message: > Whitespace > > "please fix" json > json == claudio
Re: CVS: cvs.openbsd.org: src
Jonathan Gray writes: > On Wed, Apr 26, 2023 at 07:39:38AM +0200, Anton Lindqvist wrote: >> On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: >> > CVSROOT: /cvs >> > Module name: src >> > Changes by:d...@cvs.openbsd.org2023/04/24 10:53:57 >> > >> > Modified files: >> >sys/arch/amd64/amd64: vmm.c >> > >> > Log message: >> > vmm(4): allow guests to enable and use supervisor IBT. >> > >> > Why should hosts have all the fun? Conditionally unmask the cpuid >> > bits for IBT and allow r/w access to the supervisor CET msr. >> > >> > Will need revisiting when we introduce usage of userland CET msr. >> > >> > ok marlkin@ >> >> Running on older hardware where CET is not enumerated causes guests to >> panic in cpu_fix_msrs() during RDMSR 0x6a2. > > vcpu_reset_regs_vmx() also has: > > if (rcr4() | CR4_CET) > vmx_setmsrbrw(vcpu, MSR_S_CET); > Thanks!
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 07:39:38AM +0200, Anton Lindqvist wrote: > On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: d...@cvs.openbsd.org2023/04/24 10:53:57 > > > > Modified files: > > sys/arch/amd64/amd64: vmm.c > > > > Log message: > > vmm(4): allow guests to enable and use supervisor IBT. > > > > Why should hosts have all the fun? Conditionally unmask the cpuid > > bits for IBT and allow r/w access to the supervisor CET msr. > > > > Will need revisiting when we introduce usage of userland CET msr. > > > > ok marlkin@ > > Running on older hardware where CET is not enumerated causes guests to > panic in cpu_fix_msrs() during RDMSR 0x6a2. vcpu_reset_regs_vmx() also has: if (rcr4() | CR4_CET) vmx_setmsrbrw(vcpu, MSR_S_CET); > > diff --git sys/arch/amd64/amd64/vmm.c sys/arch/amd64/amd64/vmm.c > index 42ac8007029..35b05033cdc 100644 > --- sys/arch/amd64/amd64/vmm.c > +++ sys/arch/amd64/amd64/vmm.c > @@ -7059,7 +7059,7 @@ vmm_handle_cpuid(struct vcpu *vcpu) > *rcx &= ~SEFF0ECX_PKU; > > /* Expose IBT bit if we've enabled CET on the host. */ > - if (rcr4() | CR4_CET) > + if (rcr4() & CR4_CET) > *rdx |= SEFF0EDX_IBT; > else > *rdx &= ~SEFF0EDX_IBT; > >
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2023/04/24 10:53:57 > > Modified files: > sys/arch/amd64/amd64: vmm.c > > Log message: > vmm(4): allow guests to enable and use supervisor IBT. > > Why should hosts have all the fun? Conditionally unmask the cpuid > bits for IBT and allow r/w access to the supervisor CET msr. > > Will need revisiting when we introduce usage of userland CET msr. > > ok marlkin@ Running on older hardware where CET is not enumerated causes guests to panic in cpu_fix_msrs() during RDMSR 0x6a2. diff --git sys/arch/amd64/amd64/vmm.c sys/arch/amd64/amd64/vmm.c index 42ac8007029..35b05033cdc 100644 --- sys/arch/amd64/amd64/vmm.c +++ sys/arch/amd64/amd64/vmm.c @@ -7059,7 +7059,7 @@ vmm_handle_cpuid(struct vcpu *vcpu) *rcx &= ~SEFF0ECX_PKU; /* Expose IBT bit if we've enabled CET on the host. */ - if (rcr4() | CR4_CET) + if (rcr4() & CR4_CET) *rdx |= SEFF0EDX_IBT; else *rdx &= ~SEFF0EDX_IBT;
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 24, 2023 at 10:55:06AM -0600, Theo Buehler wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: t...@cvs.openbsd.org2023/04/24 10:55:06
>
> Modified files:
> lib/libssl : ssl_tlsext.c
>
> Log message:
> Free and calloc() the tlsext_build_order and remember its length
>
> Aligns tlsext_randomize_build_order() with tlsext_linearize_build_order()
> and will help regression testing.
>
> ok jsing
Does not compile without the following:
Index: ssl_local.h
===
RCS file: /cvs/src/lib/libssl/ssl_local.h,v
retrieving revision 1.4
diff -u -p -r1.4 ssl_local.h
--- ssl_local.h 23 Apr 2023 18:51:53 - 1.4
+++ ssl_local.h 25 Apr 2023 05:13:01 -
@@ -975,6 +975,7 @@ struct ssl_st {
unsigned int max_send_fragment;
const struct tls_extension **tlsext_build_order;
+ size_t tlsext_build_order_len;
char *tlsext_hostname;
/* certificate status request info */
Re: CVS: cvs.openbsd.org: src
On Mon, Mar 27, 2023 at 08:02:20AM +0200, Anton Lindqvist wrote: > On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > > > Modified files: > > distrib/miniroot: install.sub > > > > Log message: > > simplify final MAKEDEV call > > > > No need to loop here, the script takes multiple args. > > > > OK tb afresh1 > > Seen in install log on my regress machines during autoinstall. Is this > commit the culprit? Yes. MAKEDEV doesn't like repeated args and makes noise if it sees them. If you try to create an already existing device with a single argument it is silent. Odd. I reverted the commit.
Re: CVS: cvs.openbsd.org: src
It looks like the diff wasn't tested. Back it out. Anton Lindqvist wrote: > On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > > > Modified files: > > distrib/miniroot: install.sub > > > > Log message: > > simplify final MAKEDEV call > > > > No need to loop here, the script takes multiple args. > > > > OK tb afresh1 > > Seen in install log on my regress machines during autoinstall. Is this > commit the culprit? > > Making all device nodes...mknod: sd0a: File exists > mknod: rsd0a: File exists > mknod: sd0b: File exists > mknod: rsd0b: File exists > mknod: sd0c: File exists > mknod: rsd0c: File exists > mknod: sd0d: File exists > mknod: rsd0d: File exists > mknod: sd0e: File exists > mknod: rsd0e: File exists > mknod: sd0f: File exists > mknod: rsd0f: File exists > mknod: sd0g: File exists > mknod: rsd0g: File exists > mknod: sd0h: File exists > mknod: rsd0h: File exists > mknod: sd0i: File exists > mknod: rsd0i: File exists > mknod: sd0j: File exists > mknod: rsd0j: File exists > mknod: sd0k: File exists > mknod: rsd0k: File exists > mknod: sd0l: File exists > mknod: rsd0l: File exists > mknod: sd0m: File exists > mknod: rsd0m: File exists > mknod: sd0n: File exists > mknod: rsd0n: File exists > mknod: sd0o: File exists > mknod: rsd0o: File exists > mknod: sd0p: File exists > mknod: rsd0p: File exists > mknod: cd0a: File exists > mknod: rcd0a: File exists > mknod: cd0c: File exists > mknod: rcd0c: File exists > done. >
Re: CVS: cvs.openbsd.org: src
On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > Modified files: > distrib/miniroot: install.sub > > Log message: > simplify final MAKEDEV call > > No need to loop here, the script takes multiple args. > > OK tb afresh1 Seen in install log on my regress machines during autoinstall. Is this commit the culprit? Making all device nodes...mknod: sd0a: File exists mknod: rsd0a: File exists mknod: sd0b: File exists mknod: rsd0b: File exists mknod: sd0c: File exists mknod: rsd0c: File exists mknod: sd0d: File exists mknod: rsd0d: File exists mknod: sd0e: File exists mknod: rsd0e: File exists mknod: sd0f: File exists mknod: rsd0f: File exists mknod: sd0g: File exists mknod: rsd0g: File exists mknod: sd0h: File exists mknod: rsd0h: File exists mknod: sd0i: File exists mknod: rsd0i: File exists mknod: sd0j: File exists mknod: rsd0j: File exists mknod: sd0k: File exists mknod: rsd0k: File exists mknod: sd0l: File exists mknod: rsd0l: File exists mknod: sd0m: File exists mknod: rsd0m: File exists mknod: sd0n: File exists mknod: rsd0n: File exists mknod: sd0o: File exists mknod: rsd0o: File exists mknod: sd0p: File exists mknod: rsd0p: File exists mknod: cd0a: File exists mknod: rcd0a: File exists mknod: cd0c: File exists mknod: rcd0c: File exists done.
Re: CVS: cvs.openbsd.org: src
copied out.
Re: CVS: cvs.openbsd.org: src
On 2/21/2023 4:38 PM, Jonathan Gray wrote: > On Tue, Feb 21, 2023 at 06:42:59AM -0700, Brian Callahan wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: bcal...@cvs.openbsd.org 2023/02/21 06:42:59 >> >> Modified files: >> sys/dev/pci: azalia.c >> >> Log message: >> Attach Apollo Lake HD Audio device, enabling audio on machines with it. >> ok jsg@ phessler@ > > It already attached on other Apollo Lake machines, matching with > the HD Audio pci subclass. Your machine (and likely others) has a > different subclass. > Thanks for the clarification. I've never encountered another Apollo Lake machine. ~Brian
Re: CVS: cvs.openbsd.org: src
On Tue, Feb 21, 2023 at 06:42:59AM -0700, Brian Callahan wrote: > CVSROOT: /cvs > Module name: src > Changes by: bcal...@cvs.openbsd.org 2023/02/21 06:42:59 > > Modified files: > sys/dev/pci: azalia.c > > Log message: > Attach Apollo Lake HD Audio device, enabling audio on machines with it. > ok jsg@ phessler@ It already attached on other Apollo Lake machines, matching with the HD Audio pci subclass. Your machine (and likely others) has a different subclass.
Re: CVS: cvs.openbsd.org: src
Oops. The commit message was supposed to be hppa does not do pinstall() correctly because of that weird callgate stuff. Not sure how to fix it, so disabling this on hppa for now. Theo de Raadt wrote: > CVSROOT: /cvs > Module name: src > Changes by: dera...@cvs.openbsd.org 2023/02/21 09:21:40 > > Modified files: > lib/libc/dlfcn : init.c > > Log message: > dlfcn/init.c >
Re: CVS: cvs.openbsd.org: src
On 2023/02/15 20:10, Theo de Raadt wrote: > transistor shortage, in these trying times > > Jonathan Gray wrote: > > > CVSROOT:/cvs > > Module name:src > > Changes by: j...@cvs.openbsd.org2023/02/15 20:09:33 > > > > Modified files: > > usr.sbin/fw_update: patterns.c > > > > Log message: > > remove the '(R)' from the intel cpu match pattern > > > > Intel(R) does not appear in > > cpu0: Intel Atom(R) x6425RE Processor @ 1.90GHz, 1895.90 MHz, 06-96-01 > > reported by patrick@ ok deraadt@ > > > Perhaps they need a new cpuid function that returns UTF-8 (the model name they use in some other places has a unicode registered trademark symbol in place of the now-missing '(R)').
Re: CVS: cvs.openbsd.org: src
transistor shortage, in these trying times Jonathan Gray wrote: > CVSROOT: /cvs > Module name: src > Changes by: j...@cvs.openbsd.org2023/02/15 20:09:33 > > Modified files: > usr.sbin/fw_update: patterns.c > > Log message: > remove the '(R)' from the intel cpu match pattern > > Intel(R) does not appear in > cpu0: Intel Atom(R) x6425RE Processor @ 1.90GHz, 1895.90 MHz, 06-96-01 > reported by patrick@ ok deraadt@ >
Re: CVS: cvs.openbsd.org: src
On Wed, 25 Jan 2023 12:06:50 -0700, Todd C. Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: mill...@cvs.openbsd.org 2023/01/25 12:06:50 > > Modified files: > usr.bin/pkg-config/OpenBSD: PkgConfig.pm > > Log message: > Fix CVE-2023-24056, unbounded variable expansion in pkg-config. > We now die with an error when trying to expand a variable that is > already longer than 64K. This was never a buffer overflow in our > pkg-config, but rather an unbounded memory allocation that would > eventually run up against resource limits. OK sthen@ jasper@ To avoid confusion on the matter, the CVE listed is for the C version of pkg-config, not our Perl version. This is not a security issue on OpenBSD because: 1) there is no buffer overflow in our perl version 2) only root can install .pc files anyway However, it still makes sense to limit the amount of variable expansion to avoid using excessive memory. The 64K limit was chosen to be compatible with the C version. - todd
Re: CVS: cvs.openbsd.org: src
On Tue, Dec 27 2022, Jeremie Courreges-Anglas wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: j...@cvs.openbsd.org2022/12/27 00:34:05
>
> Modified files:
> sys/lib/libsa : netif.c
> sys/arch/amd64/stand/libsa: pxe.c
> sys/arch/i386/stand/libsa: pxe.c
>
> Log message:
> Ansify pxe_netif_close() and {,pxe}socktodesc()
>
> To appease the clang 15 warning -Wdeprecated-non-prototype (turned on
> by -Wall). ok millert@
Actually with clang 15 -Wdeprecated-non-prototype is on by default,
with or without -Wall.
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: CVS: cvs.openbsd.org: src
Hi Theo, Theo de Raadt wrote on Thu, Dec 22, 2022 at 09:42:36AM -0700: > Hmm. Are you sure about this? Or do you have it backwards? If you harbour doubts, feel free to submit a new file /usr/src/lib/libcrypto/man/BIO_s_log.3 for review, but maybe wait three months and ten days before doing so. > Should we be re-writing libc/stdio as a layerr on top of BIO? Spectacularly good idea, but please make sure jsing@ gets professional psychological support when he is then forced to resolve the resulting circular dependencies in libcrypto/bio/bss_file.c / BIO_s_file(3). Then again, i suspect adding BIO_accept_ex(3) and friends (as documented in OpenSSL 1.1.1 doc/man3/BIO_connect.pod) to LibreSSL and then replacing the Berkeley socket(2) layer with that is an even more pressing task. Yours, Ingo >> CVSROOT: /cvs >> Module name: src >> Changes by: schwa...@cvs.openbsd.org2022/12/22 09:38:45 >> >> Modified files: >> lib/libcrypto/man: BIO_s_null.3 >> >> Log message: >> Mark BIO_s_log(3) as intentionally undocumented. >> >> Ben Laurie invented the system logging BIO in 1999 and yet, >> nothing whatsoever uses it according to codesearch.debian.net. >> Besides, it is poorly designed and a crypto library is absolutely >> not the place for putting a clumsy system logging facility. >> Not everything needs to be a BIO!
Re: CVS: cvs.openbsd.org: src
Hmm. Are you sure about this? Or do you have it backwards? Should we be re-writing libc/stdio as a layerr on top of BIO? Ingo Schwarze wrote: > CVSROOT: /cvs > Module name: src > Changes by: schwa...@cvs.openbsd.org2022/12/22 09:38:45 > > Modified files: > lib/libcrypto/man: BIO_s_null.3 > > Log message: > Mark BIO_s_log(3) as intentionally undocumented. > > Ben Laurie invented the system logging BIO in 1999 and yet, > nothing whatsoever uses it according to codesearch.debian.net. > Besides, it is poorly designed and a crypto library is absolutely > not the place for putting a clumsy system logging facility. > Not everything needs to be a BIO! >
Re: CVS: cvs.openbsd.org: src
On Mon, Dec 19, 2022 at 2:55 PM Philip Guenther wrote: > CVSROOT:/cvs > Module name:src > Changes by: guent...@cvs.openbsd.org2022/12/19 15:55:12 > > Modified files: > usr.bin/kdump : kdump.c kdump_subr.h mksubr > > Log message: > Improve reporting of waitid(2)'s idtype/id and options arguments > Add mimmutable(2) to report like munmap(2) > ...and this is ok deraadt@ and millert@ (not my day for typing commit messages)
Re: CVS: cvs.openbsd.org: src
On Mon, Dec 19, 2022 at 2:45 PM Philip Guenther wrote: > CVSROOT:/cvs > Module name:src > Changes by: guent...@cvs.openbsd.org2022/12/19 15:44:54 > > Modified files: > regress/lib/libc/sys: t_wait_noproc.c > > Log message: > WTRAPPED is now supported by waitid(2) > Don't test waitid(WUNTRACED) as that's not portable and only 'works' due > to an implementation decision > This was ok kettenis@
Re: CVS: cvs.openbsd.org: src
Nicholas Marriott wrote (2022-12-19 08:30 CET): > Try this again now please. Works as expected again. Thanks! - Stefan > On Sat, 17 Dec 2022 at 11:59, Stefan Hagen wrote: > > > Nicholas Marriott wrote (2022-12-16 09:13 CET): > > > CVSROOT: /cvs > > > Module name: src > > > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > > > > > Modified files: > > > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c > > >tmux.1 tmux.h > > > > > > Log message: > > > Add send-keys -K to handle keys directly as if typed (so look up in key > > > table). GitHub issue 3361. > > > > This broke sending keys to a detached session: > > > > Before: > > ./obj/tmux new-session -s mysession -d > > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > > > After: > > ./obj/tmux new-session -s mysession -d > > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > no current client > > > > - Stefan > >
Re: CVS: cvs.openbsd.org: src
Try this again now please. On Sat, 17 Dec 2022 at 11:59, Stefan Hagen wrote: > Nicholas Marriott wrote (2022-12-16 09:13 CET): > > CVSROOT: /cvs > > Module name: src > > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > > > Modified files: > > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c > >tmux.1 tmux.h > > > > Log message: > > Add send-keys -K to handle keys directly as if typed (so look up in key > > table). GitHub issue 3361. > > This broke sending keys to a detached session: > > Before: > ./obj/tmux new-session -s mysession -d > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > After: > ./obj/tmux new-session -s mysession -d > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > no current client > > - Stefan >
Re: CVS: cvs.openbsd.org: src
Nicholas Marriott wrote (2022-12-16 09:13 CET): > CVSROOT: /cvs > Module name: src > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > Modified files: > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c >tmux.1 tmux.h > > Log message: > Add send-keys -K to handle keys directly as if typed (so look up in key > table). GitHub issue 3361. This broke sending keys to a detached session: Before: ./obj/tmux new-session -s mysession -d ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter After: ./obj/tmux new-session -s mysession -d ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter no current client - Stefan
Re: CVS: cvs.openbsd.org: src
No to this proposal.
The tree will remain broken until the change is backed out or fixed.
Anton Lindqvist wrote:
> On Sun, Dec 11, 2022 at 02:19:08PM -0700, Vitaliy Makkoveev wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: m...@cvs.openbsd.org2022/12/11 14:19:08
> >
> > Modified files:
> > sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c
> > uipc_usrreq.c
> > sys/miscfs/fifofs: fifo_vnops.c
> > sys/netinet: tcp_usrreq.c
> > sys/sys: socketvar.h
> >
> > Log message:
> > This time, socket's buffer lock requires solock() to be held. As a part of
> > socket buffers standalone locking work, move socket state bits which
> > represent its buffers state to per buffer state. Introduce `sb_state' and
> > turn SS_CANTSENDMORE to SBS_CANTSENDMORE. This bit will be processed on
> > `so_snd' buffer only.
> >
> > Move SS_CANTRCVMORE and SS_RCVATMARK bits with separate diff to make
> > review easier and exclude possible so_rcv/so_snd mistypes.
> >
> > Also, don't adjust the remaining SS_* bits right now.
> >
> > ok millert@
>
> Tree is broken due to the SS_CANTSENDMORE removal.
>
> Index: fstat.c
> ===
> RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 fstat.c
> --- fstat.c 20 Jun 2022 01:39:44 - 1.103
> +++ fstat.c 12 Dec 2022 06:30:21 -
> @@ -807,8 +807,6 @@ socktrans(struct kinfo_file *kf)
> if (!(kf->so_state & SS_CANTRCVMORE))
> *cp++ = '<';
> *cp++ = '-';
> - if (!(kf->so_state & SS_CANTSENDMORE))
> - *cp++ = '>';
> *cp = '\0';
> printf(" %s ", shoconn);
> hide((void *)(uintptr_t)kf->unp_conn);
>
Re: CVS: cvs.openbsd.org: src
On Sun, Dec 11, 2022 at 02:19:08PM -0700, Vitaliy Makkoveev wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: m...@cvs.openbsd.org2022/12/11 14:19:08
>
> Modified files:
> sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c
>uipc_usrreq.c
> sys/miscfs/fifofs: fifo_vnops.c
> sys/netinet: tcp_usrreq.c
> sys/sys: socketvar.h
>
> Log message:
> This time, socket's buffer lock requires solock() to be held. As a part of
> socket buffers standalone locking work, move socket state bits which
> represent its buffers state to per buffer state. Introduce `sb_state' and
> turn SS_CANTSENDMORE to SBS_CANTSENDMORE. This bit will be processed on
> `so_snd' buffer only.
>
> Move SS_CANTRCVMORE and SS_RCVATMARK bits with separate diff to make
> review easier and exclude possible so_rcv/so_snd mistypes.
>
> Also, don't adjust the remaining SS_* bits right now.
>
> ok millert@
Tree is broken due to the SS_CANTSENDMORE removal.
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.103
diff -u -p -r1.103 fstat.c
--- fstat.c 20 Jun 2022 01:39:44 - 1.103
+++ fstat.c 12 Dec 2022 06:30:21 -
@@ -807,8 +807,6 @@ socktrans(struct kinfo_file *kf)
if (!(kf->so_state & SS_CANTRCVMORE))
*cp++ = '<';
*cp++ = '-';
- if (!(kf->so_state & SS_CANTSENDMORE))
- *cp++ = '>';
*cp = '\0';
printf(" %s ", shoconn);
hide((void *)(uintptr_t)kf->unp_conn);
Re: CVS: cvs.openbsd.org: src
On Fri, Dec 09, 2022 at 10:32:53AM -0700, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2022/12/09 10:32:53 > > Modified files: > sys/netinet6 : icmp6.c nd6.c nd6.h nd6_nbr.c nd6_rtr.c > > Log message: > Switch nd_opts from a union to just a struct. > The ND6 option handling in the kernel got a lot simpler since only > the tgt and src lladdr option are inspected by the kernel. The magic > of assigning options via one side of the union and accessing them > via the other is total overkill and actually quite error prone. > OK florian@ Also OK kn@ who actually pointed me at this madness by sending me some other nd6 cleanup diff. -- :wq Claudio
Re: CVS: cvs.openbsd.org: src
On 16 November 2022 19:30:12 CET, Florian Obser wrote: >CVSROOT: /cvs >Module name: src >Changes by:flor...@cvs.openbsd.org 2022/11/16 11:30:12 > >Modified files: > lib/libc/net : res_comp.c > >Log message: >tolower(3) guarantees to return its argument unchanged if it's not >uppercase. >While here use the correct idiom of casting to unsigned char. > >OK millert, farewell to ultrix deraadt > and ok guenther@ -- Sent from a mobile device. Please excuse poor formatting.
Re: CVS: cvs.openbsd.org: src
On Wed, Nov 09, 2022 at 07:06:43AM +0100, Stefan Sperling wrote:
> On Wed, Nov 09, 2022 at 06:58:32AM +0100, Anton Lindqvist wrote:
> > GENERIC is broken:
> >
> > vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
> > invalid in C99 [-Werror,-Wimplicit-function-declaration]
>
> ok stsp@
I have committed this to unbreak the tree.
Seems like everyone else is absent right now.
> > Index: vmm.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.329
> > diff -u -p -r1.329 vmm.c
> > --- vmm.c 8 Nov 2022 19:38:34 - 1.329
> > +++ vmm.c 9 Nov 2022 05:57:26 -
> > @@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
> > {
> > struct vm *vm;
> > struct vcpu *vcpu;
> > +#ifdef MULTIPROCESSOR
> > struct cpu_info *ci;
> > +#endif
> > int error, ret = 0;
> >
> > /* Find the desired VM */
> > @@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
> > goto out;
> > }
> >
> > +#ifdef MULTIPROCESSOR
> > vcpu->vc_intr = vip->vip_intr;
> > ci = READ_ONCE(vcpu->vc_curcpu);
> > if (ci != NULL)
> > x86_send_ipi(ci, X86_IPI_NOP);
> > +#endif
> >
> > refcnt_rele_wake(>vc_refcnt);
> > out:
> >
> >
>
>
Re: CVS: cvs.openbsd.org: src
On Wed, Nov 09, 2022 at 06:58:32AM +0100, Anton Lindqvist wrote:
> On Tue, Nov 08, 2022 at 12:18:47PM -0700, David Gwynne wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2022/11/08 12:18:47
> >
> > Modified files:
> > sys/arch/amd64/include: vmmvar.h
> > sys/arch/amd64/amd64: vmm.c
> >
> > Log message:
> > further speed up delivery of interrupts to a running vcpu.
> >
> > this records which physical cpu a vcpu is running on. this is used
> > by the code that marks a vcpu as having a pending interrupt to check
> > if the vcpu is currently running. if it thinks the vcpu is running,
> > it sends a nop IPI to the physical cpu it is running on to trigger
> > a vmexit, which in turn runs interrupt handling in the guest.
> >
> > ok mlarkin@
>
> GENERIC is broken:
>
> vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
> invalid in C99 [-Werror,-Wimplicit-function-declaration]
ok stsp@
> Index: vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.329
> diff -u -p -r1.329 vmm.c
> --- vmm.c 8 Nov 2022 19:38:34 - 1.329
> +++ vmm.c 9 Nov 2022 05:57:26 -
> @@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
> {
> struct vm *vm;
> struct vcpu *vcpu;
> +#ifdef MULTIPROCESSOR
> struct cpu_info *ci;
> +#endif
> int error, ret = 0;
>
> /* Find the desired VM */
> @@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
> goto out;
> }
>
> +#ifdef MULTIPROCESSOR
> vcpu->vc_intr = vip->vip_intr;
> ci = READ_ONCE(vcpu->vc_curcpu);
> if (ci != NULL)
> x86_send_ipi(ci, X86_IPI_NOP);
> +#endif
>
> refcnt_rele_wake(>vc_refcnt);
> out:
>
>
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 08, 2022 at 12:18:47PM -0700, David Gwynne wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: d...@cvs.openbsd.org2022/11/08 12:18:47
>
> Modified files:
> sys/arch/amd64/include: vmmvar.h
> sys/arch/amd64/amd64: vmm.c
>
> Log message:
> further speed up delivery of interrupts to a running vcpu.
>
> this records which physical cpu a vcpu is running on. this is used
> by the code that marks a vcpu as having a pending interrupt to check
> if the vcpu is currently running. if it thinks the vcpu is running,
> it sends a nop IPI to the physical cpu it is running on to trigger
> a vmexit, which in turn runs interrupt handling in the guest.
>
> ok mlarkin@
GENERIC is broken:
vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
invalid in C99 [-Werror,-Wimplicit-function-declaration]
Index: vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.329
diff -u -p -r1.329 vmm.c
--- vmm.c 8 Nov 2022 19:38:34 - 1.329
+++ vmm.c 9 Nov 2022 05:57:26 -
@@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
{
struct vm *vm;
struct vcpu *vcpu;
+#ifdef MULTIPROCESSOR
struct cpu_info *ci;
+#endif
int error, ret = 0;
/* Find the desired VM */
@@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
goto out;
}
+#ifdef MULTIPROCESSOR
vcpu->vc_intr = vip->vip_intr;
ci = READ_ONCE(vcpu->vc_curcpu);
if (ci != NULL)
x86_send_ipi(ci, X86_IPI_NOP);
+#endif
refcnt_rele_wake(>vc_refcnt);
out:
Re: CVS: cvs.openbsd.org: src
Oops, unintented commit of the new ld.script They will not be used until Makefile.inc references them, and are still undergoing test. Theo de Raadt wrote: > CVSROOT: /cvs > Module name: src > Changes by: dera...@cvs.openbsd.org 2022/11/08 06:47:22 > > Modified files: > libexec/ld.so : loader.c > Added files: > libexec/ld.so/alpha: ld.script > libexec/ld.so/hppa: ld.script > libexec/ld.so/i386: ld.script > > Log message: > In the new scheme, the main executable object needs to be marked > nodelete, so that _dl_relro() will immutable it's relro. >
Re: CVS: cvs.openbsd.org: src
On Thu, Nov 03, 2022 at 11:41:22AM -0600, Patrick Wildt wrote: > CVSROOT: /cvs > Module name: src > Changes by: patr...@cvs.openbsd.org 2022/11/03 11:41:22 > > Modified files: > sys/arch/arm64/conf: RAMDISK > > Log message: > Enable smbios0 on arm64 RAMDISK so the code mitigating crashes on the x13s > gets correct info from hw.version. > That commit was: ok kettenis@ phessler@
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 01, 2022 at 02:26:20PM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2022/11/01 14:26:20 > > Modified files: > gnu/lib/libreadline: rlstdc.h > > Log message: > Do not neuter __attribute__ with __STRICT_ANSI__ > > This broke readline support in newer Pythons and generally seems a > bad idea. Upstream have removed this conditional in 5.0. > > ok millert > Also ok daniel jca
Re: CVS: cvs.openbsd.org: src
On Wed, Oct 26, 2022 at 07:20:20AM +0200, Anton Lindqvist wrote:
> On Tue, Oct 25, 2022 at 10:08:26AM -0600, Mark Kettenis wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: kette...@cvs.openbsd.org2022/10/25 10:08:26
> >
> > Modified files:
> > sys/sys: siginfo.h wait.h
> > sys/kern : kern_exit.c
> >
> > Log message:
> > Implement waitid(2) which is now part of POSIX and used by mozilla.
> > This includes a change of siginfo_r which is technically an ABI break but
> > this should have no real-world impact since the members involved are
> > never touched by the kernel.
> >
> > ok millert@, deraadt@
>
> My snapshot build failed today, smells related to this as I have never
> seen this before. Will try to gather more info later today.
>
> robsd-exec: waitpid: No child processes
This program used to exit zero:
#include
#include
#include
#include
#include
#include
static int exitstatus(int);
static int waiteof(int, int);
int
main(void)
{
pid_t pid;
int pip[2];
int error, status;
if (pipe2(pip, O_NONBLOCK) == -1)
err(1, "pipe2");
pid = fork();
if (pid == -1)
err(1, "fork");
if (pid == 0) {
close(pip[0]);
if (setsid() == -1)
err(1, "setsid");
/* Signal to the parent that the process group is present. */
close(pip[1]);
_exit(0);
}
/* Wait for the process group to become present. */
close(pip[1]);
if (waiteof(pip[0], 1000)) {
warnx("process group failure");
if (waitpid(pid, , 0) == -1)
return 1;
return exitstatus(status);
}
close(pip[0]);
if (waitpid(-pid, , 0) == -1)
err(1, "waitpid");
error = exitstatus(status);
if (error)
warnx("process group exited %d", error);
return error;
}
static int
exitstatus(int status)
{
if (WIFEXITED(status))
return WEXITSTATUS(status);
if (WIFSIGNALED(status))
return 128 + WTERMSIG(status);
return 1;
}
static int
waiteof(int fd, int timoms)
{
int slpms = 100;
for (;;) {
char buf[1];
ssize_t n;
n = read(fd, buf, sizeof(buf));
if (n == -1) {
if (errno == EAGAIN) {
usleep(slpms * 1000);
timoms -= slpms;
if (timoms <= 0)
return 1;
} else {
warn("read");
return 1;
}
}
if (n == 0)
break;
}
return 0;
}
Re: CVS: cvs.openbsd.org: src
On Tue, Oct 25, 2022 at 10:08:26AM -0600, Mark Kettenis wrote: > CVSROOT: /cvs > Module name: src > Changes by: kette...@cvs.openbsd.org2022/10/25 10:08:26 > > Modified files: > sys/sys: siginfo.h wait.h > sys/kern : kern_exit.c > > Log message: > Implement waitid(2) which is now part of POSIX and used by mozilla. > This includes a change of siginfo_r which is technically an ABI break but > this should have no real-world impact since the members involved are > never touched by the kernel. > > ok millert@, deraadt@ My snapshot build failed today, smells related to this as I have never seen this before. Will try to gather more info later today. robsd-exec: waitpid: No child processes
Re: CVS: cvs.openbsd.org: src
On Thu, Oct 13, 2022 at 03:37:05PM -0600, Jason McIntyre wrote: > CVSROOT: /cvs > Module name: src > Changes by: j...@cvs.openbsd.org2022/10/13 15:37:05 > > Modified files: > bin/ps : ps.1 > lib/libc/compat-43: sigvec.3 > lib/libc/gen : signal.3 ttyname.3 > lib/libc/sys : execve.2 getpgrp.2 sigaction.2 > sbin/dump : dump.8 > share/man/man4 : pty.4 > usr.bin/tset : tset.1 > > Log message: > controm terminql -> controlling terminal; > sorry for messing the attributes (and mungling the spelling): diff from josiah frentsos; direction from millert
Re: CVS: cvs.openbsd.org: src
Anton Lindqvist wrote: > CVSROOT: /cvs > Module name: src > Changes by: an...@cvs.openbsd.org 2022/10/10 23:45:41 > > Modified files: > regress/sys/kern/noexec: Makefile noexec.c > > Log message: > Run noexec tests in a new thread, leveraging the fact that > pthread_create() allocates a new stack which has mutable permissions. > Allows the temporary expected failures to be dropped. > > ok deraadt@ The purpose of some of these regress tests is not to check if you can mmap or mprotect on top of the stack (or "a stack"). Rather it is testing how the stacks behave, whether the pmap and mmu/tlb have honoured the requested behaviour That's why we still want these tests to work. But obviously they won't work on the main stack, if we succeed at making the region immutable. That's why a pthread stack is more suitable for testing this. of course, an i386 with W^X as line-in-the-sand segmentation support doesn't exactly do this right for pthread stacks *shrug*
Re: CVS: cvs.openbsd.org: src
> CVSROOT: /cvs > Module name: src > Changes by: m...@cvs.openbsd.org2022/10/04 13:38:20 > > Modified files: > sys/dev/usb: ukbd.c > > Log message: > Repair Apple-specific translation support broken by mistake in 1.88; reported > by Leonardo Moreno That should read ``Leonardo Moreno Urbieta'', sorry.
Re: CVS: cvs.openbsd.org: src
Stefan Hagen wrote (2022-09-26 08:14 CEST): > CVSROOT: /cvs > Module name: src > Changes by: s...@cvs.openbsd.org2022/09/26 00:14:21 > > Modified files: > sys/dev/usb: usbdevs uwacom.c > share/man/man4 : uwacom.4 > > Log message: > uwacom(4): Support for Wacom One S (CTL-472) Tested by caspar OK sthen, miod
Re: CVS: cvs.openbsd.org: src
Stefan Hagen wrote (2022-09-26 08:14 CEST): > CVSROOT: /cvs > Module name: src > Changes by: s...@cvs.openbsd.org2022/09/26 00:14:21 > > Modified files: > sys/dev/usb: usbdevs uwacom.c > share/man/man4 : uwacom.4 > > Log message: > uwacom(4): Support for Wacom One S (CTL-472) Forgot: OK sthen, miod
Re: CVS: cvs.openbsd.org: src
> On 3 Sep 2022, at 15:56, Alexander Bluhm wrote:
>
> On Sat, Sep 03, 2022 at 03:56:38AM +0300, Vitaliy Makkoveev wrote:
>> Or may be better to check `pr_type' with the ability of splicing?
>
> I don't think that makes sense. Reasonable splicing does not depend
> on the type of socket, not on the protocol family, but on the
> protocol. That is the reason I used the usrreq pointer before. As
> it involves sending packets, I suggested pru_send now.
>
> You can splice tcp to tcp, and udp to udp.
> You cannot splice udp to rip although both use pr_type SOCK_RAW.
>
> I am not convinced that introducing sepecial IPv6 functions pointers
> was a good idea. That means a lot of code duplication.
>
> Whatever. With the current feature set your diff is equivalent to
> any other solution discussed. I have not implemented splicing for
> rip or rip6 yet.
>
> Regress is broken, fix it somehow and quickly. OK bluhm@
Done.
>
>> Index: sys/kern/uipc_socket.c
>> ===
>> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
>> retrieving revision 1.286
>> diff -u -p -r1.286 uipc_socket.c
>> --- sys/kern/uipc_socket.c 28 Aug 2022 18:43:12 - 1.286
>> +++ sys/kern/uipc_socket.c 3 Sep 2022 00:53:39 -
>> @@ -1293,7 +1293,8 @@ sosplice(struct socket *so, int fd, off_
>> if ((error = getsock(curproc, fd, )) != 0)
>> return (error);
>> sosp = fp->f_data;
>> -if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
>> +if ((sosp->so_proto->pr_flags & PR_SPLICE) == 0 ||
>> +(sosp->so_proto->pr_type != so->so_proto->pr_type)) {
>> error = EPROTONOSUPPORT;
>> goto frele;
>> }
Re: CVS: cvs.openbsd.org: src
On Sat, Sep 03, 2022 at 03:56:38AM +0300, Vitaliy Makkoveev wrote:
> Or may be better to check `pr_type' with the ability of splicing?
I don't think that makes sense. Reasonable splicing does not depend
on the type of socket, not on the protocol family, but on the
protocol. That is the reason I used the usrreq pointer before. As
it involves sending packets, I suggested pru_send now.
You can splice tcp to tcp, and udp to udp.
You cannot splice udp to rip although both use pr_type SOCK_RAW.
I am not convinced that introducing sepecial IPv6 functions pointers
was a good idea. That means a lot of code duplication.
Whatever. With the current feature set your diff is equivalent to
any other solution discussed. I have not implemented splicing for
rip or rip6 yet.
Regress is broken, fix it somehow and quickly. OK bluhm@
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.286
> diff -u -p -r1.286 uipc_socket.c
> --- sys/kern/uipc_socket.c28 Aug 2022 18:43:12 - 1.286
> +++ sys/kern/uipc_socket.c3 Sep 2022 00:53:39 -
> @@ -1293,7 +1293,8 @@ sosplice(struct socket *so, int fd, off_
> if ((error = getsock(curproc, fd, )) != 0)
> return (error);
> sosp = fp->f_data;
> - if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
> + if ((sosp->so_proto->pr_flags & PR_SPLICE) == 0 ||
> + (sosp->so_proto->pr_type != so->so_proto->pr_type)) {
> error = EPROTONOSUPPORT;
> goto frele;
> }
Re: CVS: cvs.openbsd.org: src
On Sat, Sep 03, 2022 at 02:44:56AM +0200, Alexander Bluhm wrote:
> On Sat, Sep 03, 2022 at 02:10:59AM +0300, Vitaliy Makkoveev wrote:
> > On Sat, Sep 03, 2022 at 12:35:41AM +0200, Alexander Bluhm wrote:
> > > This broke socket splicing between inet and inet6 sockets.
> > > sosplice() has this check:
> > >
> > > if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
> > > error = EPROTONOSUPPORT;
> > > goto frele;
> > > }
> > >
> > > That does not work anymore after splitting {tcp,udp}6_usrreqs.
> > >
> > > bluhm
> > >
> > > On Fri, Sep 02, 2022 at 07:12:32AM -0600, Vitaliy Makkoveev wrote:
> > > > CVSROOT:/cvs
> > > > Module name:src
> > > > Changes by: m...@cvs.openbsd.org2022/09/02 07:12:32
> > > >
> > > > Modified files:
> > > > sys/kern : sys_socket.c uipc_usrreq.c
> > > > sys/net: if.c pfkeyv2.c rtsock.c
> > > > sys/netinet: ip_divert.c ip_gre.c raw_ip.c tcp_usrreq.c
> > > > tcp_var.h udp_usrreq.c udp_var.h
> > > > sys/netinet6 : in6_proto.c ip6_divert.c raw_ip6.c
> > > > sys/sys: protosw.h
> > > >
> > > > Log message:
> > > > Move PRU_CONTROL request to (*pru_control)().
> > > >
> > > > The 'proc *' arg is not used for PRU_CONTROL request, so remove it from
> > > > pru_control() wrapper.
> > > >
> > > > Split out {tcp,udp}6_usrreqs from {tcp,udp}_usrreqs and use them for
> > > > inet6 case.
> > > >
> > > > ok guenther@ bluhm@
> >
> > IIRC it's assumed that inet tcp/udp socket on port N and inet6 tcp/udp
> > socket on the same port N are different sockets, because the protocols
> > are different.
>
> The sockets you splice togeher can have totally different addresses
> and ports. They need nothing in common, just put mbufs from one
> into the other.
>
> So the ability of splicing inet and inet6 sockets looks
> > strange for me. Also I wonder, is this really used?
>
> Yes.
>
> > This diff should restore existing behaviour. The `pru_attach' handler is
> > the same for inet and inet6 cases.
>
> gre also uses rip_attach. Better use the pru_send, they are unique.
>
Or may be better to check `pr_type' with the ability of splicing?
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.286
diff -u -p -r1.286 uipc_socket.c
--- sys/kern/uipc_socket.c 28 Aug 2022 18:43:12 - 1.286
+++ sys/kern/uipc_socket.c 3 Sep 2022 00:53:39 -
@@ -1293,7 +1293,8 @@ sosplice(struct socket *so, int fd, off_
if ((error = getsock(curproc, fd, )) != 0)
return (error);
sosp = fp->f_data;
- if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
+ if ((sosp->so_proto->pr_flags & PR_SPLICE) == 0 ||
+ (sosp->so_proto->pr_type != so->so_proto->pr_type)) {
error = EPROTONOSUPPORT;
goto frele;
}
Re: CVS: cvs.openbsd.org: src
On Sat, Sep 03, 2022 at 02:10:59AM +0300, Vitaliy Makkoveev wrote:
> On Sat, Sep 03, 2022 at 12:35:41AM +0200, Alexander Bluhm wrote:
> > This broke socket splicing between inet and inet6 sockets.
> > sosplice() has this check:
> >
> > if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
> > error = EPROTONOSUPPORT;
> > goto frele;
> > }
> >
> > That does not work anymore after splitting {tcp,udp}6_usrreqs.
> >
> > bluhm
> >
> > On Fri, Sep 02, 2022 at 07:12:32AM -0600, Vitaliy Makkoveev wrote:
> > > CVSROOT: /cvs
> > > Module name: src
> > > Changes by: m...@cvs.openbsd.org2022/09/02 07:12:32
> > >
> > > Modified files:
> > > sys/kern : sys_socket.c uipc_usrreq.c
> > > sys/net: if.c pfkeyv2.c rtsock.c
> > > sys/netinet: ip_divert.c ip_gre.c raw_ip.c tcp_usrreq.c
> > >tcp_var.h udp_usrreq.c udp_var.h
> > > sys/netinet6 : in6_proto.c ip6_divert.c raw_ip6.c
> > > sys/sys: protosw.h
> > >
> > > Log message:
> > > Move PRU_CONTROL request to (*pru_control)().
> > >
> > > The 'proc *' arg is not used for PRU_CONTROL request, so remove it from
> > > pru_control() wrapper.
> > >
> > > Split out {tcp,udp}6_usrreqs from {tcp,udp}_usrreqs and use them for
> > > inet6 case.
> > >
> > > ok guenther@ bluhm@
>
> IIRC it's assumed that inet tcp/udp socket on port N and inet6 tcp/udp
> socket on the same port N are different sockets, because the protocols
> are different.
The sockets you splice togeher can have totally different addresses
and ports. They need nothing in common, just put mbufs from one
into the other.
So the ability of splicing inet and inet6 sockets looks
> strange for me. Also I wonder, is this really used?
Yes.
> This diff should restore existing behaviour. The `pru_attach' handler is
> the same for inet and inet6 cases.
gre also uses rip_attach. Better use the pru_send, they are unique.
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.286
> diff -u -p -r1.286 uipc_socket.c
> --- sys/kern/uipc_socket.c28 Aug 2022 18:43:12 - 1.286
> +++ sys/kern/uipc_socket.c2 Sep 2022 23:10:20 -
> @@ -1293,7 +1293,8 @@ sosplice(struct socket *so, int fd, off_
> if ((error = getsock(curproc, fd, )) != 0)
> return (error);
> sosp = fp->f_data;
> - if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
> + if (sosp->so_proto->pr_usrreqs->pru_attach !=
> + so->so_proto->pr_usrreqs->pru_attach) {
> error = EPROTONOSUPPORT;
> goto frele;
> }
Re: CVS: cvs.openbsd.org: src
On Sat, Sep 03, 2022 at 12:35:41AM +0200, Alexander Bluhm wrote:
> This broke socket splicing between inet and inet6 sockets.
> sosplice() has this check:
>
> if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
> error = EPROTONOSUPPORT;
> goto frele;
> }
>
> That does not work anymore after splitting {tcp,udp}6_usrreqs.
>
> bluhm
>
> On Fri, Sep 02, 2022 at 07:12:32AM -0600, Vitaliy Makkoveev wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: m...@cvs.openbsd.org2022/09/02 07:12:32
> >
> > Modified files:
> > sys/kern : sys_socket.c uipc_usrreq.c
> > sys/net: if.c pfkeyv2.c rtsock.c
> > sys/netinet: ip_divert.c ip_gre.c raw_ip.c tcp_usrreq.c
> > tcp_var.h udp_usrreq.c udp_var.h
> > sys/netinet6 : in6_proto.c ip6_divert.c raw_ip6.c
> > sys/sys: protosw.h
> >
> > Log message:
> > Move PRU_CONTROL request to (*pru_control)().
> >
> > The 'proc *' arg is not used for PRU_CONTROL request, so remove it from
> > pru_control() wrapper.
> >
> > Split out {tcp,udp}6_usrreqs from {tcp,udp}_usrreqs and use them for
> > inet6 case.
> >
> > ok guenther@ bluhm@
IIRC it's assumed that inet tcp/udp socket on port N and inet6 tcp/udp
socket on the same port N are different sockets, because the protocols
are different. So the ability of splicing inet and inet6 sockets looks
strange for me. Also I wonder, is this really used?
This diff should restore existing behaviour. The `pru_attach' handler is
the same for inet and inet6 cases.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.286
diff -u -p -r1.286 uipc_socket.c
--- sys/kern/uipc_socket.c 28 Aug 2022 18:43:12 - 1.286
+++ sys/kern/uipc_socket.c 2 Sep 2022 23:10:20 -
@@ -1293,7 +1293,8 @@ sosplice(struct socket *so, int fd, off_
if ((error = getsock(curproc, fd, )) != 0)
return (error);
sosp = fp->f_data;
- if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
+ if (sosp->so_proto->pr_usrreqs->pru_attach !=
+ so->so_proto->pr_usrreqs->pru_attach) {
error = EPROTONOSUPPORT;
goto frele;
}
Re: CVS: cvs.openbsd.org: src
This broke socket splicing between inet and inet6 sockets.
sosplice() has this check:
if (sosp->so_proto->pr_usrreqs != so->so_proto->pr_usrreqs) {
error = EPROTONOSUPPORT;
goto frele;
}
That does not work anymore after splitting {tcp,udp}6_usrreqs.
bluhm
On Fri, Sep 02, 2022 at 07:12:32AM -0600, Vitaliy Makkoveev wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: m...@cvs.openbsd.org2022/09/02 07:12:32
>
> Modified files:
> sys/kern : sys_socket.c uipc_usrreq.c
> sys/net: if.c pfkeyv2.c rtsock.c
> sys/netinet: ip_divert.c ip_gre.c raw_ip.c tcp_usrreq.c
>tcp_var.h udp_usrreq.c udp_var.h
> sys/netinet6 : in6_proto.c ip6_divert.c raw_ip6.c
> sys/sys: protosw.h
>
> Log message:
> Move PRU_CONTROL request to (*pru_control)().
>
> The 'proc *' arg is not used for PRU_CONTROL request, so remove it from
> pru_control() wrapper.
>
> Split out {tcp,udp}6_usrreqs from {tcp,udp}_usrreqs and use them for
> inet6 case.
>
> ok guenther@ bluhm@
Re: CVS: cvs.openbsd.org: src
On Fri, Aug 26, 2022 at 03:09:34PM -0600, Todd C. Miller wrote: > On Fri, 26 Aug 2022 23:00:42 +0200, Alexander Bluhm wrote: > > > I have this line in my /etc/fstab > > > > /dev/sd0b /build mfs rw,noperm,nodev,nosuid,-s=2064348,noauto > > > > # mount /build > > mount_mfs: /dev/sd0b: Block device required > > > > According to ktrace this command fails. > > # /sbin/mount_mfs -o rw -o noperm -o nodev -o nosuid -s 2064348 -o noauto > > /de > > v/sd0b /build > > Perhaps mount_mfs should call opendev() with OPENDEV_BLCK since > that is the normal way to invoking it. Either way, I'll back it out now as bluhm's case should work. > > - todd
Re: CVS: cvs.openbsd.org: src
On Fri, 26 Aug 2022 23:00:42 +0200, Alexander Bluhm wrote: > This breaks my memory file system for building release. > I have this line in my /etc/fstab > > /dev/sd0b /build mfs rw,noperm,nodev,nosuid,-s=2064348,noauto > > # mount /build > mount_mfs: /dev/sd0b: Block device required > > According to ktrace this command fails. > # /sbin/mount_mfs -o rw -o noperm -o nodev -o nosuid -s 2064348 -o noauto /de > v/sd0b /build Reviewing how opendev(3) is used again I think this needs to be backed out. The the block vs. character device distinction is really only relevant when converting a short-form device name to its full path. There is no reason to reject an existing file just because it is not a device--many utilities that use opendev() should work with the contents of a device dumped to a file. - todd
Re: CVS: cvs.openbsd.org: src
On Fri, 26 Aug 2022 23:00:42 +0200, Alexander Bluhm wrote: > I have this line in my /etc/fstab > > /dev/sd0b /build mfs rw,noperm,nodev,nosuid,-s=2064348,noauto > > # mount /build > mount_mfs: /dev/sd0b: Block device required > > According to ktrace this command fails. > # /sbin/mount_mfs -o rw -o noperm -o nodev -o nosuid -s 2064348 -o noauto /de > v/sd0b /build Perhaps mount_mfs should call opendev() with OPENDEV_BLCK since that is the normal way to invoking it. - todd
Re: CVS: cvs.openbsd.org: src
This breaks my memory file system for building release. I have this line in my /etc/fstab /dev/sd0b /build mfs rw,noperm,nodev,nosuid,-s=2064348,noauto # mount /build mount_mfs: /dev/sd0b: Block device required According to ktrace this command fails. # /sbin/mount_mfs -o rw -o noperm -o nodev -o nosuid -s 2064348 -o noauto /dev/sd0b /build Without this commit it works, bluhm On Thu, Aug 25, 2022 at 11:09:54AM -0600, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2022/08/25 11:09:54 > > Modified files: > lib/libutil: opendev.3 opendev.c > > Log message: > Only return file descriptors to block or character devices > > If the requested path contained a slash, opendev(3) blindly opened the file > and returned a file descriptor to it. > > Check for block or character devices (according to OPENDEV_BLCK) and fail > for other types. > > Spotted through installboot(8) which happily opened a stage file as device > when forgetting the device argument: > # installboot -v ./biosboot > Using / as root > installing bootstrap on ./biosboot > using first-stage /usr/mdec/biosboot, second-stage /usr/mdec/boot > installboot: disklabel: ./biosboot: Inappropriate ioctl for device > > This makes it fail earlier, as expected: > # installboot -v ./biosboot > installboot: open: ./biosboot: Block device required > > The case where opendev(3) is passed a string not containing a slash, i.e. > a supposed DUID, is fine, as diskmap(4) will ensure that only valid device > paths are returned, if the DUID is valid. > > Feedback OK millert
Re: CVS: cvs.openbsd.org: src
On 2022/08/24 15:08, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2022/08/24 15:08:51 > > Modified files: > distrib/special/installboot: Makefile > > Log message: > Forgotten to commit as part of > > > /usr/src/usr.sbin/installboot/Makefile revision 1.25 It's unhelpful to include those lines directly quoted in a separate commit log, it breaks tools which parse log output (like cvsps). I normally scratch over one of the -'s to avoid the problem.
Re: CVS: cvs.openbsd.org: src
On Sat, Aug 20, 2022 at 7:24 PM Jonathan Gray wrote: > > On Sat, Aug 20, 2022 at 01:26:00PM -0600, Daniel Dickman wrote: > > CVSROOT: /cvs > > Module name: src > > Changes by: dan...@cvs.openbsd.org 2022/08/20 13:26:00 > > > > Modified files: > > sys/arch/amd64/include: specialreg.h > > > > Log message: > > remove Cyrix 486DLC register defines from amd64 > > > > Cyrix CPUs don't support amd64. These defines were probably carried > > over from i386 accidentally when the amd64 code was first imported. > > This file was purposefully kept in sync between archs. > That hasn't been the case with vmm and some other parts though. As you say, the files are quite different currently. Although a lot of the differences should be easily mergable (i.e. whitespace and variable name difference only) but there are many other differences. I just committed the i386 bits to remove 486DLC registers so that will get the files slightly closer to alignment. diff -ub i386/include/specialreg.h amd64/include/specialreg.h | diffstat specialreg.h | 703 +++--- 1 file changed, 675 insertions(+), 28 deletions(-)
Re: CVS: cvs.openbsd.org: src
On Sat, Aug 20, 2022 at 01:26:00PM -0600, Daniel Dickman wrote: > CVSROOT: /cvs > Module name: src > Changes by: dan...@cvs.openbsd.org 2022/08/20 13:26:00 > > Modified files: > sys/arch/amd64/include: specialreg.h > > Log message: > remove Cyrix 486DLC register defines from amd64 > > Cyrix CPUs don't support amd64. These defines were probably carried > over from i386 accidentally when the amd64 code was first imported. This file was purposefully kept in sync between archs. That hasn't been the case with vmm and some other parts though.
Re: CVS: cvs.openbsd.org: src
On Sat, Aug 13, 2022 at 10:02:15AM -0600, Sebastien Marie wrote: > CVSROOT: /cvs > Module name: src > Changes by: sema...@cvs.openbsd.org 2022/08/13 10:02:15 > > Modified files: > regress/sys/uvm/blist: Makefile > sys/kern : subr_blist.c > Added files: > regress/sys/uvm/blist: test-6.in test-6.out > > Log message: > blist: fix a possible blist corruption with blist_alloc() due to unsigned > swblk_t on OpenBSD. > > reorder if condition in blst_meta_alloc(), in order to check if the node is > 'Terminator' node first (and leave the loop). > > DragonFlyBSD is unaffected by it as swblk_t is signed (and the first condition > isn't taken). > > add a regress test for it. > > while here, more the KASSERT() to KDASSERT(). it is useful but only with > DEBUG. > > ok miod@ todd@ EWRONGTODD it was ok millert@ sorry about that. -- Sebastien Marie
Re: CVS: cvs.openbsd.org: src
On Tue, Aug 09, 2022 at 03:10:03PM -0600, Klemens Nanni wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: k...@cvs.openbsd.org2022/08/09 15:10:03
>
> Modified files:
> sys/netinet6 : ip6_forward.c ip6_mroute.c nd6.c
>
> Log message:
> Backout "Call getuptime() just once per function"
>
> This caused stuck ndp cache entries as found by naddy, sorry.
Maybe the timeout layer can do more to help developers catch these
issues in the future.
This patch is strict. It would require a sizeable audit. There are
many callers.
Would guardrails like this have helped you?
Index: kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.85
diff -u -p -r1.85 kern_timeout.c
--- kern_timeout.c 19 Jun 2021 02:05:33 - 1.85
+++ kern_timeout.c 11 Aug 2022 18:09:24 -
@@ -335,6 +335,8 @@ timeout_add_tv(struct timeout *to, const
{
uint64_t to_ticks;
+ KASSERT(tv->tv_sec >= 0 && timerisvalid(tv));
+
to_ticks = (uint64_t)hz * tv->tv_sec + tv->tv_usec / tick;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
@@ -349,6 +351,8 @@ timeout_add_sec(struct timeout *to, int
{
uint64_t to_ticks;
+ KASSERT(secs >= 0);
+
to_ticks = (uint64_t)hz * secs;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
@@ -363,6 +367,8 @@ timeout_add_msec(struct timeout *to, int
{
uint64_t to_ticks;
+ KASSERT(msecs >= 0);
+
to_ticks = (uint64_t)msecs * 1000 / tick;
if (to_ticks > INT_MAX)
to_ticks = INT_MAX;
@@ -375,8 +381,11 @@ timeout_add_msec(struct timeout *to, int
int
timeout_add_usec(struct timeout *to, int usecs)
{
- int to_ticks = usecs / tick;
+ int to_ticks;
+ KASSERT(usecs >= 0);
+
+ to_ticks = usecs / tick;
if (to_ticks == 0 && usecs > 0)
to_ticks = 1;
@@ -386,8 +395,11 @@ timeout_add_usec(struct timeout *to, int
int
timeout_add_nsec(struct timeout *to, int nsecs)
{
- int to_ticks = nsecs / (tick * 1000);
+ int to_ticks;
+
+ KASSERT(nsecs >= 0);
+ to_ticks = nsecs / (tick * 1000);
if (to_ticks == 0 && nsecs > 0)
to_ticks = 1;
Re: CVS: cvs.openbsd.org: src
On Sat, Jul 09, 2022 at 05:24:44PM -0600, Alexander Hall wrote: > CVSROOT: /cvs > Module name: src > Changes by: ha...@cvs.openbsd.org 2022/07/09 17:24:44 > > Modified files: > usr.sbin/tcpdump: tcpdump.c > > Log message: > Explicitly set the default value for Bflag to BPF_FILDROP_PASS > > Technically a nop since the value of the initial constant is 0 anyway > but we should not rely on that. This was OK kn@
Re: CVS: cvs.openbsd.org: src
Committed, thanks!
On Sat, Jul 02, 2022 at 08:50:33AM +0200, Anton Lindqvist wrote:
> On Fri, Jul 01, 2022 at 03:56:18AM -0600, Vitaliy Makkoveev wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: m...@cvs.openbsd.org2022/07/01 03:56:18
> >
> > Modified files:
> > sys/kern : uipc_socket.c uipc_socket2.c uipc_syscalls.c
> > uipc_usrreq.c
> > sys/miscfs/fifofs: fifo_vnops.c
> > sys/sys: socketvar.h unpcb.h
> >
> > Log message:
> > Make fine grained unix(4) domain sockets locking. Use the per-socket
> > `so_lock' rwlock(9) instead of global `unp_lock' which locks the whole
> > layer.
> >
> > The PCB of unix(4) sockets are linked to each other and we need to lock
> > them both. This introduces the lock ordering problem, because when the
> > thread (1) keeps lock on `so1' and trying to lock `so2', the thread (2)
> > could hold lock on `so2' and trying to lock `so1'. To solve this we
> > always lock sockets in the strict order.
> >
> > For the sockets which are already accessible from userland, we always
> > lock socket with the smallest memory address first. Sometimes we need to
> > unlock socket before lock it's peer and lock it again.
> >
> > We use reference counters for prevent the connected peer destruction
> > during to relock. We also handle the case where the peer socket was
> > replaced by another socket.
> >
> > For the newly connected sockets, which are not yet exported to the
> > userland by accept(2), we always lock the listening socket `head' first.
> > This allows us to avoid unwanted relock within accept(2) syscall.
> >
> > ok claudio@
>
> syzkaller found a panic, I'm spotting one missed unlock. Feel free to
> commit.
>
> diff --git sys/kern/uipc_usrreq.c sys/kern/uipc_usrreq.c
> index 0710393d376..7231b16b735 100644
> --- sys/kern/uipc_usrreq.c
> +++ sys/kern/uipc_usrreq.c
> @@ -363,6 +363,7 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m,
> struct mbuf *nam,
> control = NULL;
> } else {
> error = ENOBUFS;
> + sounlock(so2);
> break;
> }
> } else if (so->so_type == SOCK_SEQPACKET)
>
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit:a4a82b864d54 Remove PIPEXCSESSION ioctl(2) call only from ..
> git tree: openbsd
> console output: https://syzkaller.appspot.com/x/log.txt?x=10cffe2408
> kernel config: https://syzkaller.appspot.com/x/.config?x=7058272de1526588
> dashboard link: https://syzkaller.appspot.com/bug?extid=a648408d6a58fd40b59a
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15cc61f408
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=178e44ec08
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+a648408d6a58fd40b...@syzkaller.appspotmail.com
>
> witness: userret: returning with the following locks held:
> exclusive rwlock solock r = 0 (0xfd8073dde420)
> #0 witness_lock+0x44d
> #1 unp_solock_peer+0x64 sys/kern/uipc_usrreq.c:168
> #2 uipc_usrreq+0x7c6 sys/kern/uipc_usrreq.c:350
> #3 sosend+0x61b sys/kern/uipc_socket.c:657
> #4 sendit+0x65d sys/kern/uipc_syscalls.c:682
> #5 sys_sendmsg+0x198 sys/kern/uipc_syscalls.c:589
> #6 syscall+0x4c3 mi_syscall sys/sys/syscall_mi.h:101 [inline]
> #6 syscall+0x4c3 sys/arch/amd64/amd64/trap.c:585
> #7 Xsyscall+0x128
Re: CVS: cvs.openbsd.org: src
On Fri, Jul 01, 2022 at 03:56:18AM -0600, Vitaliy Makkoveev wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: m...@cvs.openbsd.org2022/07/01 03:56:18
>
> Modified files:
> sys/kern : uipc_socket.c uipc_socket2.c uipc_syscalls.c
>uipc_usrreq.c
> sys/miscfs/fifofs: fifo_vnops.c
> sys/sys: socketvar.h unpcb.h
>
> Log message:
> Make fine grained unix(4) domain sockets locking. Use the per-socket
> `so_lock' rwlock(9) instead of global `unp_lock' which locks the whole
> layer.
>
> The PCB of unix(4) sockets are linked to each other and we need to lock
> them both. This introduces the lock ordering problem, because when the
> thread (1) keeps lock on `so1' and trying to lock `so2', the thread (2)
> could hold lock on `so2' and trying to lock `so1'. To solve this we
> always lock sockets in the strict order.
>
> For the sockets which are already accessible from userland, we always
> lock socket with the smallest memory address first. Sometimes we need to
> unlock socket before lock it's peer and lock it again.
>
> We use reference counters for prevent the connected peer destruction
> during to relock. We also handle the case where the peer socket was
> replaced by another socket.
>
> For the newly connected sockets, which are not yet exported to the
> userland by accept(2), we always lock the listening socket `head' first.
> This allows us to avoid unwanted relock within accept(2) syscall.
>
> ok claudio@
syzkaller found a panic, I'm spotting one missed unlock. Feel free to
commit.
diff --git sys/kern/uipc_usrreq.c sys/kern/uipc_usrreq.c
index 0710393d376..7231b16b735 100644
--- sys/kern/uipc_usrreq.c
+++ sys/kern/uipc_usrreq.c
@@ -363,6 +363,7 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m,
struct mbuf *nam,
control = NULL;
} else {
error = ENOBUFS;
+ sounlock(so2);
break;
}
} else if (so->so_type == SOCK_SEQPACKET)
syzbot has found a reproducer for the following issue on:
HEAD commit:a4a82b864d54 Remove PIPEXCSESSION ioctl(2) call only from ..
git tree: openbsd
console output: https://syzkaller.appspot.com/x/log.txt?x=10cffe2408
kernel config: https://syzkaller.appspot.com/x/.config?x=7058272de1526588
dashboard link: https://syzkaller.appspot.com/bug?extid=a648408d6a58fd40b59a
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15cc61f408
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=178e44ec08
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+a648408d6a58fd40b...@syzkaller.appspotmail.com
witness: userret: returning with the following locks held:
exclusive rwlock solock r = 0 (0xfd8073dde420)
#0 witness_lock+0x44d
#1 unp_solock_peer+0x64 sys/kern/uipc_usrreq.c:168
#2 uipc_usrreq+0x7c6 sys/kern/uipc_usrreq.c:350
#3 sosend+0x61b sys/kern/uipc_socket.c:657
#4 sendit+0x65d sys/kern/uipc_syscalls.c:682
#5 sys_sendmsg+0x198 sys/kern/uipc_syscalls.c:589
#6 syscall+0x4c3 mi_syscall sys/sys/syscall_mi.h:101 [inline]
#6 syscall+0x4c3 sys/arch/amd64/amd64/trap.c:585
#7 Xsyscall+0x128
panic: witness_warn
Stopped at db_enter+0x18: addq$0x8,%rsp
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
* 35921 8907 0 0x2 00 syz-executor3905931047
442241 79361 73 0x1100010 01 syslogd
db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
panic(82593e80) at panic+0x177 sys/kern/subr_prf.c:202
witness_warn(2,0,82620d13) at witness_warn+0x65e witness_debugger
sys/kern/subr_witness.c:2505 [inline]
witness_warn(2,0,82620d13) at witness_warn+0x65e
sys/kern/subr_witness.c:1473
userret(800021233268) at userret+0x265 sys/kern/kern_sig.c:2012
syscall(8000212825b0) at syscall+0x57e mi_syscall_return
sys/sys/syscall_mi.h:128 [inline]
syscall(8000212825b0) at syscall+0x57e sys/arch/amd64/amd64/trap.c:607
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7f0b10, count: 9
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports. Insufficient info makes it difficult to find and fix bugs.
ddb{0}>
ddb{0}> set $lines = 0
ddb{0}> set $maxwidth = 0
ddb{0}> show panic
*cpu0: witness_warn
ddb{0}> trace
db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
panic(82593e80) at panic+0x177 sys/kern/subr_prf.c:202
witness_warn(2,0,82620d13) at witness_warn+0x65e witness_debugger
sys/kern/subr_witness.c:2505 [inline]
witness_warn(2,0,82620d13) at witness_warn+0x65e
sys/kern/subr_witness.c:1473
userret(800021233268) at userret+0x265 sys/kern/kern_sig.c:2012
syscall(8000212825b0) at syscall+0x57e
Re: CVS: cvs.openbsd.org: src
On 29/06/22(Wed) 07:02, Anton Lindqvist wrote:
> On Tue, Jun 28, 2022 at 06:08:18AM -0600, Claudio Jeker wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: clau...@cvs.openbsd.org 2022/06/28 06:08:18
> >
> > Modified files:
> > sys/kern : kern_sig.c
> >
> > Log message:
> > Cleanup the sleep loop in single_thread_check_locked(). The deep checks
> > only matter on entry and the pr->ps_single check is done in the while body.
> > With and OK mpi@
>
> syzkaller just found a reproducer for the following panic. Is it related
> to this commit?
I believe it is. If there's a race between SINGLE_SUSPEND and
PS_SINGLEEXIT the deep check should be re-done.
The problem reported by syzkaller is a lock ordering issue. The SCHED_LOCK()
is held when trying to grab the KERNEL_LOCK(). This can only happen if
a thread is calling exit1() inside single_thread_check_locked() but that
should only be possible at the boundary of the kernel not in the middle
of a sleep.
So I believe this change was wrong and we should revert it. Thanks!
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit:37c734d33dee constify miscellaneous arm64 pin and clock ta..
> > git tree: openbsd
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1397129808
> > kernel config: https://syzkaller.appspot.com/x/.config?x=7058272de1526588
> > dashboard link: https://syzkaller.appspot.com/bug?extid=f7634539e73108238c2a
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17fe7ac008
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1625358808
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+f7634539e73108238...@syzkaller.appspotmail.com
> >
> > panic: kernel diagnostic assertion "__mp_lock_held(_lock, curcpu())
> > == 0" failed: file
> > "/syzkaller/managers/setuid/kernel/sys/kern/kern_lock.c", line 63
> > Stopped at db_enter+0x18: addq$0x8,%rsp
> > TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> > *226148 13902 0 0x1802 0x40800801 syz-executor2883823057
> > db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
> > panic(8259bd82) at panic+0x177 sys/kern/subr_prf.c:202
> > __assert(8261504d,8261b5a3,3f,8264d4fb) at
> > __assert+0x25 sys/kern/subr_prf.c:161
> > _kernel_lock() at _kernel_lock+0xb2 sys/kern/kern_lock.c:63
> > single_thread_check_locked(800021232548,1,c) at
> > single_thread_check_locked+0x1f6 sys/kern/kern_sig.c:2037
> > single_thread_check(800021232548,1) at single_thread_check+0x4b
> > sys/kern/kern_sig.c:2057
> > sleep_finish(8000212c2550,1) at sleep_finish+0x75 sleep_signal_check
> > sys/kern/kern_synch.c:464 [inline]
> > sleep_finish(8000212c2550,1) at sleep_finish+0x75
> > sys/kern/kern_synch.c:400
> > rwsleep(800021232548,82957750,120,82595e88,0) at
> > rwsleep+0xd5 sys/kern/kern_synch.c:314
> > futex_wait(bc96a93640,1,0,2) at futex_wait+0x13c sys/kern/sys_futex.c:260
> > sys_futex(800021232548,8000212c26e0,8000212c2740) at
> > sys_futex+0xf8 sys/kern/sys_futex.c:111
> > syscall(8000212c27b0) at syscall+0x484 mi_syscall
> > sys/sys/syscall_mi.h:102 [inline]
> > syscall(8000212c27b0) at syscall+0x484 sys/arch/amd64/amd64/trap.c:585
> > Xsyscall() at Xsyscall+0x128
> > end of kernel
> > end trace frame: 0xbcc5099500, count: 3
> > https://www.openbsd.org/ddb.html describes the minimum info required in bug
> > reports. Insufficient info makes it difficult to find and fix bugs.
> > ddb{1}>
> > ddb{1}> set $lines = 0
> > ddb{1}> set $maxwidth = 0
> > ddb{1}> show panic
> > *cpu1: kernel diagnostic assertion "__mp_lock_held(_lock, curcpu())
> > == 0" failed: file
> > "/syzkaller/managers/setuid/kernel/sys/kern/kern_lock.c", line 63
> > ddb{1}> trace
> > db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
> > panic(8259bd82) at panic+0x177 sys/kern/subr_prf.c:202
> > __assert(8261504d,8261b5a3,3f,8264d4fb) at
> > __assert+0x25 sys/kern/subr_prf.c:161
> > _kernel_lock() at _kernel_lock+0xb2 sys/kern/kern_lock.c:63
> > single_thread_check_locked(800021232548,1,c) at
> > single_thread_check_locked+0x1f6 sys/kern/kern_sig.c:2037
> > single_thread_check(800021232548,1) at single_thread_check+0x4b
> > sys/kern/kern_sig.c:2057
> > sleep_finish(8000212c2550,1) at sleep_finish+0x75 sleep_signal_check
> > sys/kern/kern_synch.c:464 [inline]
> > sleep_finish(8000212c2550,1) at sleep_finish+0x75
> > sys/kern/kern_synch.c:400
> > rwsleep(800021232548,82957750,120,82595e88,0) at
> > rwsleep+0xd5 sys/kern/kern_synch.c:314
> > futex_wait(bc96a93640,1,0,2) at futex_wait+0x13c sys/kern/sys_futex.c:260
> > sys_futex(800021232548,8000212c26e0,8000212c2740) at
> > sys_futex+0xf8 sys/kern/sys_futex.c:111
> >
Re: CVS: cvs.openbsd.org: src
On Tue, Jun 28, 2022 at 06:08:18AM -0600, Claudio Jeker wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: clau...@cvs.openbsd.org 2022/06/28 06:08:18
>
> Modified files:
> sys/kern : kern_sig.c
>
> Log message:
> Cleanup the sleep loop in single_thread_check_locked(). The deep checks
> only matter on entry and the pr->ps_single check is done in the while body.
> With and OK mpi@
syzkaller just found a reproducer for the following panic. Is it related
to this commit?
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit:37c734d33dee constify miscellaneous arm64 pin and clock ta..
> git tree: openbsd
> console output: https://syzkaller.appspot.com/x/log.txt?x=1397129808
> kernel config: https://syzkaller.appspot.com/x/.config?x=7058272de1526588
> dashboard link: https://syzkaller.appspot.com/bug?extid=f7634539e73108238c2a
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17fe7ac008
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1625358808
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+f7634539e73108238...@syzkaller.appspotmail.com
>
> panic: kernel diagnostic assertion "__mp_lock_held(_lock, curcpu()) ==
> 0" failed: file "/syzkaller/managers/setuid/kernel/sys/kern/kern_lock.c",
> line 63
> Stopped at db_enter+0x18: addq$0x8,%rsp
> TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> *226148 13902 0 0x1802 0x40800801 syz-executor2883823057
> db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
> panic(8259bd82) at panic+0x177 sys/kern/subr_prf.c:202
> __assert(8261504d,8261b5a3,3f,8264d4fb) at
> __assert+0x25 sys/kern/subr_prf.c:161
> _kernel_lock() at _kernel_lock+0xb2 sys/kern/kern_lock.c:63
> single_thread_check_locked(800021232548,1,c) at
> single_thread_check_locked+0x1f6 sys/kern/kern_sig.c:2037
> single_thread_check(800021232548,1) at single_thread_check+0x4b
> sys/kern/kern_sig.c:2057
> sleep_finish(8000212c2550,1) at sleep_finish+0x75 sleep_signal_check
> sys/kern/kern_synch.c:464 [inline]
> sleep_finish(8000212c2550,1) at sleep_finish+0x75
> sys/kern/kern_synch.c:400
> rwsleep(800021232548,82957750,120,82595e88,0) at
> rwsleep+0xd5 sys/kern/kern_synch.c:314
> futex_wait(bc96a93640,1,0,2) at futex_wait+0x13c sys/kern/sys_futex.c:260
> sys_futex(800021232548,8000212c26e0,8000212c2740) at
> sys_futex+0xf8 sys/kern/sys_futex.c:111
> syscall(8000212c27b0) at syscall+0x484 mi_syscall
> sys/sys/syscall_mi.h:102 [inline]
> syscall(8000212c27b0) at syscall+0x484 sys/arch/amd64/amd64/trap.c:585
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0xbcc5099500, count: 3
> https://www.openbsd.org/ddb.html describes the minimum info required in bug
> reports. Insufficient info makes it difficult to find and fix bugs.
> ddb{1}>
> ddb{1}> set $lines = 0
> ddb{1}> set $maxwidth = 0
> ddb{1}> show panic
> *cpu1: kernel diagnostic assertion "__mp_lock_held(_lock, curcpu()) ==
> 0" failed: file "/syzkaller/managers/setuid/kernel/sys/kern/kern_lock.c",
> line 63
> ddb{1}> trace
> db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
> panic(8259bd82) at panic+0x177 sys/kern/subr_prf.c:202
> __assert(8261504d,8261b5a3,3f,8264d4fb) at
> __assert+0x25 sys/kern/subr_prf.c:161
> _kernel_lock() at _kernel_lock+0xb2 sys/kern/kern_lock.c:63
> single_thread_check_locked(800021232548,1,c) at
> single_thread_check_locked+0x1f6 sys/kern/kern_sig.c:2037
> single_thread_check(800021232548,1) at single_thread_check+0x4b
> sys/kern/kern_sig.c:2057
> sleep_finish(8000212c2550,1) at sleep_finish+0x75 sleep_signal_check
> sys/kern/kern_synch.c:464 [inline]
> sleep_finish(8000212c2550,1) at sleep_finish+0x75
> sys/kern/kern_synch.c:400
> rwsleep(800021232548,82957750,120,82595e88,0) at
> rwsleep+0xd5 sys/kern/kern_synch.c:314
> futex_wait(bc96a93640,1,0,2) at futex_wait+0x13c sys/kern/sys_futex.c:260
> sys_futex(800021232548,8000212c26e0,8000212c2740) at
> sys_futex+0xf8 sys/kern/sys_futex.c:111
> syscall(8000212c27b0) at syscall+0x484 mi_syscall
> sys/sys/syscall_mi.h:102 [inline]
> syscall(8000212c27b0) at syscall+0x484 sys/arch/amd64/amd64/trap.c:585
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0xbcc5099500, count: -12
> ddb{1}> show registers
> rdi0
> rsi 0x1
> rbp 0x8000212c2310
> rbx 0x800020dd9bb7
> rdx0x3fd
> rcx0
> rax 0x9f
> r8 0x101010101010101
> r90x8080808080808080
> r10 0x3b3e1894665f128c
> r11 0x7302afdd430d6aad
> r12
Re: CVS: cvs.openbsd.org: src
For bgplg in www chroot, I think -- Sent from a phone, apologies for poor formatting. On 28 June 2022 07:50:06 Theo Buehler wrote: On Tue, Jun 28, 2022 at 06:45:30AM +0200, Anton Lindqvist wrote: On Mon, Jun 27, 2022 at 07:27:39AM -0600, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by:clau...@cvs.openbsd.org 2022/06/27 07:27:38 > > Modified files: >usr.sbin/bgpctl: bgpctl.c output.c output_json.c > > Log message: > bgpctl bits for RFC 9234 support. > OK tb@ This broke the tree over here, looks like the log_policy() implementation is missing. Fixed, thanks. cc -static -pie -o bgpctl bgpctl.o output.o output_json.o parser.o mrtparser.o util.o json.o -lutil -lm Why does this link statically?
Re: CVS: cvs.openbsd.org: src
On Tue, Jun 28, 2022 at 06:45:30AM +0200, Anton Lindqvist wrote: > On Mon, Jun 27, 2022 at 07:27:39AM -0600, Claudio Jeker wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: clau...@cvs.openbsd.org 2022/06/27 07:27:38 > > > > Modified files: > > usr.sbin/bgpctl: bgpctl.c output.c output_json.c > > > > Log message: > > bgpctl bits for RFC 9234 support. > > OK tb@ > > This broke the tree over here, looks like the log_policy() > implementation is missing. Fixed, thanks. > cc -static -pie -o bgpctl bgpctl.o output.o output_json.o parser.o > mrtparser.o util.o json.o -lutil -lm Why does this link statically?
Re: CVS: cvs.openbsd.org: src
On Mon, Jun 27, 2022 at 07:27:39AM -0600, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2022/06/27 07:27:38 > > Modified files: > usr.sbin/bgpctl: bgpctl.c output.c output_json.c > > Log message: > bgpctl bits for RFC 9234 support. > OK tb@ This broke the tree over here, looks like the log_policy() implementation is missing. cc -static -pie -o bgpctl bgpctl.o output.o output_json.o parser.o mrtparser.o util.o json.o -lutil -lm ld: error: undefined symbol: log_policy >>> referenced by output.c >>> output.o:(show_neighbor) >>> referenced by output.c >>> output.o:(show_neighbor) >>> referenced by output.c >>> output.o:(show_neighbor) >>> referenced 2 more times cc: error: linker command failed with exit code 1 (use -v to see invocation) *** Error 1 in usr.bin/bgplg/bgpctl (:126 'bgpctl') *** Error 2 in usr.bin/bgplg (:48 'all') *** Error 2 in usr.bin (:48 'all') *** Error 2 in . (:48 'all') *** Error 2 in . (Makefile:97 'do-build') *** Error 2 in /home/src2 (Makefile:74 'build') robsd-exec: process group exited 2
Re: CVS: cvs.openbsd.org: src
On Mon, 23 May 2022 23:04:07 -0700, Philip Guenther wrote: > To be clear, this is _not_ a POSIX conformance issue and when multiple > possible error conditions occcur, as happens here, the exact error > returned is unspecified by POSIX: XSH 2.3p4 > If more than one error occurs in processing a function call, any one > of the possible errors may be returned, as the order of detection is > undefined. Right, callers need to be able to handle any of the potential error codes. I think adjusting the regress test in this case is fine. - todd
Re: CVS: cvs.openbsd.org: src
> CVSROOT: /cvs
> Module name: src
> Changes by: an...@cvs.openbsd.org 2022/05/23 23:14:30
>
> Modified files:
> regress/lib/libc/sys: t_truncate.c
>
> Log message:
> Recent changes to truncate(2) swapped the ordering of some validations
> causing EACCESS as opposed of ESDIR to be returned while trying to
> truncate a directory as a user lacking write permissions to the same
> directory. As this behavior is reasonable, change the truncate directory
> from /etc/ to /tmp which makes the test pass both as root and non-root.
Pretty certain that is a mistake in the kernel commit. A big part of the
discussion around that diff wasn't just fixing the rlimit problem, but
trying to make sure the order of evaluation of error was correct. Looks
like it is wrong..
I guess the VDIR check should be put back into the two callers, not the
helper function, which will prevent the VOP_ACCESS errno from being
returned.
Maybe something like this (untested)
Index: vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.357
diff -u -p -u -r1.357 vfs_syscalls.c
--- vfs_syscalls.c 23 May 2022 15:17:11 - 1.357
+++ vfs_syscalls.c 24 May 2022 05:30:59 -
@@ -2821,8 +2821,6 @@ dotruncate(struct proc *p, struct vnode
if (len < 0)
return EINVAL;
- if (vp->v_type == VDIR)
- return EISDIR;
if ((error = vn_writechk(vp)) != 0)
return error;
if (vp->v_type == VREG && len > lim_cur_proc(p, RLIMIT_FSIZE)) {
@@ -2860,7 +2858,9 @@ sys_truncate(struct proc *p, void *v, re
return (error);
vp = nd.ni_vp;
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
- if ((error = VOP_ACCESS(vp, VWRITE, p->p_ucred, p)) == 0)
+ if (vp->v_type == VDIR)
+ error = EISDIR;
+ else if ((error = VOP_ACCESS(vp, VWRITE, p->p_ucred, p)) == 0)
error = dotruncate(p, vp, SCARG(uap, length));
vput(vp);
return (error);
@@ -2888,7 +2888,10 @@ sys_ftruncate(struct proc *p, void *v, r
}
vp = fp->f_data;
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
- error = dotruncate(p, vp, SCARG(uap, length));
+ if (vp->v_type == VDIR)
+ error = EISDIR;
+ else
+ error = dotruncate(p, vp, SCARG(uap, length));
VOP_UNLOCK(vp);
bad:
FRELE(fp, p);
Anton Lindqvist wrote:
Re: CVS: cvs.openbsd.org: src
On Thu, 12 May 2022 18:17:20 -0600 (MDT) YASUOKA Masahiko wrote: > CVSROOT: /cvs > Module name: src > Changes by: yasu...@cvs.openbsd.org 2022/05/12 18:17:20 > > Modified files: > usr.sbin/vmctl : main.c vmctl.c vmctl.h > > Log message: > Follow the recent change that the unit of `imgsize' argument of > create_imagefile() became MB. Also change the arguement's type from > long to uint64_t that is preferred. Sorry first sentence has a false. The unit became byte actually.
Re: CVS: cvs.openbsd.org: src
On Fri, May 06, 2022 at 07:00:17AM +0200, Anton Lindqvist wrote: > On Wed, May 04, 2022 at 10:58:43PM -0600, Theo de Raadt wrote: > > These complaints about trivial regress failures, generally just > > output strings, are getting a bit tiring. > > You can fix it in about 1 minute, right? > > I generally try to fix trivial things like this one but I don't always > have the time. If people find this more annoying than helpful I will > stop. I think it's valuable that you do this. Something broke. Ideally, the one who broke it fixes it. If you don't have time to figure out a fix, it's perfectly fine to send a mail to the responsible person. However, I think the initial report doesn't necessarily need to be public.
Re: CVS: cvs.openbsd.org: src
On Wed, May 04, 2022 at 10:58:43PM -0600, Theo de Raadt wrote: > These complaints about trivial regress failures, generally just > output strings, are getting a bit tiring. > You can fix it in about 1 minute, right? I generally try to fix trivial things like this one but I don't always have the time. If people find this more annoying than helpful I will stop.
Re: CVS: cvs.openbsd.org: src
Anton Lindqvist writes:
> On Tue, May 03, 2022 at 03:39:19PM -0600, Dave Voutila wrote:
>> CVSROOT: /cvs
>> Module name: src
>> Changes by: d...@cvs.openbsd.org2022/05/03 15:39:19
>>
>> Modified files:
>> usr.sbin/vmd : parse.y vm.c vmd.h
>> usr.sbin/vmctl : main.c vmctl.c vmctl.h
>> sys/arch/amd64/amd64: vmm.c
>> sys/arch/amd64/include: vmmvar.h
>>
>> Log message:
>> vmm/vmd/vmctl: standardize memory units to bytes
>>
>> At different points in the vm lifecycle vmm(4), vmctl(8), and vmd(8)
>> refer to a vm's memory range sizes in either bytes or megabytes.
>> This is needlessly complex.
>>
>> Switch to using bytes everywhere and adjust types and constants
>> accordingly. While this makes it possible to specify vm's with
>> memory in fractions of megabytes, the logic requiring whole
>> megabyte values remains.
>>
>> Feedback from deraadt@, mlarkin@, and Matthew Martin.
>>
>> ok mlarkin@
>
> This broke regress.
>
>> usr.sbin/vmd:
> Exit: 1
> Duration: 00:00:03
> Log: 253-usr.sbin-vmd.log
>
> run-pass-memory-round
> /usr/sbin/vmd -n -f
> /home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.conf 2>&1
> | diff -u
> /home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.ok
> /dev/stdin
> --- /home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.okWed Oct
> 11 14:01:20 2017
> +++ /dev/stdinThu May 5 06:04:37 2022
> @@ -1,2 +1,2 @@
> -size rounded to 1023 megabytes
> +memory size rounded to 1023M
> configuration OK
> *** Error 1 in config (Makefile:15 'run-pass-memory-round')
> FAILED
>
> run-fail-too-few-ram
> /usr/sbin/vmd -n -f
> /home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.conf 2>&1 |
> cut -d : -f 2,3,4 | diff -u
> /home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok
> /dev/stdin
> --- /home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok Wed Oct
> 11 14:01:20 2017
> +++ /dev/stdinThu May 5 06:04:38 2022
> @@ -1,2 +1,2 @@
> -size must be at least one megabyte
> +memory size must be at least 1MB
> 4: failed to parse size: 1048575
> *** Error 1 in config (Makefile:22 'run-fail-too-few-ram')
> FAILED
>
This is fixed now though.
> run-regress-vioscribble
> rm -f scribble.{raw,qcow2} vioscribble.d vioqcow2.d vioraw.d log.d
> /usr/sbin/vmctl create -s 4G scribble.raw
> vmctl: create imagefile operation failed: File too large
> *** Error 1 in diskfmt (Makefile:24 'setup')
> FAILED
This was fixed in a subsequent commit after it was reported by a user.
-dv
Re: CVS: cvs.openbsd.org: src
These complaints about trivial regress failures, generally just output strings, are getting a bit tiring. You can fix it in about 1 minute, right?
Re: CVS: cvs.openbsd.org: src
On Tue, May 03, 2022 at 03:39:19PM -0600, Dave Voutila wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: d...@cvs.openbsd.org2022/05/03 15:39:19
>
> Modified files:
> usr.sbin/vmd : parse.y vm.c vmd.h
> usr.sbin/vmctl : main.c vmctl.c vmctl.h
> sys/arch/amd64/amd64: vmm.c
> sys/arch/amd64/include: vmmvar.h
>
> Log message:
> vmm/vmd/vmctl: standardize memory units to bytes
>
> At different points in the vm lifecycle vmm(4), vmctl(8), and vmd(8)
> refer to a vm's memory range sizes in either bytes or megabytes.
> This is needlessly complex.
>
> Switch to using bytes everywhere and adjust types and constants
> accordingly. While this makes it possible to specify vm's with
> memory in fractions of megabytes, the logic requiring whole
> megabyte values remains.
>
> Feedback from deraadt@, mlarkin@, and Matthew Martin.
>
> ok mlarkin@
This broke regress.
> usr.sbin/vmd:
Exit: 1
Duration: 00:00:03
Log: 253-usr.sbin-vmd.log
run-pass-memory-round
/usr/sbin/vmd -n -f
/home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.conf 2>&1 | diff
-u /home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.ok /dev/stdin
--- /home/src/regress/usr.sbin/vmd/config/vmd-pass-memory-round.ok Wed Oct
11 14:01:20 2017
+++ /dev/stdin Thu May 5 06:04:37 2022
@@ -1,2 +1,2 @@
-size rounded to 1023 megabytes
+memory size rounded to 1023M
configuration OK
*** Error 1 in config (Makefile:15 'run-pass-memory-round')
FAILED
run-fail-too-few-ram
/usr/sbin/vmd -n -f
/home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.conf 2>&1 | cut -d
: -f 2,3,4 | diff -u
/home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok /dev/stdin
--- /home/src/regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok Wed Oct
11 14:01:20 2017
+++ /dev/stdin Thu May 5 06:04:38 2022
@@ -1,2 +1,2 @@
-size must be at least one megabyte
+memory size must be at least 1MB
4: failed to parse size: 1048575
*** Error 1 in config (Makefile:22 'run-fail-too-few-ram')
FAILED
run-regress-vioscribble
rm -f scribble.{raw,qcow2} vioscribble.d vioqcow2.d vioraw.d log.d
/usr/sbin/vmctl create -s 4G scribble.raw
vmctl: create imagefile operation failed: File too large
*** Error 1 in diskfmt (Makefile:24 'setup')
FAILED
Re: CVS: cvs.openbsd.org: src
On Tue, May 03, 2022 at 07:32:47AM -0600, Alexandr Nedvedicky wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: sas...@cvs.openbsd.org 2022/05/03 07:32:47
>
> Modified files:
> sys/net: pf.c
>
> Log message:
> Make pf(4) more paranoid about IGMP/MLP messages. MLD/IGMP messages
> with ttl other than 1 will be discarded. Also MLD messages with
> other than link-local source address will be discarded. IGMP
> messages with destination address other than multicast class
> will be discarded.
>
> feedback and OK bluhm@, cluadio@
This most likely broke regress.
> sys/net/pf_opts:
Exit: 1
Duration: 00:01:32
Log: 153-sys-net-pf_opts.log
run-bpf-mcast
sleep 2 # XXX
doas -n pkill -f '^/usr/sbin/tcpdump -l -e -vvv -s 2048 -ni'
2 packets received by filter
0 packets dropped by kernel
4 packets received by filter
0 packets dropped by kernel
2 packets received by filter
0 packets dropped by kernel
rm -f stamp-bpf*
# Check that multicast protocol packet with router alert passed
grep ' 127.0.0.12: igmp query .* IPOPT-148{4}' lo12.tcpdump
05:00:20.575196 127.0.0.12 > 127.0.0.12: igmp query [ttl 1] (id 1, len 32,
optlen=4 IPOPT-148{4})
grep ' fe80::12: HBH (rtalert:.* icmp6: multicast ' lo12.tcpdump
05:00:23.921469 fe80::12 > fe80::12: HBH (rtalert: 0x) icmp6: multicast
listener query max resp delay: 1 addr: :: [icmp6 cksum ok] (len 32, hlim 64)
! grep '127.0.0.11' pflog0.tcpdump
05:00:18.943109 rule def/(ip-option) [uid 0, pid 0] pass in on lo11: 127.0.0.11
> 127.0.0.11: igmp query [ttl 1] (id 1, len 32, optlen=4 IPOPT-148{4})
*** Error 1 in . (Makefile:321 'run-bpf-mcast')
FAILED
run-bpf-mcast-bad
sleep 2 # XXX
doas -n pkill -f '^/usr/sbin/tcpdump -l -e -vvv -s 2048 -ni'
2 packets received by filter
0 packets dropped by kernel
4 packets received by filter
0 packets dropped by kernel
2 packets received by filter
0 packets dropped by kernel
rm -f stamp-bpf*
# Check that multicast protocol packet with options were blocked
grep ' 127.0.0.12: igmp query .* IPOPT-3{4}' pflog0.tcpdump
05:00:31.505047 rule def/(ip-option) [uid 0, pid 0] pass in on lo12: 127.0.0.12
> 127.0.0.12: igmp query [ttl 1] (id 1, len 32, optlen=4 IPOPT-3{4})
grep ' fe80::12: HBH (type 0x03:.* icmp6: multicast ' pflog0.tcpdump
05:00:34.842050 rule def/(ip-option) [uid 0, pid 0] pass in on lo12: fe80::12 >
fe80::12: HBH (type 0x03: len=0) icmp6: multicast listener query max resp
delay: 1 addr: :: [icmp6 cksum ok] (len 32, hlim 64)
! grep '127.0.0.11' pflog0.tcpdump
05:00:29.868675 rule def/(ip-option) [uid 0, pid 0] pass in on lo11: 127.0.0.11
> 127.0.0.11: igmp query [ttl 1] (id 1, len 32, optlen=4 IPOPT-3{4})
*** Error 1 in . (Makefile:341 'run-bpf-mcast-bad')
FAILED
Re: CVS: cvs.openbsd.org: src
On 28/04/22(Thu) 12:12, Martin Pieuchot wrote: > CVSROOT: /cvs > Module name: src > Changes by: m...@cvs.openbsd.org2022/04/28 12:12:33 > > Modified files: > sys/uvm: uvm_vnode.c > > Log message: > Always acquire the `vmobjlock' before incrementing an object's reference. With and ok semarie.
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 20, 2022 at 03:58:19AM -0600, Marc Espie wrote: > CVSROOT: /cvs > Module name: src > Changes by: es...@cvs.openbsd.org 2022/04/20 03:58:19 > > Modified files: > regress/usr.sbin/pkg_add: Makefile > > Log message: > add a symlink test that currently fails, I want to fix it soonish Please make use of REGRESS_EXPECTED_FAILURES, it makes the process of tracking down actual regressions easier (for me). http://man.openbsd.org/bsd.regress.mk#REGRESS_EXPECTED_FAILURES
Re: CVS: cvs.openbsd.org: src
On Tue, Apr 19, 2022 at 10:40:34PM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2022/04/19 22:40:34 > > Modified files: > usr.sbin/rpki-client: main.c > > Log message: > Remove an unreachable error message > > If timeout != 0 and 0 <= timeout <= 24*60*60, then timeout < 1 is > impossible. > > ok > That was supposed to be ok claudio
Re: CVS: cvs.openbsd.org: src
Alexander Bluhm writes: > On Fri, Apr 08, 2022 at 07:49:43PM -0600, Dave Voutila wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: d...@cvs.openbsd.org2022/04/08 19:49:43 >> >> Modified files: >> sys/arch/amd64/amd64: vmm_support.S >> >> Log message: >> Simplify vmx instruction error handling. >> >> Removes jumps and shortens the code while keeping functionality the >> same. >> >> ok mlarkin@ > > This commit breaks vmm on my ThinkPad T430s. > > cpu1: failed to enter VMM mode > cpu0: failed to enter VMM mode > > cpu0: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz, 2893.83 MHz, 06-3a-09 > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN > cpu0: 256KB 64b/line 8-way L2 cache > > I think the problem is that XOR affects the zero flag. Does it? > This diff fixes it. > > ok? Yes, looks like my fault here. Odd that my testing didn't trigger the issue. ok dv@ > > bluhm > > Index: arch/amd64/amd64/vmm_support.S > === > RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/vmm_support.S,v > retrieving revision 1.19 > diff -u -p -r1.19 vmm_support.S > --- arch/amd64/amd64/vmm_support.S9 Apr 2022 01:49:43 - 1.19 > +++ arch/amd64/amd64/vmm_support.S13 Apr 2022 17:10:42 - > @@ -62,8 +62,8 @@ _C_LABEL(vmm_dispatch_intr): > > _C_LABEL(vmxon): > RETGUARD_SETUP(vmxon, r11) > - vmxon (%rdi) > xorq%rax, %rax > + vmxon (%rdi) > setna %al > RETGUARD_CHECK(vmxon, r11) > ret > @@ -71,8 +71,8 @@ _C_LABEL(vmxon): > > _C_LABEL(vmxoff): > RETGUARD_SETUP(vmxoff, r11) > - vmxoff > xorq%rax, %rax > + vmxoff > setna %al > RETGUARD_CHECK(vmxoff, r11) > ret > @@ -80,8 +80,8 @@ _C_LABEL(vmxoff): > > _C_LABEL(vmclear): > RETGUARD_SETUP(vmclear, r11) > - vmclear (%rdi) > xorq%rax, %rax > + vmclear (%rdi) > setna %al > RETGUARD_CHECK(vmclear, r11) > ret > @@ -89,8 +89,8 @@ _C_LABEL(vmclear): > > _C_LABEL(vmptrld): > RETGUARD_SETUP(vmptrld, r11) > - vmptrld (%rdi) > xorq%rax, %rax > + vmptrld (%rdi) > setna %al > RETGUARD_CHECK(vmptrld, r11) > ret > @@ -98,8 +98,8 @@ _C_LABEL(vmptrld): > > _C_LABEL(vmptrst): > RETGUARD_SETUP(vmptrst, r11) > - vmptrst (%rdi) > xorq%rax, %rax > + vmptrst (%rdi) > setna %al > RETGUARD_CHECK(vmptrst, r11) > ret > @@ -107,8 +107,8 @@ _C_LABEL(vmptrst): > > _C_LABEL(vmwrite): > RETGUARD_SETUP(vmwrite, r11) > - vmwrite %rsi, %rdi > xorq%rax, %rax > + vmwrite %rsi, %rdi > setna %al > RETGUARD_CHECK(vmwrite, r11) > ret > @@ -116,8 +116,8 @@ _C_LABEL(vmwrite): > > _C_LABEL(vmread): > RETGUARD_SETUP(vmread, r11) > - vmread %rdi, (%rsi) > xorq%rax, %rax > + vmread %rdi, (%rsi) > setna %al > RETGUARD_CHECK(vmread, r11) > ret -- -Dave Voutila
Re: CVS: cvs.openbsd.org: src
On 14.4.2022. 20:55, hrvoje wrote: > On 13.4.2022. 19:22, Alexander Bluhm wrote: >> On Fri, Apr 08, 2022 at 07:49:43PM -0600, Dave Voutila wrote: >>> CVSROOT:/cvs >>> Module name:src >>> Changes by: d...@cvs.openbsd.org2022/04/08 19:49:43 >>> >>> Modified files: >>> sys/arch/amd64/amd64: vmm_support.S >>> >>> Log message: >>> Simplify vmx instruction error handling. >>> >>> Removes jumps and shortens the code while keeping functionality the >>> same. >>> >>> ok mlarkin@ >> >> This commit breaks vmm on my ThinkPad T430s. >> >> cpu1: failed to enter VMM mode >> cpu0: failed to enter VMM mode >> >> cpu0: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz, 2893.83 MHz, 06-3a-09 >> cpu0: >> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN >> cpu0: 256KB 64b/line 8-way L2 cache >> >> I think the problem is that XOR affects the zero flag. Does it? >> This diff fixes it. >> > > Hi, > > I don't know if this commit broke things but i'm having same errors as > bluhm@ and vm's doesn't come up . > > login: cpu9: failed to enter VMM mode > cpu8: failed to enter VMM mode > cpu7: failed to enter VMM mode > cpu6: failed to enter VMM mode > cpu5: failed to enter VMM mode > cpu4: failed to enter VMM mode > cpu3: failed to enter VMM mode > cpu1: failed to enter VMM mode > cpu2: failed to enter VMM mode > cpu11: failed to enter VMM mode > cpu10: failed to enter VMM mode > cpu9: failed to enter VMM mode > cpu8: failed to enter VMM mode > cpu7: failed to enter VMM mode > cpu6: failed to enter VMM mode > cpu5: failed to enter VMM mode > cpu4: failed to enter VMM mode > cpu3: failed to enter VMM mode > cpu2: failed to enter VMM mode > cpu1: failed to enter VMM mode > cpu0: failed to enter VMM mode > smr: dispatch took 11.170215s > cpu0: failed to enter VMM mode > cpu11: failed to enter VMM mode > cpu10: failed to enter VMM mode > cpu9: failed to enter VMM mode > cpu8: failed to enter VMM mode > cpu7: failed to enter VMM mode > cpu6: failed to enter VMM mode > cpu5: failed to enter VMM mode > cpu3: failed to enter VMM mode > cpu2: failed to enter VMM mode > cpu1: failed to enter VMM mode > cpu4: failed to enter VMM mode > cpu0: failed to enter VMM mode > cpu11: failed to enter VMM mode > cpu10: failed to enter VMM mode > cpu9: failed to enter VMM mode > cpu8: failed to enter VMM mode > cpu7: failed to enter VMM mode > cpu6: failed to enter VMM mode > cpu4: failed to enter VMM mode > cpu3: failed to enter VMM mode > cpu2: failed to enter VMM mode > cpu1: failed to enter VMM mode > cpu5: failed to enter VMM mode > > with bluhm@ diff vmm seems good as before .. dmesg OpenBSD 7.1-current (GENERIC.MP) #22: Thu Apr 14 21:15:30 CEST 2022 hrv...@x3550m4.srce.hr:/sys/arch/amd64/compile/GENERIC.MP real mem = 34297651200 (32708MB) avail mem = 33240903680 (31700MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x7e67b000 (84 entries) bios0: vendor IBM version "-[D7E174BUS-3.20]-" date 06/15/2020 bios0: IBM IBM System x3550 M4 -[791425Z]- acpi0 at bios0: ACPI 4.0 acpi0: sleep states S0 S5 acpi0: tables DSDT FACP TCPA ERST HEST HPET APIC MCFG OEM1 SLIT SLIC SSDT SSDT SSDT SSDT DMAR acpi0: wakeup devices MRP1(S4) DCC0(S4) MRP3(S4) MRP5(S4) EHC2(S5) PEX0(S5) PEX7(S5) EHC1(S5) IP2P(S3) MRPB(S4) MRPC(S4) MRPD(S4) MRPF(S4) MRPG(S4) MRPH(S4) MRPI(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.39 MHz, 06-3e-04 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 100MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.01 MHz, 06-3e-04 cpu1:
Re: CVS: cvs.openbsd.org: src
On 13.4.2022. 19:22, Alexander Bluhm wrote: > On Fri, Apr 08, 2022 at 07:49:43PM -0600, Dave Voutila wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: d...@cvs.openbsd.org2022/04/08 19:49:43 >> >> Modified files: >> sys/arch/amd64/amd64: vmm_support.S >> >> Log message: >> Simplify vmx instruction error handling. >> >> Removes jumps and shortens the code while keeping functionality the >> same. >> >> ok mlarkin@ > > This commit breaks vmm on my ThinkPad T430s. > > cpu1: failed to enter VMM mode > cpu0: failed to enter VMM mode > > cpu0: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz, 2893.83 MHz, 06-3a-09 > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN > cpu0: 256KB 64b/line 8-way L2 cache > > I think the problem is that XOR affects the zero flag. Does it? > This diff fixes it. > Hi, I don't know if this commit broke things but i'm having same errors as bluhm@ and vm's doesn't come up . login: cpu9: failed to enter VMM mode cpu8: failed to enter VMM mode cpu7: failed to enter VMM mode cpu6: failed to enter VMM mode cpu5: failed to enter VMM mode cpu4: failed to enter VMM mode cpu3: failed to enter VMM mode cpu1: failed to enter VMM mode cpu2: failed to enter VMM mode cpu11: failed to enter VMM mode cpu10: failed to enter VMM mode cpu9: failed to enter VMM mode cpu8: failed to enter VMM mode cpu7: failed to enter VMM mode cpu6: failed to enter VMM mode cpu5: failed to enter VMM mode cpu4: failed to enter VMM mode cpu3: failed to enter VMM mode cpu2: failed to enter VMM mode cpu1: failed to enter VMM mode cpu0: failed to enter VMM mode smr: dispatch took 11.170215s cpu0: failed to enter VMM mode cpu11: failed to enter VMM mode cpu10: failed to enter VMM mode cpu9: failed to enter VMM mode cpu8: failed to enter VMM mode cpu7: failed to enter VMM mode cpu6: failed to enter VMM mode cpu5: failed to enter VMM mode cpu3: failed to enter VMM mode cpu2: failed to enter VMM mode cpu1: failed to enter VMM mode cpu4: failed to enter VMM mode cpu0: failed to enter VMM mode cpu11: failed to enter VMM mode cpu10: failed to enter VMM mode cpu9: failed to enter VMM mode cpu8: failed to enter VMM mode cpu7: failed to enter VMM mode cpu6: failed to enter VMM mode cpu4: failed to enter VMM mode cpu3: failed to enter VMM mode cpu2: failed to enter VMM mode cpu1: failed to enter VMM mode cpu5: failed to enter VMM mode
Re: CVS: cvs.openbsd.org: src
On Fri, Apr 08, 2022 at 07:49:43PM -0600, Dave Voutila wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2022/04/08 19:49:43 > > Modified files: > sys/arch/amd64/amd64: vmm_support.S > > Log message: > Simplify vmx instruction error handling. > > Removes jumps and shortens the code while keeping functionality the > same. > > ok mlarkin@ This commit breaks vmm on my ThinkPad T430s. cpu1: failed to enter VMM mode cpu0: failed to enter VMM mode cpu0: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz, 2893.83 MHz, 06-3a-09 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache I think the problem is that XOR affects the zero flag. Does it? This diff fixes it. ok? bluhm Index: arch/amd64/amd64/vmm_support.S === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/vmm_support.S,v retrieving revision 1.19 diff -u -p -r1.19 vmm_support.S --- arch/amd64/amd64/vmm_support.S 9 Apr 2022 01:49:43 - 1.19 +++ arch/amd64/amd64/vmm_support.S 13 Apr 2022 17:10:42 - @@ -62,8 +62,8 @@ _C_LABEL(vmm_dispatch_intr): _C_LABEL(vmxon): RETGUARD_SETUP(vmxon, r11) - vmxon (%rdi) xorq%rax, %rax + vmxon (%rdi) setna %al RETGUARD_CHECK(vmxon, r11) ret @@ -71,8 +71,8 @@ _C_LABEL(vmxon): _C_LABEL(vmxoff): RETGUARD_SETUP(vmxoff, r11) - vmxoff xorq%rax, %rax + vmxoff setna %al RETGUARD_CHECK(vmxoff, r11) ret @@ -80,8 +80,8 @@ _C_LABEL(vmxoff): _C_LABEL(vmclear): RETGUARD_SETUP(vmclear, r11) - vmclear (%rdi) xorq%rax, %rax + vmclear (%rdi) setna %al RETGUARD_CHECK(vmclear, r11) ret @@ -89,8 +89,8 @@ _C_LABEL(vmclear): _C_LABEL(vmptrld): RETGUARD_SETUP(vmptrld, r11) - vmptrld (%rdi) xorq%rax, %rax + vmptrld (%rdi) setna %al RETGUARD_CHECK(vmptrld, r11) ret @@ -98,8 +98,8 @@ _C_LABEL(vmptrld): _C_LABEL(vmptrst): RETGUARD_SETUP(vmptrst, r11) - vmptrst (%rdi) xorq%rax, %rax + vmptrst (%rdi) setna %al RETGUARD_CHECK(vmptrst, r11) ret @@ -107,8 +107,8 @@ _C_LABEL(vmptrst): _C_LABEL(vmwrite): RETGUARD_SETUP(vmwrite, r11) - vmwrite %rsi, %rdi xorq%rax, %rax + vmwrite %rsi, %rdi setna %al RETGUARD_CHECK(vmwrite, r11) ret @@ -116,8 +116,8 @@ _C_LABEL(vmwrite): _C_LABEL(vmread): RETGUARD_SETUP(vmread, r11) - vmread %rdi, (%rsi) xorq%rax, %rax + vmread %rdi, (%rsi) setna %al RETGUARD_CHECK(vmread, r11) ret
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 06, 2022 at 12:59:30PM -0600, Christian Weisgerber wrote: > CVSROOT: /cvs > Module name: src > Changes by: na...@cvs.openbsd.org 2022/04/06 12:59:30 > > Modified files: > sys/arch/arm64/dev: acpiiort.c apldart.c apldog.c aplhidev.c > aplintc.c aplns.c aplpcie.c aplpinctrl.c > aplpmu.c aplspi.c aplspmi.c smmu_acpi.c > smmu_fdt.c > sys/arch/armv7/omap: omclock.c omcm.c omrng.c omsysc.c > sys/arch/landisk/dev: obio.c power.c wdc_obio.c > sys/arch/landisk/landisk: mainbus.c > sys/arch/loongson/dev: apm.c gdiumiic.c glxclk.c mcclock_isa.c > ohci_voyager.c > sys/arch/luna88k/cbus: i82365_cbus.c if_ne_cbus.c necsb.c > sys/arch/luna88k/dev: spc.c timekeeper.c > sys/arch/macppc/dev: if_mc.c > sys/arch/mips64/mips64: clock.c cpu.c > sys/arch/octeon/dev: amdcf.c cn30xxgmx.c cn30xxuart.c octcf.c >octeon_iobus.c octrng.c octrtc.c > sys/arch/powerpc64/dev: astfb.c ipmi_opal.c mainbus.c opal.c > opalcons.c opalsens.c phb.c xicp.c > xics.c xive.c > sys/arch/powerpc64/powerpc64: cpu.c > sys/arch/riscv64/dev: plic.c riscv_cpu_intc.c sfclock.c sfuart.c > simplebus.c > sys/arch/riscv64/riscv64: cpu.c > sys/arch/sh/dev: scif.c shb.c shpcic.c > sys/arch/sh/sh : cpu.c > sys/dev: ipmi.c midi.c radio.c rd.c softraid.c video.c >vscsi.c > sys/dev/acpi : abl.c acpiac.c acpiasus.c acpibat.c acpibtn.c >acpicbkbd.c acpicmos.c acpicpu.c acpidmar.c >acpidock.c acpiec.c acpige.c acpihid.c >acpihpet.c acpihve.c acpimadt.c acpimcfg.c >acpiprt.c acpipwrres.c acpisony.c acpisurface.c >acpithinkpad.c acpitimer.c acpitoshiba.c >acpitz.c acpivideo.c acpivout.c ahci_acpi.c >amdgpio.c aplgpio.c atk0110.c bytgpio.c >ccp_acpi.c ccpmic.c chvgpio.c com_acpi.c >dwgpio.c dwiic_acpi.c ehci_acpi.c glkgpio.c >if_bse_acpi.c imxiic_acpi.c ipmi_acpi.c >ohci_acpi.c pchgpio.c pluart_acpi.c sdhc_acpi.c >tipmic.c tpm.c xhci_acpi.c > sys/dev/adb: akbd.c ams.c > sys/dev/ata: wd.c > sys/dev/atapiscsi: atapiscsi.c > sys/dev/cardbus: cardbus.c cardslot.c com_cardbus.c >ehci_cardbus.c if_acx_cardbus.c >if_ath_cardbus.c if_athn_cardbus.c >if_atw_cardbus.c if_bwi_cardbus.c >if_dc_cardbus.c if_fxp_cardbus.c >if_malo_cardbus.c if_pgt_cardbus.c >if_ral_cardbus.c if_re_cardbus.c >if_rl_cardbus.c if_rtw_cardbus.c >if_xl_cardbus.c ohci_cardbus.c puc_cardbus.c >uhci_cardbus.c > sys/dev/eisa : ahc_eisa.c cac_eisa.c eisa.c if_ep_eisa.c >uha_eisa.c > sys/dev/fdt: amlpwrc.c bcm2711_pcie.c bcm2711_rng.c >bcm2711_tmon.c bcm2835_bsc.c bcm2835_clock.c >bcm2835_dmac.c bcm2835_gpio.c bcm2835_mbox.c >bcm2835_sdhost.c bd718x7.c cwfg.c dapmic.c >es8316ac.c exuart.c gfrtc.c gpiocharger.c >gpioleds.c graphaudio.c if_bse_fdt.c if_mvpp.c >iicmux.c imxdog.c imxdwusb.c imxehci.c >imxiic_fdt.c imxpciephy.c imxpwm.c ipmi_fdt.c >mvkpcie.c mvsw.c ociic.c pciecam.c rkanxdp.c >rkdrm.c rkdwhdmi.c rkiis.c rkrng.c rktcphy.c >rkvop.c simpleamp.c simpleaudio.c tcpci.c > sys/dev/gpio : gpio.c gpioiic.c gpioow.c gpiosim.c > sys/dev/hil: hilid.c hilkbd.c hilms.c > sys/dev/i2c: abx80x.c ad741x.c adm1021.c adm1024.c adm1025.c >adm1026.c adm1030.c adm1031.c adt7460.c >adt7462.c asb100.c asc7611.c asc7621.c bmc150.c >ds1307.c ds1631.c ds3231.c fcu.c fintek.c >gl518sm.c i2c.c iatp.c ihidev.c ipmi_i2c.c >isl1208.c lis331dl.c lm75.c lm78_i2c.c lm87.c >lm93.c maxim6690.c mcp794xx.c pca9532.c >pca9548.c pca9554.c pcf85063.c pcf8523.c >pcf8563.c rs5c372.c sdtemp.c spdmem_i2c.c >thmc50.c tmp451.c tsl2560.c w83793g.c w83795g.c >w83l784r.c > sys/dev/isa: addcom_isa.c aic_isa.c aic_isapnp.c
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 06, 2022 at 08:44:51PM -0600, Kevin Lo wrote: > > CVSROOT: /cvs > Module name: src > Changes by: ke...@cvs.openbsd.org 2022/04/06 20:44:51 > > Modified files: > distrib/notes/arm64: hardware > > Log message: > Mention Allwinner H6 > > ok jsg@ and ok deraadt@
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 04, 2022 at 07:45:02AM -0600, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2022/04/04 07:45:02 > > Modified files: > usr.sbin/rpki-client: rrdp.c > > Log message: > Remove outdated XXX comment. This got fixed in January. > Missing OK tb@ -- :wq Claudio
Re: CVS: cvs.openbsd.org: src
On 2022/04/02 06:22, Kevin Lo wrote: > CVSROOT: /cvs > Module name: src > Changes by: ke...@cvs.openbsd.org 2022/04/02 06:22:56 > > Modified files: > sys/dev/usb: if_ure.c if_urereg.h > > Log message: > Add preliminary support for RTL8156B and bug fixes for RTL8153/RTL8156. > > ok stsp@ > > Tested: > RTL8152 (0x4c10): jmatthew, Marcus Merighi > RTL8153 (0x5c10): Yifei Zhan > RTL8153 (0x5c20): James Jerkins, Paul de Weerd, stsp > RTL8153 (0x5c30): gerhard, sthen > RTL8153B (0x6010): Paul de Weerd Mine was RTL8153B (0x6010)
Re: CVS: cvs.openbsd.org: src
On Wed, 30 Mar 2022, Damien Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2022/03/30 21:05:49 > > Modified files: > usr.bin/ssh: PROTOCOL sftp-server.c > usr.bin/ssh/sftp-server: Makefile > > Log message: > add support for the "corp-data" protocol extension to allow * copy-data > server-side copies to be performed without having to go via > the client. Patch by Mike Frysinger, ok dtucker@ > >
Re: CVS: cvs.openbsd.org: src
Theo Buehler writes:
> On Mon, Mar 28, 2022 at 07:47:45AM +0200, Anton Lindqvist wrote:
>> On Sun, Mar 27, 2022 at 06:22:20PM -0600, Dave Voutila wrote:
>> > CVSROOT: /cvs
>> > Module name: src
>> > Changes by:d...@cvs.openbsd.org2022/03/27 18:22:20
>> >
>> > Modified files:
>> >sys/arch/amd64/amd64: vmm.c
>> >
>> > Log message:
>> > vmm(4): add quiesce/wakeup hooks to sync vcpu state.
>> >
>> > If a host suspends or hibernates, a task in the middle of using
>> > vcpu state may be rescheduled to another cpu. This is primarily a
>> > problem for Intel hosts as vcpu state is kept local to the physical
>> > cpu and must be flushed back to physical memory before another cpu
>> > can issue certain vmx instructions.
>> >
>> > This change ensures no tasks are actively using the vmm device,
>> > flushes all vcpu state (if Intel hardware), and turns off virtualization
>> > mode on the host cpus. Upon wakeup, we reverse the process.
>> >
>> > Reported on bugs@ by mpi@. OK mlarkin@
>>
>> This broke kernel compilation on syzkaller.
>>
>> sys/arch/amd64/amd64/vmm.c:499:11: error: implicit declaration of function
>> 'vmx_remote_vmclear' is invalid in C99
>> [-Werror,-Wimplicit-function-declaration]
>>
>> https://syzkaller.appspot.com/bug?id=c58155e5ad57153055f3b4db9d86234cd086fe3b
>
> Just ran into this as well. The below fixes 'make release'. Does this
> look right?
>
I believe this was committed, but yes, this is correct. Thanks for the
assist, tb.
> Index: arch/amd64/amd64/vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.304
> diff -u -p -r1.304 vmm.c
> --- arch/amd64/amd64/vmm.c28 Mar 2022 00:22:20 - 1.304
> +++ arch/amd64/amd64/vmm.c28 Mar 2022 05:52:33 -
> @@ -494,6 +494,7 @@ vmm_quiesce_vmx(void)
> continue;
> }
>
> +#ifdef MULTIPROCESSOR
> if (vcpu->vc_last_pcpu != curcpu()) {
> /* Remote cpu vmclear via ipi. */
> err = vmx_remote_vmclear(vcpu->vc_last_pcpu,
> @@ -502,7 +503,9 @@ vmm_quiesce_vmx(void)
> printf("%s: failed to remote vmclear "
> "vcpu %d of vm %d\n", __func__,
> vcpu->vc_id, vm->vm_id);
> - } else {
> + } else
> +#endif
> + {
> /* Local cpu vmclear instruction. */
> if ((err = vmclear(>vc_control_pa)))
> printf("%s: failed to locally vmclear "
Re: CVS: cvs.openbsd.org: src
On Mon, Mar 28, 2022 at 07:47:45AM +0200, Anton Lindqvist wrote:
> On Sun, Mar 27, 2022 at 06:22:20PM -0600, Dave Voutila wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2022/03/27 18:22:20
> >
> > Modified files:
> > sys/arch/amd64/amd64: vmm.c
> >
> > Log message:
> > vmm(4): add quiesce/wakeup hooks to sync vcpu state.
> >
> > If a host suspends or hibernates, a task in the middle of using
> > vcpu state may be rescheduled to another cpu. This is primarily a
> > problem for Intel hosts as vcpu state is kept local to the physical
> > cpu and must be flushed back to physical memory before another cpu
> > can issue certain vmx instructions.
> >
> > This change ensures no tasks are actively using the vmm device,
> > flushes all vcpu state (if Intel hardware), and turns off virtualization
> > mode on the host cpus. Upon wakeup, we reverse the process.
> >
> > Reported on bugs@ by mpi@. OK mlarkin@
>
> This broke kernel compilation on syzkaller.
>
> sys/arch/amd64/amd64/vmm.c:499:11: error: implicit declaration of function
> 'vmx_remote_vmclear' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
>
> https://syzkaller.appspot.com/bug?id=c58155e5ad57153055f3b4db9d86234cd086fe3b
Just ran into this as well. The below fixes 'make release'. Does this
look right?
Index: arch/amd64/amd64/vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.304
diff -u -p -r1.304 vmm.c
--- arch/amd64/amd64/vmm.c 28 Mar 2022 00:22:20 - 1.304
+++ arch/amd64/amd64/vmm.c 28 Mar 2022 05:52:33 -
@@ -494,6 +494,7 @@ vmm_quiesce_vmx(void)
continue;
}
+#ifdef MULTIPROCESSOR
if (vcpu->vc_last_pcpu != curcpu()) {
/* Remote cpu vmclear via ipi. */
err = vmx_remote_vmclear(vcpu->vc_last_pcpu,
@@ -502,7 +503,9 @@ vmm_quiesce_vmx(void)
printf("%s: failed to remote vmclear "
"vcpu %d of vm %d\n", __func__,
vcpu->vc_id, vm->vm_id);
- } else {
+ } else
+#endif
+ {
/* Local cpu vmclear instruction. */
if ((err = vmclear(>vc_control_pa)))
printf("%s: failed to locally vmclear "
Re: CVS: cvs.openbsd.org: src
On Sun, Mar 27, 2022 at 06:22:20PM -0600, Dave Voutila wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2022/03/27 18:22:20 > > Modified files: > sys/arch/amd64/amd64: vmm.c > > Log message: > vmm(4): add quiesce/wakeup hooks to sync vcpu state. > > If a host suspends or hibernates, a task in the middle of using > vcpu state may be rescheduled to another cpu. This is primarily a > problem for Intel hosts as vcpu state is kept local to the physical > cpu and must be flushed back to physical memory before another cpu > can issue certain vmx instructions. > > This change ensures no tasks are actively using the vmm device, > flushes all vcpu state (if Intel hardware), and turns off virtualization > mode on the host cpus. Upon wakeup, we reverse the process. > > Reported on bugs@ by mpi@. OK mlarkin@ This broke kernel compilation on syzkaller. sys/arch/amd64/amd64/vmm.c:499:11: error: implicit declaration of function 'vmx_remote_vmclear' is invalid in C99 [-Werror,-Wimplicit-function-declaration] https://syzkaller.appspot.com/bug?id=c58155e5ad57153055f3b4db9d86234cd086fe3b
Re: CVS: cvs.openbsd.org: src
On 2022-03-21 10:25 -06, Florian Obser wrote: > CVSROOT: /cvs > Module name: src > Changes by: flor...@cvs.openbsd.org 2022/03/21 10:25:47 > > Modified files: > sbin/slaacd: engine.c frontend.c slaacd.h > usr.sbin/slaacctl: slaacctl.c > > Log message: > Prevent crash of unprivileged engine process (pledged stdio). > > The length field of a DNS label in the DNS search list option is an 8 > bit unsigned value. parse_dnssl() treats the search list option as an > array of char, which are signed on most archs. When we read this value > into an int variable it gets sign extended, allowing it to bypass > sanity checks and eventually we pass it as the length to memcpy which > treats it as a huge unsigned value leading to a heap overflow. > > An easy fix would be change the signature of parse_dnssl to > parse_dnssl(uint8_t* data, int datalen). > > However, the DNS search list option is unused and the function fails > to check if the parsed value is a valid domain name. The function is > also getting in the way of future work so it's best to just delete it. > > The problem was found and reported by qualys, thanks! > > OK bluhm > Unfortunately there was a misunderstanding, this problem was found and reported by Francisco Falcon of Quarkslab. I'm very sorry for the misattribution.
Re: CVS: cvs.openbsd.org: src
On Sat, Mar 19, 2022 at 07:43:26AM +0100, Anton Lindqvist wrote: > On Thu, Mar 17, 2022 at 03:45:52PM -0600, Marc Espie wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: es...@cvs.openbsd.org 2022/03/17 15:45:52 > > > > Modified files: > > usr.sbin/pkg_add/OpenBSD: Add.pm Delete.pm UpdateSet.pm Vstat.pm > > > > Log message: > > add some extra logic to prevent moving files around when this is possible. > > Specifically, we created pkg. temp files for updates to work. > > > > When I added tied files, I generalized this to installs as well, because > > it was becoming too complex. > > > > Forward a few years: > > - we have tags and define-tag, so we can deem a lot of UpdateSets "safe" > > (because they don't run command during the deletion/installation, but at > > the end, so they won't see unwanted files) > > - the tied logic is well-proven > > > > With this diff: > > - installs will again extract files directly in-place, so that install > > is (mostly) chown + utimes. > > > > - updates will extract files with new names directly in-place > > > > - tied files that didn't change names will have zero churn (instead > > of link to pkg., rm orig file, mv pkg. back to orig file) > > > > After lots of tests involving somewhat broken things. > > > > Okay sthen@ > > > > (if it breaks it's easy to revert, but the speed-up for stuff like > > texlive minor updates is significant) > > Something changed causing regress to fail. > > > usr.sbin/pkg_add: > Exit: 1 > Duration: 00:02:08 > Log: 244-usr.sbin-pkg_add.log > > temp-error3 > --- temp3.out Sat Mar 19 06:05:51 2022 > +++ /home/src/regress/usr.sbin/pkg_add/temp3.ref Tue Jul 16 11:34:29 2019 > @@ -0,0 +1,2 @@ > +User USER couldn't create temp file as /nonexistent/pkg.XX: No such > file or directory > +1 > *** Error 1 in . (Makefile:705 'temp-error3': @diff -u temp3.out > /home/src/regress/usr.sbin/pkg_add/temp3.ref) > FAILED > Ah that's okay. It's actually fault injection that no longer triggers because that case doesn't create a temp file
Re: CVS: cvs.openbsd.org: src
On Thu, Mar 17, 2022 at 03:45:52PM -0600, Marc Espie wrote: > CVSROOT: /cvs > Module name: src > Changes by: es...@cvs.openbsd.org 2022/03/17 15:45:52 > > Modified files: > usr.sbin/pkg_add/OpenBSD: Add.pm Delete.pm UpdateSet.pm Vstat.pm > > Log message: > add some extra logic to prevent moving files around when this is possible. > Specifically, we created pkg. temp files for updates to work. > > When I added tied files, I generalized this to installs as well, because > it was becoming too complex. > > Forward a few years: > - we have tags and define-tag, so we can deem a lot of UpdateSets "safe" > (because they don't run command during the deletion/installation, but at > the end, so they won't see unwanted files) > - the tied logic is well-proven > > With this diff: > - installs will again extract files directly in-place, so that install > is (mostly) chown + utimes. > > - updates will extract files with new names directly in-place > > - tied files that didn't change names will have zero churn (instead > of link to pkg., rm orig file, mv pkg. back to orig file) > > After lots of tests involving somewhat broken things. > > Okay sthen@ > > (if it breaks it's easy to revert, but the speed-up for stuff like > texlive minor updates is significant) Something changed causing regress to fail. > usr.sbin/pkg_add: Exit: 1 Duration: 00:02:08 Log: 244-usr.sbin-pkg_add.log temp-error3 --- temp3.out Sat Mar 19 06:05:51 2022 +++ /home/src/regress/usr.sbin/pkg_add/temp3.refTue Jul 16 11:34:29 2019 @@ -0,0 +1,2 @@ +User USER couldn't create temp file as /nonexistent/pkg.XX: No such file or directory +1 *** Error 1 in . (Makefile:705 'temp-error3': @diff -u temp3.out /home/src/regress/usr.sbin/pkg_add/temp3.ref) FAILED
