Re: uhidppctl(8)

2021-12-22 Thread Raf Czlonka
On Wed, Dec 22, 2021 at 08:32:16AM GMT, Claudio Jeker wrote:
> On Tue, Dec 21, 2021 at 03:49:47PM -0500, jwinnie@tilde.institute wrote:
> > 
> > Hello OpenBSD developers,
> > 
> > I am interested in contributing to improve the uhidpp(4)
> > (Logitech Unifying Reciever) support in OpenBSD.
> > 
> > Currently, the uhidpp(4) driver only handles detecting certain
> > sensors, but I would like more robust support for these devices,
> > including:
> > 
> > * pairing and unpairing devices on the command-line
> > * controlling the scrolling speed/auxiliary buttons of wireless mice
> > 
> > Do you know where I should start? Should I work on the uhidpp(4)
> > driver, or should I go ahead and start writing a command-line utility,
> > like uhidppctl(8)?
> 
> I doubt we want a uhidppctl(8) unless it is really necessary.
> I would expect wsmouse(4) to handle the controlling of scrolling
> speed and auxiliary buttons.

Hi Claudio,

I had to do pair some devices recently for both Logitech Unifying
receiver as well as How Dell Universal Pairing receiver (out of
scope here), and it was a bit of a faff having to source another
machine and OS. So, purely from a user perspective, given that a
single receiver can pair up to six devices, it would be nice not
having to use another machine/OS to do the pairing step :^)

Regards,

Raf



Re: uhidppctl(8)

2021-12-22 Thread Anton Lindqvist
On Tue, Dec 21, 2021 at 03:49:47PM -0500, jwinnie@tilde.institute wrote:
> Hello OpenBSD developers,
> 
> I am interested in contributing to improve the uhidpp(4)
> (Logitech Unifying Reciever) support in OpenBSD.
> 
> Currently, the uhidpp(4) driver only handles detecting certain
> sensors, but I would like more robust support for these devices,
> including:
> 
> * pairing and unpairing devices on the command-line
> * controlling the scrolling speed/auxiliary buttons of wireless mice
> 
> Do you know where I should start? Should I work on the uhidpp(4)
> driver, or should I go ahead and start writing a command-line utility,
> like uhidppctl(8)?

There's already Solaar[1], I have used it on Linux to pair devices but I
doubt it works on OpenBSD. It would probably need a new /dev/uhidppX
character device similar to /dev/fido/X in order to communicate with the
receiver from user space through uhid(4). The device pairing protocol is
documented here[2].

[1] https://github.com/pwr-Solaar/Solaar
[2] 
https://lekensteyn.nl/files/logitech/logitech_hidpp10_specification_for_Unifying_Receivers.pdf



Re: sysctl diskinit tailq foreach

2021-12-22 Thread Alexander Bluhm
On Tue, Dec 21, 2021 at 04:03:22PM +0100, Alexander Bluhm wrote:
> I would like to use TAILQ_FOREACH to traverse the disk list.
> Code is easier to read.

Merged to -current.

ok?

bluhm

Index: kern/kern_sysctl.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.397
diff -u -p -r1.397 kern_sysctl.c
--- kern/kern_sysctl.c  22 Dec 2021 22:20:13 -  1.397
+++ kern/kern_sysctl.c  22 Dec 2021 23:51:03 -
@@ -121,7 +121,6 @@
 extern struct forkstat forkstat;
 extern struct nchstats nchstats;
 extern int nselcoll, fscale;
-extern struct disklist_head disklist;
 extern fixpt_t ccpu;
 extern long numvnodes;
 extern int allowdt;
@@ -2132,12 +2131,12 @@ sysctl_diskinit(int update, struct proc 
struct diskstats *sdk;
struct disk *dk;
const char *duid;
-   int i, changed = 0;
+   int error, changed = 0;
 
KERNEL_ASSERT_LOCKED();
 
-   if ((i = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
-   return i;
+   if ((error = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
+   return error;
 
/* Run in a loop, disks may change while malloc sleeps. */
while (disk_change) {
@@ -2145,8 +2144,8 @@ sysctl_diskinit(int update, struct proc 
 
disk_change = 0;
 
-   for (dk = TAILQ_FIRST(), tlen = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link)) {
+   tlen = 0;
+   TAILQ_FOREACH(dk, , dk_link) {
if (dk->dk_name)
tlen += strlen(dk->dk_name);
tlen += 18; /* label uid + separators */
@@ -2173,8 +2172,9 @@ sysctl_diskinit(int update, struct proc 
if (changed) {
int l;
 
-   for (dk = TAILQ_FIRST(), i = 0, l = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
+   l = 0;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
duid = NULL;
if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
duid = duid_format(dk->dk_label->d_uid);
@@ -2182,7 +2182,6 @@ sysctl_diskinit(int update, struct proc 
dk->dk_name ? dk->dk_name : "",
duid ? duid : "");
l += strlen(disknames + l);
-   sdk = diskstats + i;
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2196,6 +2195,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
 
/* Eliminate trailing comma */
@@ -2203,9 +2203,8 @@ sysctl_diskinit(int update, struct proc 
disknames[l - 1] = '\0';
} else if (update) {
/* Just update, number of drives hasn't changed */
-   for (dk = TAILQ_FIRST(), i = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
-   sdk = diskstats + i;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2219,6 +2218,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
}
rw_exit_write(_disklock);



Re: syzkaller vnd ioctl unlock

2021-12-22 Thread Jonathan Gray
On Wed, Dec 22, 2021 at 11:56:38PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> syzkaller found a missing unlock in vnd ioctl error path.
> 
> https://syzkaller.appspot.com/bug?id=b35a411a91f835fffb793df63aa8bcd7be99ad87
> 
> ok?

ok jsg@

> 
> bluhm
> 
> Index: dev/vnd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/vnd.c,v
> retrieving revision 1.176
> diff -u -p -r1.176 vnd.c
> --- dev/vnd.c 21 Dec 2021 06:12:03 -  1.176
> +++ dev/vnd.c 22 Dec 2021 22:46:29 -
> @@ -498,6 +498,7 @@ fail:
>   if ((error = disk_lock(>sc_dk)) != 0)
>   goto fail;
>   if (sc->sc_flags & VNF_INITED) {
> + disk_unlock(>sc_dk);
>   error = EBUSY;
>   goto fail;
>   }
> 
> 



syzkaller vnd ioctl unlock

2021-12-22 Thread Alexander Bluhm
Hi,

syzkaller found a missing unlock in vnd ioctl error path.

https://syzkaller.appspot.com/bug?id=b35a411a91f835fffb793df63aa8bcd7be99ad87

ok?

bluhm

Index: dev/vnd.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/vnd.c,v
retrieving revision 1.176
diff -u -p -r1.176 vnd.c
--- dev/vnd.c   21 Dec 2021 06:12:03 -  1.176
+++ dev/vnd.c   22 Dec 2021 22:46:29 -
@@ -498,6 +498,7 @@ fail:
if ((error = disk_lock(>sc_dk)) != 0)
goto fail;
if (sc->sc_flags & VNF_INITED) {
+   disk_unlock(>sc_dk);
error = EBUSY;
goto fail;
}



witness generic mp

2021-12-22 Thread Alexander Bluhm
Hi,

Template for option WITNESS is in the architeture GENERIC.MP file
if it is supported.  It should not be in the global GENERIC config.

ok?

bluhm

Index: conf/GENERIC
===
RCS file: /data/mirror/openbsd/cvs/src/sys/conf/GENERIC,v
retrieving revision 1.280
diff -u -p -r1.280 GENERIC
--- conf/GENERIC11 Nov 2021 10:03:10 -  1.280
+++ conf/GENERIC22 Dec 2021 22:32:05 -
@@ -14,7 +14,6 @@ optionKTRACE  # system call tracing, a
 option ACCOUNTING  # acct(2) process accounting
 option KMEMSTATS   # collect malloc(9) statistics
 option PTRACE  # ptrace(2) system call
-#optionWITNESS # witness(4) lock checker
 
 #optionKVA_GUARDPAGES  # slow virtual address recycling (+ 
guarding)
 option POOL_DEBUG  # pool corruption detection



Re: fix vmctl -B net -b bsd.rd to autoinstall

2021-12-22 Thread Dave Voutila


Claudio Jeker  writes:

> On Wed, Dec 22, 2021 at 10:14:40AM -0500, Dave Voutila wrote:
>>
>> Claudio Jeker  writes:
>>
>> > I added support for vmctl -cL -B net -b bsd.rd -d disk.img to run the
>> > autoinstall by emulating a PXE boot. In the commit
>> > https://github.com/openbsd/src/commit/a13de4d12a4c9ba0edc05aab2ad635f782449229
>> > the feature got removed over eagerly.
>> >
>> > This diff adds this back because I find this super practical.
>>
>> Seems to add back more than that...see below.
>>
>
> Those are becasue of warnings from the compiler and I do not like to send
> out diffs for files that compile with a bunch of warnings.
> I can split this out into an extra commit if people prefer that.
>

Yes please. :) Understand the warnings concern, though.

-dv



Re: fix vmctl -B net -b bsd.rd to autoinstall

2021-12-22 Thread Claudio Jeker
On Wed, Dec 22, 2021 at 10:14:40AM -0500, Dave Voutila wrote:
> 
> Claudio Jeker  writes:
> 
> > I added support for vmctl -cL -B net -b bsd.rd -d disk.img to run the
> > autoinstall by emulating a PXE boot. In the commit
> > https://github.com/openbsd/src/commit/a13de4d12a4c9ba0edc05aab2ad635f782449229
> > the feature got removed over eagerly.
> >
> > This diff adds this back because I find this super practical.
> 
> Seems to add back more than that...see below.
> 

Those are becasue of warnings from the compiler and I do not like to send
out diffs for files that compile with a bunch of warnings.
I can split this out into an extra commit if people prefer that.

> >
> > Index: loadfile.h
> > ===
> > RCS file: /cvs/src/usr.sbin/vmd/loadfile.h,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 loadfile.h
> > --- loadfile.h  16 Jun 2021 16:55:02 -  1.15
> > +++ loadfile.h  22 Dec 2021 14:34:06 -
> > @@ -80,7 +80,8 @@
> >  #define PML2_PAGE 0x13000
> >  #define NPTE_PG (PAGE_SIZE / sizeof(uint64_t))
> >
> > -int loadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state 
> > *);
> > +intloadfile_elf(gzFile, struct vm_create_params *, struct 
> > vcpu_reg_state *,
> > +   unsigned int);
> >
> >  size_t mread(gzFile, paddr_t, size_t);
> >
> > Index: loadfile_elf.c
> > ===
> > RCS file: /cvs/src/usr.sbin/vmd/loadfile_elf.c,v
> > retrieving revision 1.39
> > diff -u -p -r1.39 loadfile_elf.c
> > --- loadfile_elf.c  4 May 2021 10:48:51 -   1.39
> > +++ loadfile_elf.c  22 Dec 2021 14:38:55 -
> > @@ -118,7 +118,7 @@ static void setsegment(struct mem_segmen
> >  static int elf32_exec(gzFile, Elf32_Ehdr *, u_long *, int);
> >  static int elf64_exec(gzFile, Elf64_Ehdr *, u_long *, int);
> >  static size_t create_bios_memmap(struct vm_create_params *, bios_memmap_t 
> > *);
> > -static uint32_t push_bootargs(bios_memmap_t *, size_t);
> > +static uint32_t push_bootargs(bios_memmap_t *, size_t, bios_bootmac_t *);
> >  static size_t push_stack(uint32_t, uint32_t);
> >  static void push_gdt(void);
> >  static void push_pt_32(void);
> > @@ -264,13 +264,14 @@ push_pt_64(void)
> >   */
> >  int
> >  loadfile_elf(gzFile fp, struct vm_create_params *vcp,
> > -struct vcpu_reg_state *vrs)
> > +struct vcpu_reg_state *vrs, unsigned int bootdevice)
> >  {
> > int r, is_i386 = 0;
> > uint32_t bootargsz;
> > size_t n, stacksize;
> > u_long marks[MARK_MAX];
> > bios_memmap_t memmap[VMM_MAX_MEM_RANGES + 1];
> > +   bios_bootmac_t bm, *bootmac = NULL;
> >
> > if ((r = gzread(fp, , sizeof(hdr))) != sizeof(hdr))
> > return 1;
> > @@ -301,8 +302,12 @@ loadfile_elf(gzFile fp, struct vm_create
> > else
> > push_pt_64();
> >
> > +   if (bootdevice & VMBOOTDEV_NET) {
> > +   bootmac = 
> > +   memcpy(bootmac, vcp->vcp_macs[0], ETHER_ADDR_LEN);
> > +   }
> > n = create_bios_memmap(vcp, memmap);
> > -   bootargsz = push_bootargs(memmap, n);
> > +   bootargsz = push_bootargs(memmap, n, bootmac);
> > stacksize = push_stack(bootargsz, marks[MARK_END]);
> >
> > vrs->vrs_gprs[VCPU_REGS_RIP] = (uint64_t)marks[MARK_ENTRY];
> > @@ -382,9 +387,9 @@ create_bios_memmap(struct vm_create_para
> >   *  The size of the bootargs
> >   */
> >  static uint32_t
> > -push_bootargs(bios_memmap_t *memmap, size_t n)
> > +push_bootargs(bios_memmap_t *memmap, size_t n, bios_bootmac_t *bootmac)
> >  {
> > -   uint32_t memmap_sz, consdev_sz, i;
> > +   uint32_t memmap_sz, consdev_sz, bootmac_sz, i;
> > bios_consdev_t consdev;
> > uint32_t ba[1024];
> >
> > @@ -408,6 +413,15 @@ push_bootargs(bios_memmap_t *memmap, siz
> > memcpy([i + 3], , sizeof(bios_consdev_t));
> > i += consdev_sz / sizeof(int);
> >
> > +   if (bootmac) {
> > +   bootmac_sz = 3 * sizeof(int) + (sizeof(bios_bootmac_t) + 3) & 
> > ~3;
> > +   ba[i] = 0x7;   /* bootmac */
> > +   ba[i + 1] = bootmac_sz;
> > +   ba[i + 2] = bootmac_sz;
> > +   memcpy([i + 3], bootmac, sizeof(bios_bootmac_t));
> > +   i += bootmac_sz / sizeof(int);
> > +   }
> > +
> > ba[i++] = 0x; /* BOOTARG_END */
> >
> > write_mem(BOOTARGS_PAGE, ba, PAGE_SIZE);
> > @@ -485,7 +499,7 @@ mread(gzFile fp, paddr_t addr, size_t sz
> > const char *errstr = NULL;
> > int errnum = 0;
> > size_t ct;
> > -   size_t i, rd, osz;
> > +   size_t i, osz;
> 
> Not sure the removal of rd is related.
> 
> > char buf[PAGE_SIZE];
> >
> > /*
> > @@ -493,7 +507,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
> >  * write_mem
> >  */
> > ct = 0;
> > -   rd = 0;
> > osz = sz;
> > if ((addr & PAGE_MASK) != 0) {
> > memset(buf, 0, sizeof(buf));
> > @@ -510,7 +523,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
> > errnum, errstr);
> >  

Re: boot stuck with latest cvs checkout

2021-12-22 Thread Alexander Bluhm
On Wed, Dec 22, 2021 at 05:20:02PM +0100, Hrvoje Popovski wrote:
> On 22.12.2021. 17:07, Hrvoje Popovski wrote:
> > i've sysupgrade box and reboot it and everything seems fine. then cvs
> > checkout it, compile and then box stuck at boot
> > 
> >>> OpenBSD/amd64 BOOT 3.53
> > boot>
> > booting hd0a:/bsd: 10246939+2425860+258068+0+1130496
> > [97+573872+611580]=0xe8c924
> > entry point at 0xd0201000

For me custom kernel works and looks like this.

probing: pc0 com0 mem[636K 1023M 943M 123M 6M 30448M]
disk: hd0 hd1*
>> OpenBSD/amd64 BOOTX64 3.59
switching console to com0
>> OpenBSD/amd64 BOOTX64 3.59
boot> 
booting hd0a:/bsd: 14800152+3384328+34+0+1171456 
[1069960+128+1164600+876833]=0x15c2e68
entry point at 0x1001000
[ using 3112552 bytes of bsd ELF symbol table ]

Some numbers are very different.  Are you sure you booted the correct
kernel?



Re: boot stuck with latest cvs checkout

2021-12-22 Thread Hrvoje Popovski
On 22.12.2021. 17:20, Hrvoje Popovski wrote:
> On 22.12.2021. 17:07, Hrvoje Popovski wrote:
>> Hi all,
>>
>> i've sysupgrade box and reboot it and everything seems fine. then cvs
>> checkout it, compile and then box stuck at boot
>>
 OpenBSD/amd64 BOOT 3.53
>> boot>
>> booting hd0a:/bsd: 10246939+2425860+258068+0+1130496
>> [97+573872+611580]=0xe8c924
>> entry point at 0xd0201000
>>
>>


please forget this .. i did something very very stupid ...




Re: boot stuck with latest cvs checkout

2021-12-22 Thread Hrvoje Popovski
On 22.12.2021. 17:07, Hrvoje Popovski wrote:
> Hi all,
> 
> i've sysupgrade box and reboot it and everything seems fine. then cvs
> checkout it, compile and then box stuck at boot
> 
>>> OpenBSD/amd64 BOOT 3.53
> boot>
> booting hd0a:/bsd: 10246939+2425860+258068+0+1130496
> [97+573872+611580]=0xe8c924
> entry point at 0xd0201000
> 
> 


r620-1# dmesg
OpenBSD 7.0-current (GENERIC.MP) #188: Mon Dec 20 22:32:56 MST 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 17115840512 (16322MB)
avail mem = 16581124096 (15812MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xcf42c000 (99 entries)
bios0: vendor Dell Inc. version "2.9.0" date 12/06/2019
bios0: Dell Inc. PowerEdge R620
acpi0 at bios0: ACPI 3.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WDAT SLIC ERST HEST
BERT EINJ TCPA PC__ SRAT SSDT
acpi0: wakeup devices PCI0(S5)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 4 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.59 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 2, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE
cpu1 at mainbus0: apid 6 (application processor)
cpu1: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.01 MHz, 06-3e-04
cpu1:
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
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 3, package 0
cpu2 at mainbus0: apid 8 (application processor)
cpu2: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.01 MHz, 06-3e-04
cpu2:
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
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 4, package 0
cpu3 at mainbus0: apid 16 (application processor)
cpu3: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.01 MHz, 06-3e-04
cpu3:
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
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 8, package 0
cpu4 at mainbus0: apid 18 (application processor)
cpu4: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.01 MHz, 06-3e-04
cpu4:
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
cpu4: 256KB 64b/line 8-way L2 cache
cpu4: smt 0, core 9, package 0
cpu5 at mainbus0: apid 20 (application processor)
cpu5: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz, 3600.01 MHz, 06-3e-04
cpu5:
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
cpu5: 256KB 64b/line 8-way L2 cache
cpu5: smt 0, core 10, package 0
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 20, 24 pins
ioapic1 at mainbus0: apid 1 pa 0xfec3f000, version 20, 24 pins, remapped
acpihpet0 at acpi0: 14318179 Hz
acpimcfg0 at acpi0
acpimcfg0: addr 0xe000, bus 0-255
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (PEX1)
acpiprt2 at acpi0: bus 

Re: fix vmctl -B net -b bsd.rd to autoinstall

2021-12-22 Thread Dave Voutila


Claudio Jeker  writes:

> I added support for vmctl -cL -B net -b bsd.rd -d disk.img to run the
> autoinstall by emulating a PXE boot. In the commit
> https://github.com/openbsd/src/commit/a13de4d12a4c9ba0edc05aab2ad635f782449229
> the feature got removed over eagerly.
>
> This diff adds this back because I find this super practical.

Seems to add back more than that...see below.

>
> Index: loadfile.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/loadfile.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 loadfile.h
> --- loadfile.h16 Jun 2021 16:55:02 -  1.15
> +++ loadfile.h22 Dec 2021 14:34:06 -
> @@ -80,7 +80,8 @@
>  #define PML2_PAGE 0x13000
>  #define NPTE_PG (PAGE_SIZE / sizeof(uint64_t))
>
> -int loadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state *);
> +int  loadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state *,
> + unsigned int);
>
>  size_t mread(gzFile, paddr_t, size_t);
>
> Index: loadfile_elf.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/loadfile_elf.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 loadfile_elf.c
> --- loadfile_elf.c4 May 2021 10:48:51 -   1.39
> +++ loadfile_elf.c22 Dec 2021 14:38:55 -
> @@ -118,7 +118,7 @@ static void setsegment(struct mem_segmen
>  static int elf32_exec(gzFile, Elf32_Ehdr *, u_long *, int);
>  static int elf64_exec(gzFile, Elf64_Ehdr *, u_long *, int);
>  static size_t create_bios_memmap(struct vm_create_params *, bios_memmap_t *);
> -static uint32_t push_bootargs(bios_memmap_t *, size_t);
> +static uint32_t push_bootargs(bios_memmap_t *, size_t, bios_bootmac_t *);
>  static size_t push_stack(uint32_t, uint32_t);
>  static void push_gdt(void);
>  static void push_pt_32(void);
> @@ -264,13 +264,14 @@ push_pt_64(void)
>   */
>  int
>  loadfile_elf(gzFile fp, struct vm_create_params *vcp,
> -struct vcpu_reg_state *vrs)
> +struct vcpu_reg_state *vrs, unsigned int bootdevice)
>  {
>   int r, is_i386 = 0;
>   uint32_t bootargsz;
>   size_t n, stacksize;
>   u_long marks[MARK_MAX];
>   bios_memmap_t memmap[VMM_MAX_MEM_RANGES + 1];
> + bios_bootmac_t bm, *bootmac = NULL;
>
>   if ((r = gzread(fp, , sizeof(hdr))) != sizeof(hdr))
>   return 1;
> @@ -301,8 +302,12 @@ loadfile_elf(gzFile fp, struct vm_create
>   else
>   push_pt_64();
>
> + if (bootdevice & VMBOOTDEV_NET) {
> + bootmac = 
> + memcpy(bootmac, vcp->vcp_macs[0], ETHER_ADDR_LEN);
> + }
>   n = create_bios_memmap(vcp, memmap);
> - bootargsz = push_bootargs(memmap, n);
> + bootargsz = push_bootargs(memmap, n, bootmac);
>   stacksize = push_stack(bootargsz, marks[MARK_END]);
>
>   vrs->vrs_gprs[VCPU_REGS_RIP] = (uint64_t)marks[MARK_ENTRY];
> @@ -382,9 +387,9 @@ create_bios_memmap(struct vm_create_para
>   *  The size of the bootargs
>   */
>  static uint32_t
> -push_bootargs(bios_memmap_t *memmap, size_t n)
> +push_bootargs(bios_memmap_t *memmap, size_t n, bios_bootmac_t *bootmac)
>  {
> - uint32_t memmap_sz, consdev_sz, i;
> + uint32_t memmap_sz, consdev_sz, bootmac_sz, i;
>   bios_consdev_t consdev;
>   uint32_t ba[1024];
>
> @@ -408,6 +413,15 @@ push_bootargs(bios_memmap_t *memmap, siz
>   memcpy([i + 3], , sizeof(bios_consdev_t));
>   i += consdev_sz / sizeof(int);
>
> + if (bootmac) {
> + bootmac_sz = 3 * sizeof(int) + (sizeof(bios_bootmac_t) + 3) & 
> ~3;
> + ba[i] = 0x7;   /* bootmac */
> + ba[i + 1] = bootmac_sz;
> + ba[i + 2] = bootmac_sz;
> + memcpy([i + 3], bootmac, sizeof(bios_bootmac_t));
> + i += bootmac_sz / sizeof(int);
> + }
> +
>   ba[i++] = 0x; /* BOOTARG_END */
>
>   write_mem(BOOTARGS_PAGE, ba, PAGE_SIZE);
> @@ -485,7 +499,7 @@ mread(gzFile fp, paddr_t addr, size_t sz
>   const char *errstr = NULL;
>   int errnum = 0;
>   size_t ct;
> - size_t i, rd, osz;
> + size_t i, osz;

Not sure the removal of rd is related.

>   char buf[PAGE_SIZE];
>
>   /*
> @@ -493,7 +507,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
>* write_mem
>*/
>   ct = 0;
> - rd = 0;
>   osz = sz;
>   if ((addr & PAGE_MASK) != 0) {
>   memset(buf, 0, sizeof(buf));
> @@ -510,7 +523,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
>   errnum, errstr);
>   return (0);
>   }
> - rd += ct;
>

Same here

>   if (write_mem(addr, buf, ct))
>   return (0);
> @@ -538,7 +550,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
>   errnum, errstr);
>   return (0);
>   }
> - rd += ct;
>

And here

>   if (write_mem(addr, buf, ct))
>   

boot stuck with latest cvs checkout

2021-12-22 Thread Hrvoje Popovski
Hi all,

i've sysupgrade box and reboot it and everything seems fine. then cvs
checkout it, compile and then box stuck at boot

>> OpenBSD/amd64 BOOT 3.53
boot>
booting hd0a:/bsd: 10246939+2425860+258068+0+1130496
[97+573872+611580]=0xe8c924
entry point at 0xd0201000





malloc sleep in sysctl diskinit

2021-12-22 Thread Alexander Bluhm
Hi,

syzkaller found a race in sysctl_diskinit().

https://syzkaller.appspot.com/bug?id=76838ab8f15c5f1bc22541c60c3c279314e13db0

While malloc sleeps, the disk list could change.  Retry allocating
enough space until it did not change.

Not sure if this is the bug which syzkaller has found.  But the
allocated memory could be too short for the list of disks.

The whole thing is protected by kernel lock.  Use asserts to make
it explicit.

ok?

bluhm

Index: kern/kern_sysctl.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.396
diff -u -p -r1.396 kern_sysctl.c
--- kern/kern_sysctl.c  30 Oct 2021 23:24:48 -  1.396
+++ kern/kern_sysctl.c  21 Dec 2021 16:38:59 -
@@ -2132,12 +2132,19 @@ sysctl_diskinit(int update, struct proc 
struct diskstats *sdk;
struct disk *dk;
const char *duid;
-   int i, tlen, l;
+   int i, changed = 0;
+
+   KERNEL_ASSERT_LOCKED();
 
if ((i = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
return i;
 
-   if (disk_change) {
+   /* Run in a loop, disks may change while malloc sleeps. */
+   while (disk_change) {
+   int tlen;
+
+   disk_change = 0;
+
for (dk = TAILQ_FIRST(), tlen = 0; dk;
dk = TAILQ_NEXT(dk, dk_link)) {
if (dk->dk_name)
@@ -2146,10 +2153,12 @@ sysctl_diskinit(int update, struct proc 
}
tlen++;
 
-   if (disknames)
-   free(disknames, M_SYSCTL, disknameslen);
-   if (diskstats)
-   free(diskstats, M_SYSCTL, diskstatslen);
+   /*
+* The sysctl_disklock ensures that no other process can
+* allocate disknames and diskstats while our malloc sleeps.
+*/
+   free(disknames, M_SYSCTL, disknameslen);
+   free(diskstats, M_SYSCTL, diskstatslen);
diskstats = NULL;
disknames = NULL;
diskstats = mallocarray(disk_count, sizeof(struct diskstats),
@@ -2158,13 +2167,18 @@ sysctl_diskinit(int update, struct proc 
disknames = malloc(tlen, M_SYSCTL, M_WAITOK|M_ZERO);
disknameslen = tlen;
disknames[0] = '\0';
+   changed = 1;
+   }
+
+   if (changed) {
+   int l;
 
for (dk = TAILQ_FIRST(), i = 0, l = 0; dk;
dk = TAILQ_NEXT(dk, dk_link), i++) {
duid = NULL;
if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
duid = duid_format(dk->dk_label->d_uid);
-   snprintf(disknames + l, tlen - l, "%s:%s,",
+   snprintf(disknames + l, disknameslen - l, "%s:%s,",
dk->dk_name ? dk->dk_name : "",
duid ? duid : "");
l += strlen(disknames + l);
@@ -2187,7 +2201,6 @@ sysctl_diskinit(int update, struct proc 
/* Eliminate trailing comma */
if (l != 0)
disknames[l - 1] = '\0';
-   disk_change = 0;
} else if (update) {
/* Just update, number of drives hasn't changed */
for (dk = TAILQ_FIRST(), i = 0; dk;
Index: kern/subr_disk.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.246
diff -u -p -r1.246 subr_disk.c
--- kern/subr_disk.c24 Oct 2021 00:02:25 -  1.246
+++ kern/subr_disk.c21 Dec 2021 15:20:46 -
@@ -1045,6 +1045,8 @@ disk_attach(struct device *dv, struct di
 {
int majdev;
 
+   KERNEL_ASSERT_LOCKED();
+
if (!ISSET(diskp->dk_flags, DKF_CONSTRUCTED))
disk_construct(diskp);
 
@@ -1130,6 +1132,7 @@ done:
 void
 disk_detach(struct disk *diskp)
 {
+   KERNEL_ASSERT_LOCKED();
 
if (softraid_disk_attach)
softraid_disk_attach(diskp, -1);
@@ -1845,6 +1848,8 @@ const char *
 duid_format(u_char *duid)
 {
static char duid_str[17];
+
+   KERNEL_ASSERT_LOCKED();
 
snprintf(duid_str, sizeof(duid_str),
"%02x%02x%02x%02x%02x%02x%02x%02x",



Re: [External] : Re: make 'set skip on ...' dynamic

2021-12-22 Thread Claudio Jeker
On Sat, Dec 04, 2021 at 07:01:23PM +0100, Alexandr Nedvedicky wrote:
> Hello,
> 
> 
> On Fri, Dec 03, 2021 at 03:42:09PM +0100, Claudio Jeker wrote:
> > 
> > See comments below.
> > 
> > 
> > > +void
> > > +pfi_group_delmember(const char *group, struct ifnet *ifp)
> > > +{
> > > + struct pfi_kif  *gkif, *ikif;
> > > +
> > > + if ((gkif = pfi_kif_get(group, NULL)) == NULL ||
> > > + (ikif = pfi_kif_get(ifp->if_xname, NULL)) == NULL)
> > > + panic("%s: pfi_kif_get failed", __func__);
> > > + ikif->pfik_flags_new = ikif->pfik_flags & ~gkif->pfik_flags;
> > > +
> > > + pfi_group_change(group);
> > > +}
> > > +
> > 
> > This function is not quite right. For example:
> > 
> > ifconfig vio0 group foo group bar
> > pfctl -f - < > set skip on { foo bar }
> > block return
> > EOF
> > ping -qc2 100.64.1.2
> > 
> > PING 100.64.1.2 (100.64.1.2): 56 data bytes
> > 
> > --- 100.64.1.2 ping statistics ---
> > 2 packets transmitted, 2 packets received, 0.0% packet loss
> > round-trip min/avg/max/std-dev = 0.153/0.178/0.202/0.024 ms
> > 
> > 
> > Now lets remove just group bar from the interface.
> > 
> > ifconfig vio0 -group bar  
> > ping -qc2 100.64.1.2  
> > 
> > PING 100.64.1.2 (100.64.1.2): 56 data bytes
> > ping: sendmsg: Permission denied
> > ping: wrote 100.64.1.2 64 chars, ret=-1
> > ping: sendmsg: Permission denied
> > ping: wrote 100.64.1.2 64 chars, ret=-1
> >  
> > pfi_group_delmember() does not take into consideration other groups
> > (including the interface itself) that may still allow PFI_IFLAG_SKIP.
> > It just clears the flag.
> > 
> > I think on delete the flag needs to be recalculated after the group has
> > been removed. Not sure if this is easy possible though.
> > 
> 
> yes, you are absolutely right. the current logic around pfi_skip_if() is
> bit convoluted. in  order to fix it I must rework pfi_xcommit() and 
> related
> set_flags()/clear_flags() functions. The idea is to let pfi_xcommit() to
> combine skip flags from all groups, which are attached to interface.
> 
> 
> updated diff is attached.

One comment below but this diff is OK claudio@
 
> thanks and
> regards
> sashan
> 
> 8<---8<---8<--8<
>  void
>  pfi_xcommit(void)
>  {
> - struct pfi_kif  *p;
> + struct pfi_kif  *p, *gkif;
> + struct ifg_list *g;
> + struct ifnet*ifp;
> + size_t n;
>  
> - RB_FOREACH(p, pfi_ifhead, _ifs)
> + RB_FOREACH(p, pfi_ifhead, _ifs) {
>   p->pfik_flags = p->pfik_flags_new;
> + n = strlen(p->pfik_name);
> + ifp = p->pfik_ifp;
> + /*
> +  * if kif is backed by existing interface, then we must use
> +  * skip flags found in groups. We use pfik_flags_new, otherwise
> +  * we would need to do two RB_FOREACH() passes: the first to
> +  * commit group changes the second to commit flag changes for
> +  * interfaces.
> +  */
> + if (isdigit(p->pfik_name[n - 1]) && ifp != NULL)

No other code in pf_if.c is checking both pfik_name and ifp != NULL in
similar situations.  I think you should skip the isdigit() check here.
If there is a real interface connected to a pfi_kif than it should be updated.
Lets not introduce more complexity here.

> + TAILQ_FOREACH(g, >if_groups, ifgl_next) {
> + gkif =
> + (struct pfi_kif *)g->ifgl_group->ifg_pf_kif;
> + KASSERT(gkif != NULL);
> + p->pfik_flags |= gkif->pfik_flags_new;
> + }
> + }
>  }

-- 
:wq Claudio



fix vmctl -B net -b bsd.rd to autoinstall

2021-12-22 Thread Claudio Jeker
I added support for vmctl -cL -B net -b bsd.rd -d disk.img to run the
autoinstall by emulating a PXE boot. In the commit
https://github.com/openbsd/src/commit/a13de4d12a4c9ba0edc05aab2ad635f782449229
the feature got removed over eagerly.

This diff adds this back because I find this super practical.
-- 
:wq Claudio

Index: loadfile.h
===
RCS file: /cvs/src/usr.sbin/vmd/loadfile.h,v
retrieving revision 1.15
diff -u -p -r1.15 loadfile.h
--- loadfile.h  16 Jun 2021 16:55:02 -  1.15
+++ loadfile.h  22 Dec 2021 14:34:06 -
@@ -80,7 +80,8 @@
 #define PML2_PAGE 0x13000
 #define NPTE_PG (PAGE_SIZE / sizeof(uint64_t))
 
-int loadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state *);
+intloadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state *,
+   unsigned int);
 
 size_t mread(gzFile, paddr_t, size_t);
 
Index: loadfile_elf.c
===
RCS file: /cvs/src/usr.sbin/vmd/loadfile_elf.c,v
retrieving revision 1.39
diff -u -p -r1.39 loadfile_elf.c
--- loadfile_elf.c  4 May 2021 10:48:51 -   1.39
+++ loadfile_elf.c  22 Dec 2021 14:38:55 -
@@ -118,7 +118,7 @@ static void setsegment(struct mem_segmen
 static int elf32_exec(gzFile, Elf32_Ehdr *, u_long *, int);
 static int elf64_exec(gzFile, Elf64_Ehdr *, u_long *, int);
 static size_t create_bios_memmap(struct vm_create_params *, bios_memmap_t *);
-static uint32_t push_bootargs(bios_memmap_t *, size_t);
+static uint32_t push_bootargs(bios_memmap_t *, size_t, bios_bootmac_t *);
 static size_t push_stack(uint32_t, uint32_t);
 static void push_gdt(void);
 static void push_pt_32(void);
@@ -264,13 +264,14 @@ push_pt_64(void)
  */
 int
 loadfile_elf(gzFile fp, struct vm_create_params *vcp,
-struct vcpu_reg_state *vrs)
+struct vcpu_reg_state *vrs, unsigned int bootdevice)
 {
int r, is_i386 = 0;
uint32_t bootargsz;
size_t n, stacksize;
u_long marks[MARK_MAX];
bios_memmap_t memmap[VMM_MAX_MEM_RANGES + 1];
+   bios_bootmac_t bm, *bootmac = NULL;
 
if ((r = gzread(fp, , sizeof(hdr))) != sizeof(hdr))
return 1;
@@ -301,8 +302,12 @@ loadfile_elf(gzFile fp, struct vm_create
else
push_pt_64();
 
+   if (bootdevice & VMBOOTDEV_NET) {
+   bootmac = 
+   memcpy(bootmac, vcp->vcp_macs[0], ETHER_ADDR_LEN);
+   }
n = create_bios_memmap(vcp, memmap);
-   bootargsz = push_bootargs(memmap, n);
+   bootargsz = push_bootargs(memmap, n, bootmac);
stacksize = push_stack(bootargsz, marks[MARK_END]);
 
vrs->vrs_gprs[VCPU_REGS_RIP] = (uint64_t)marks[MARK_ENTRY];
@@ -382,9 +387,9 @@ create_bios_memmap(struct vm_create_para
  *  The size of the bootargs
  */
 static uint32_t
-push_bootargs(bios_memmap_t *memmap, size_t n)
+push_bootargs(bios_memmap_t *memmap, size_t n, bios_bootmac_t *bootmac)
 {
-   uint32_t memmap_sz, consdev_sz, i;
+   uint32_t memmap_sz, consdev_sz, bootmac_sz, i;
bios_consdev_t consdev;
uint32_t ba[1024];
 
@@ -408,6 +413,15 @@ push_bootargs(bios_memmap_t *memmap, siz
memcpy([i + 3], , sizeof(bios_consdev_t));
i += consdev_sz / sizeof(int);
 
+   if (bootmac) {
+   bootmac_sz = 3 * sizeof(int) + (sizeof(bios_bootmac_t) + 3) & 
~3;
+   ba[i] = 0x7;   /* bootmac */
+   ba[i + 1] = bootmac_sz;
+   ba[i + 2] = bootmac_sz;
+   memcpy([i + 3], bootmac, sizeof(bios_bootmac_t));
+   i += bootmac_sz / sizeof(int);
+   } 
+
ba[i++] = 0x; /* BOOTARG_END */
 
write_mem(BOOTARGS_PAGE, ba, PAGE_SIZE);
@@ -485,7 +499,7 @@ mread(gzFile fp, paddr_t addr, size_t sz
const char *errstr = NULL;
int errnum = 0;
size_t ct;
-   size_t i, rd, osz;
+   size_t i, osz;
char buf[PAGE_SIZE];
 
/*
@@ -493,7 +507,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
 * write_mem
 */
ct = 0;
-   rd = 0;
osz = sz;
if ((addr & PAGE_MASK) != 0) {
memset(buf, 0, sizeof(buf));
@@ -510,7 +523,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
errnum, errstr);
return (0);
}
-   rd += ct;
 
if (write_mem(addr, buf, ct))
return (0);
@@ -538,7 +550,6 @@ mread(gzFile fp, paddr_t addr, size_t sz
errnum, errstr);
return (0);
}
-   rd += ct;
 
if (write_mem(addr, buf, ct))
return (0);
@@ -664,7 +675,6 @@ elf64_exec(gzFile fp, Elf64_Ehdr *elf, u
Elf64_Off off;
int i;
size_t sz;
-   int first;
int havesyms;
paddr_t minp = ~0, maxp = 0, pos = 0;
paddr_t offset 

ipsec kernel lock

2021-12-22 Thread Alexander Bluhm
Hi,

IPsec is not MP safe yet.  To allow forwarding in parallel without
dirty hacks, it is better to protect IPsec input and output with
kernel lock.  We do not loose much as crypto needs the kernel lock
anyway.  From here we can refine the lock later.

Note that there is no kernel lock in the SPD lockup path.  I want
to keep that lock free to allow fast forwarding with non IPsec
traffic.

There are still some races in special cases, but in general it works
with parallel IP input.

ok?

bluhm

Index: net/if_bridge.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.361
diff -u -p -r1.361 if_bridge.c
--- net/if_bridge.c 3 Dec 2021 17:18:34 -   1.361
+++ net/if_bridge.c 22 Dec 2021 10:49:59 -
@@ -1625,12 +1625,15 @@ bridge_ipsec(struct ifnet *ifp, struct e
if ((af == AF_INET) &&
ip_mtudisc && (ip->ip_off & htons(IP_DF)) &&
tdb->tdb_mtu && ntohs(ip->ip_len) > tdb->tdb_mtu &&
-   tdb->tdb_mtutimeout > gettime())
+   tdb->tdb_mtutimeout > gettime()) {
bridge_send_icmp_err(ifp, eh, m,
hassnap, llc, tdb->tdb_mtu,
ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG);
-   else
+   } else {
+   KERNEL_LOCK();
error = ipsp_process_packet(m, tdb, af, 0);
+   KERNEL_UNLOCK();
+   }
tdb_unref(tdb);
return (1);
} else
Index: netinet/ip_ah.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.171
diff -u -p -r1.171 ip_ah.c
--- netinet/ip_ah.c 20 Dec 2021 17:09:18 -  1.171
+++ netinet/ip_ah.c 22 Dec 2021 10:49:59 -
@@ -687,13 +687,11 @@ ah_input(struct mbuf **mp, struct tdb *t
crp->crp_buf = (caddr_t)m;
crp->crp_sid = tdb->tdb_cryptoid;
 
-   KERNEL_LOCK();
while ((error = crypto_invoke(crp)) == EAGAIN) {
/* Reset the session ID */
if (tdb->tdb_cryptoid != 0)
tdb->tdb_cryptoid = crp->crp_sid;
}
-   KERNEL_UNLOCK();
if (error) {
DPRINTF("crypto error %d", error);
ipsecstat_inc(ipsec_noxform);
@@ -1112,13 +1110,11 @@ ah_output(struct mbuf *m, struct tdb *td
crp->crp_buf = (caddr_t)m;
crp->crp_sid = tdb->tdb_cryptoid;
 
-   KERNEL_LOCK();
while ((error = crypto_invoke(crp)) == EAGAIN) {
/* Reset the session ID */
if (tdb->tdb_cryptoid != 0)
tdb->tdb_cryptoid = crp->crp_sid;
}
-   KERNEL_UNLOCK();
if (error) {
DPRINTF("crypto error %d", error);
ipsecstat_inc(ipsec_noxform);
Index: netinet/ip_esp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_esp.c,v
retrieving revision 1.191
diff -u -p -r1.191 ip_esp.c
--- netinet/ip_esp.c20 Dec 2021 17:09:18 -  1.191
+++ netinet/ip_esp.c22 Dec 2021 10:49:59 -
@@ -502,13 +502,11 @@ esp_input(struct mbuf **mp, struct tdb *
crde->crd_len = plen;
}
 
-   KERNEL_LOCK();
while ((error = crypto_invoke(crp)) == EAGAIN) {
/* Reset the session ID */
if (tdb->tdb_cryptoid != 0)
tdb->tdb_cryptoid = crp->crp_sid;
}
-   KERNEL_UNLOCK();
if (error) {
DPRINTF("crypto error %d", error);
ipsecstat_inc(ipsec_noxform);
@@ -948,13 +946,11 @@ esp_output(struct mbuf *m, struct tdb *t
crda->crd_len = m->m_pkthdr.len - (skip + alen);
}
 
-   KERNEL_LOCK();
while ((error = crypto_invoke(crp)) == EAGAIN) {
/* Reset the session ID */
if (tdb->tdb_cryptoid != 0)
tdb->tdb_cryptoid = crp->crp_sid;
}
-   KERNEL_UNLOCK();
if (error) {
DPRINTF("crypto error %d", error);
ipsecstat_inc(ipsec_noxform);
Index: netinet/ip_ipcomp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipcomp.c,v
retrieving revision 1.90
diff -u -p -r1.90 ip_ipcomp.c
--- netinet/ip_ipcomp.c 20 Dec 2021 15:59:09 -  1.90
+++ netinet/ip_ipcomp.c 22 Dec 2021 10:49:59 -
@@ -171,13 +171,11 @@ ipcomp_input(struct mbuf **mp, struct td
crp->crp_buf = (caddr_t)m;
crp->crp_sid = tdb->tdb_cryptoid;
 
-   KERNEL_LOCK();
while ((error = 

Re: rpki-client, stop using size_t for ids

2021-12-22 Thread Claudio Jeker
On Tue, Dec 21, 2021 at 06:24:44PM +, Job Snijders wrote:
> On Tue, Dec 21, 2021 at 07:00:03PM +0100, Claudio Jeker wrote:
> > For some reasons various ids were stored as size_t (probably because once
> > they used to be the index in an array). This is just silly and annoyed me
> > for long enough. I think this fixes all of them.
> > 
> > While there also stop using size_t for maxlength of a prefix. Everywhere
> > else the code uses just a unsigned char for that so do it there as well.
> > Shuffle struct a little bit in hope for better packing.
> > 
> > Seems to work with RRDP and RSYNC.
> 
> print.c line 155 may need a small change too (-Wformat error)

Cool, I changed that %zu to %hhu.
 
> other than that, OK job@

Thanks.

-- 
:wq Claudio



Re: uhidppctl(8)

2021-12-22 Thread Claudio Jeker
On Tue, Dec 21, 2021 at 03:49:47PM -0500, jwinnie@tilde.institute wrote:
> 
> Hello OpenBSD developers,
> 
> I am interested in contributing to improve the uhidpp(4)
> (Logitech Unifying Reciever) support in OpenBSD.
> 
> Currently, the uhidpp(4) driver only handles detecting certain
> sensors, but I would like more robust support for these devices,
> including:
> 
> * pairing and unpairing devices on the command-line
> * controlling the scrolling speed/auxiliary buttons of wireless mice
> 
> Do you know where I should start? Should I work on the uhidpp(4)
> driver, or should I go ahead and start writing a command-line utility,
> like uhidppctl(8)?

I doubt we want a uhidppctl(8) unless it is really necessary.
I would expect wsmouse(4) to handle the controlling of scrolling
speed and auxiliary buttons.

-- 
:wq Claudio