Re: Zenbleed

2023-07-24 Thread Theo de Raadt
Would like to know if this patch helps anyone with this type of
problem.

Index: sys/arch/amd64/amd64/cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.172
diff -u -p -u -r1.172 cpu.c
--- sys/arch/amd64/amd64/cpu.c  24 Jul 2023 14:53:58 -  1.172
+++ sys/arch/amd64/amd64/cpu.c  25 Jul 2023 03:28:35 -
@@ -1216,7 +1216,8 @@ cpu_fix_msrs(struct cpu_info *ci)
if (msr != nmsr)
wrmsr(MSR_DE_CFG, nmsr);
}
-   if (family == 0x17 && ci->ci_model >= 0x31) {
+   if (family == 0x17 && ci->ci_model >= 0x31 &&
+   (cpu_ecxfeature & CPUIDECX_HV) == 0) {
nmsr = msr = rdmsr(MSR_DE_CFG);
nmsr |= DE_CFG_SERIALIZE_9;
if (msr != nmsr)
Index: sys/arch/i386/i386/machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.664
diff -u -p -u -r1.664 machdep.c
--- sys/arch/i386/i386/machdep.c24 Jul 2023 14:54:00 -  1.664
+++ sys/arch/i386/i386/machdep.c25 Jul 2023 03:28:29 -
@@ -1993,7 +1993,8 @@ identifycpu(struct cpu_info *ci)
if (msr != nmsr)
wrmsr(MSR_DE_CFG, nmsr);
}
-   if (family == 0x17 && ci->ci_model >= 0x31) {
+   if (family == 0x17 && ci->ci_model >= 0x31 &&
+   (cpu_ecxfeature & CPUIDECX_HV) == 0) {
nmsr = msr = rdmsr(MSR_DE_CFG);
nmsr |= DE_CFG_SERIALIZE_9;
if (msr != nmsr)



Re: Zenbleed

2023-07-24 Thread Theo de Raadt
I am not aware of a feature flag we can check for if we can
write to MSR_DE_CFG, and my guess is their VM needs to silently
ignore the bits we modify, and not generate a fault.

I'm not sure what we can do here, but I suspect some developers will
think about it. 

Anyways, they decided to fault.  That seems wrong.  I think they will
get other complaints in the near future.  I think you should let them
know.  It is an easy fix on their side.

Lucas  wrote:

> Hey,
> 
> I'm running this on a Hetzner VPS, with the dmesg below. I ran syspatch
> followed by installboot -v sd0. After each boot, 100% I get
> 
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: AMD EPYC Processor, 2445.60 MHz, 17-31-00
> cpu0: FPU,... (proc capabilities)
> cpu0: 32KB ... (cache info)
> cpu0: smt 0, core 0, package 0
> kernel: protection fault trap, code=0
> Stopped at  cpu_fix_msrs+0x1a4:  wrmsr
> ddb{0}> show panic
> the kernel did not panic
> ddb{0}> show reg
> rdi 0x8243dff0cpu_info_full_primary+0x1ff0
> rsi0x2
> rbp 0x829277f0end+0x3277f0
> rbx 0x80043880
> rdx  0
> rcx 0xc0011029
> rax  0x202
> r8   0x101010101010101
> r9  0x8080808080808080
> r10 0xf74eb8da9a13c7be
> r11 0x4299783767f1ac05
> r12 0x800438a4
> r13 0x800020a11000
> r14 0x8243dff0cpu_info_full_primary+0x1ff0
> r15   0x17
> rip 0x811b50f4cpu_fix_msrs+0x1a4
> cs 0x8
> rflags 0x10202__ALIGN_SIZE+0xf202
> rsp 0x829277c0end+0x3277c0
> ss0x10
> cpu_fix_msrs+0x1a4:   wrmsr
> ddb{0}> bt
> cpu_fix_msrs(...) at cpu_fix_msrs+0x1a4
> cpu_attach(...) at cpu_attach+0x3fd
> config_attach(...) at config_attach+0x1f4
> acpimadt_attach(...) at acpimadt_attach+0x349
> config_attach(...) at config_attach+0x1f4
> acpi_attach_common(...) at acpi_attach_common+0x5db
> config_attach(...) at config_attach+0x1f4
> bios_attach(...) at bios_attach+0x77e
> config_attach(...) at config_attach+0x1f4
> mainbus_attach(...) at mainbus_attach+0x778
> config_attach(...) at config_attach+0x1f4
> cpu_configure(...) at cpu_configure+0x33
> main(0,0,0,0,0,1) at main+0x3d3
> end trace fram: 0x0, count: 13
> ddb{0}> mach ddbcpu 1
> Invalid cpu 1
> ddb{0}> 
> 
> 
> I've screenshots of the backtrace, should the function parameters be
> required.
> 
> If other people are lucky like, remember that you can `boot /obsd` at
> the boot prompt to boot an older kernel.
> 
> After booting the old kernel, I successfully ran fw_update and fetched
> the amd firmware package. Nevertheless, the problem persists. I tried
> rerunning `syspatch` and `installboot -v sd0` again, and the problem
> still persists. In case it's relevant, the installboot output is
> 
> Using / as root
> installing bootstrap on /dev/rsd0c
> using first-stage /usr/mdec/biosboot, second-stage /usr/mdec/boot
> copying /usr/mdec/boot to //boot
> looking for superblock at 65536
> found valid ffs2 superblock
> //boot is 6 blocks x 16384 bytes
> fs block shift 2; part offset 64; inode block 56, offset 5232
> expecting 64-bit fs blocks (incr 4)
> master boot record (MBR) at sector 0
>   partition 3: type 0xA6 offset 64 size 80003008
> /usr/mdec/biosboot will be written at sector 64
> 
> 
> OpenBSD 7.3 (GENERIC.MP) #0: Wed Jul 12 05:09:49 MDT 2023
> 
> r...@syspatch-73-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 2080227328 (1983MB)
> avail mem = 1997848576 (1905MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf59d0 (10 entries)
> bios0: vendor Hetzner version "2017" date 11/11/2017
> bios0: Hetzner vServer
> acpi0 at bios0: ACPI 3.0
> acpi0: sleep states S5
> acpi0: tables DSDT FACP APIC HPET MCFG WAET
> acpi0: wakeup devices
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: AMD EPYC Processor, 2445.58 MHz, 17-31-00
> cpu0: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,TOPEXT,CPCTR,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
> cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 
> 64b/line 8-way L2 cache, 16MB 64b/line 16-way L3 cache
> cpu0: smt 0, core 0, package 0
> mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
> cpu0: apic clock running at 

Re: Zenbleed

2023-07-24 Thread Lucas
Hey,

I'm running this on a Hetzner VPS, with the dmesg below. I ran syspatch
followed by installboot -v sd0. After each boot, 100% I get

cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD EPYC Processor, 2445.60 MHz, 17-31-00
cpu0: FPU,... (proc capabilities)
cpu0: 32KB ... (cache info)
cpu0: smt 0, core 0, package 0
kernel: protection fault trap, code=0
Stopped at  cpu_fix_msrs+0x1a4:  wrmsr
ddb{0}> show panic
the kernel did not panic
ddb{0}> show reg
rdi   0x8243dff0cpu_info_full_primary+0x1ff0
rsi  0x2
rbp   0x829277f0end+0x3277f0
rbx   0x80043880
rdx0
rcx   0xc0011029
rax0x202
r8 0x101010101010101
r90x8080808080808080
r10   0xf74eb8da9a13c7be
r11   0x4299783767f1ac05
r12   0x800438a4
r13   0x800020a11000
r14   0x8243dff0cpu_info_full_primary+0x1ff0
r15 0x17
rip   0x811b50f4cpu_fix_msrs+0x1a4
cs   0x8
rflags   0x10202__ALIGN_SIZE+0xf202
rsp   0x829277c0end+0x3277c0
ss  0x10
cpu_fix_msrs+0x1a4: wrmsr
ddb{0}> bt
cpu_fix_msrs(...) at cpu_fix_msrs+0x1a4
cpu_attach(...) at cpu_attach+0x3fd
config_attach(...) at config_attach+0x1f4
acpimadt_attach(...) at acpimadt_attach+0x349
config_attach(...) at config_attach+0x1f4
acpi_attach_common(...) at acpi_attach_common+0x5db
config_attach(...) at config_attach+0x1f4
bios_attach(...) at bios_attach+0x77e
config_attach(...) at config_attach+0x1f4
mainbus_attach(...) at mainbus_attach+0x778
config_attach(...) at config_attach+0x1f4
cpu_configure(...) at cpu_configure+0x33
main(0,0,0,0,0,1) at main+0x3d3
end trace fram: 0x0, count: 13
ddb{0}> mach ddbcpu 1
Invalid cpu 1
ddb{0}> 


I've screenshots of the backtrace, should the function parameters be
required.

If other people are lucky like, remember that you can `boot /obsd` at
the boot prompt to boot an older kernel.

After booting the old kernel, I successfully ran fw_update and fetched
the amd firmware package. Nevertheless, the problem persists. I tried
rerunning `syspatch` and `installboot -v sd0` again, and the problem
still persists. In case it's relevant, the installboot output is

Using / as root
installing bootstrap on /dev/rsd0c
using first-stage /usr/mdec/biosboot, second-stage /usr/mdec/boot
copying /usr/mdec/boot to //boot
looking for superblock at 65536
found valid ffs2 superblock
//boot is 6 blocks x 16384 bytes
fs block shift 2; part offset 64; inode block 56, offset 5232
expecting 64-bit fs blocks (incr 4)
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 80003008
/usr/mdec/biosboot will be written at sector 64


OpenBSD 7.3 (GENERIC.MP) #0: Wed Jul 12 05:09:49 MDT 2023

r...@syspatch-73-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 2080227328 (1983MB)
avail mem = 1997848576 (1905MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf59d0 (10 entries)
bios0: vendor Hetzner version "2017" date 11/11/2017
bios0: Hetzner vServer
acpi0 at bios0: ACPI 3.0
acpi0: sleep states S5
acpi0: tables DSDT FACP APIC HPET MCFG WAET
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD EPYC Processor, 2445.58 MHz, 17-31-00
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,TOPEXT,CPCTR,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 16MB 64b/line 16-way L3 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 1000MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD EPYC Processor, 2445.89 MHz, 17-31-00
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,TOPEXT,CPCTR,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 
8-way L2 cache, 16MB 64b/line 16-way L3 

ldd: check {,p}read return value consistently

2023-07-24 Thread Lucas
Hi,

I wanted to understand how the pledge execpromises commit worked in ldd
and went to read it, and noticed that there is both a 

if (read(fd, , sizeof(ehdr)) < 0) {

and a

if (pread(fd, phdr, size, ehdr.e_phoff) != size) {

In particular, the "read < 0" confused me quite a lot, but the manpage
states that, if the file descriptor _is a regular file_ and there are
enough bytes, it reads to completion. The check for a being a regular
file is already in place, but there is nothing guarding against a short
file, so check instead if read == sizeof(ehdr).

-Lucas


diff refs/heads/master 7e3ddbbb1ef48b81704d0e34d128de01a109fa8c
commit - d50cee607213855e35b101e74926cd801369edd4
commit + 7e3ddbbb1ef48b81704d0e34d128de01a109fa8c
blob - 9e8c5065cd843ff36d91efcb868b94ffd4c98365
blob + ad624d9cd0e72944b93e951de9b31f57a6258601
--- libexec/ld.so/ldd/ldd.c
+++ libexec/ld.so/ldd/ldd.c
@@ -118,7 +118,7 @@ doit(char *name)
return 1;
}
 
-   if (read(fd, , sizeof(ehdr)) < 0) {
+   if (read(fd, , sizeof(ehdr)) != sizeof(ehdr)) {
warn("read(%s)", name);
close(fd);
return 1;



OpenBSD Errata: July 24, 2023 (amd cpu firmware, wscons)

2023-07-24 Thread Alexander Bluhm
Errata patches for AMD cpus with their firmware updates and for
wscons have been released for OpenBSD 7.2 and 7.3.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata72.html
  https://www.openbsd.org/errata73.html

After syspatch on amd64 and i386, "fw_update" and "installboot"
must be run before reboot.  See errata file for detailed instructions.



Zenbleed

2023-07-24 Thread Theo de Raadt
Zenbleed errata for 7.2 and 7.3 will come out soon.

sysupgrade of the -current snapshot already contains a fix.

I wanted to share some notes on impact:

OpenBSD does not use the AVX instructions to the same extent that Linux
and Microsoft do, so this is not as important.

On Linux, glibc has AVX-based optimizations for simple functions (string
and memory copies) which will store secrets into the register file which
can be extracted trivially, so the impact on glibc-based systems is
HUGE.

While working on our fixes, I ran the test programs for quite a while
and I never saw anything resembling a 'text' string.  However when I ran
a browser I saw streams of what was probably graphics-related fragments
flowing past.  The base system clearly uses AVX very rarely by itself.

In summary: in OpenBSD, this isn't a big deal today.  However, attacks
built upon primitives always get better over time, so I urge everyone to
install these workarounds as soon as our errata ship.

--

ps. If you use syspatch for these new errata, you must install the
bootblocks yourself!  syspatch cannot install them for you.  So you must
run this yourself, before the last reboot:

   installboot -v sd0
or
   installboot -v wd0

Our cpu firmware update mechanism uses the bootblocks to load the firmware
from disk and provides it to the kernel, so if you don't have new bootblocks
you won't be protected.



Re: Make USB WiFi drivers more robust

2023-07-24 Thread Martin Pieuchot
On 24/07/23(Mon) 12:07, Mark Kettenis wrote:
> Hi All,
> 
> I recently committed a change to the xhci(4) driver that fixed an
> issue with suspending a machine while it has USB devices plugged in.
> Unfortunately this diff had some unintended side effects.  After
> looking at the way the USB stack works, I've come to the conclusion
> that it is best to try and fix the drivers to self-protect against
> events coming in while the device is being detached.  Some drivers
> already do this, some drivers only do this partially.  The diff below
> makes sure that all of the USB WiFi drivers do this in a consistent
> way by checking that we're in the processes of detaching the devices
> at the following points:

We spend quite some time in the past years trying to get rid of the
usbd_is_dying() mechanism.  I'm quite sad to see such diff.  I've no
idea what the underlying issue is and if an alternative is possible.

The idea is that the USB stack should already take care of this, not
every driver.  Because we've seen it's hard for drivers to do that
correctly.
 
> 1. The driver's ioctl function.
> 
> 2. The driver's USB transfer completion callbacks.

Those are called by usb_transfer_complete().  This correspond to
xhci_xfer_done() and xhci_event_port_change().  Does those functions
need to be called during suspend?

It is not clear to me what your issue is.  I wish we could find a fix
inside xhci(4) or the USB stack.

Thanks,
Martin



Re: Onyx driver set_input support

2023-07-24 Thread Tobias Heider
On Wed, Aug 10, 2022 at 11:08:43AM +, jon@elytron.openbsd.amsterdam wrote:
> Hello everyone. The following diff adds support for
> switching the record.source with the macppc onyx
> driver. I'm still unsure how to get mixerctl or 
> sndctl to set the volume, any hints appreciated.
> 
> Drew some inspiration from 
> https://git.sipsolutions.net/snd-aoa.git

Nice! This does fix the built-in mic on my PowerBook G4.

ok anyone?

> 
> Index: arch/macppc/dev/onyx.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/dev/onyx.c,v
> retrieving revision 1.15
> diff -u -p -u -p -r1.15 onyx.c
> --- arch/macppc/dev/onyx.c21 Mar 2022 19:22:39 -  1.15
> +++ arch/macppc/dev/onyx.c10 Aug 2022 11:07:14 -
> @@ -59,6 +59,7 @@
>  /* PCM3052 registers */
>  #define PCM3052_REG_LEFT_VOLUME  0x41
>  #define PCM3052_REG_RIGHT_VOLUME 0x42
> +#define PCM3052_REG_ADC_CONTROL  0x48
>  
>  /* XXX */
>  #define onyx_softc i2s_softc
> @@ -71,6 +72,7 @@ int onyx_match(struct device *, void *, 
>  void onyx_attach(struct device *, struct device *, void *);
>  void onyx_defer(struct device *);
>  void onyx_set_volume(struct onyx_softc *, int, int);
> +void onyx_set_input(struct onyx_softc *, int);
>  
>  const struct cfattach onyx_ca = {
>   sizeof(struct onyx_softc), onyx_match, onyx_attach
> @@ -143,6 +145,7 @@ onyx_attach(struct device *parent, struc
>   struct onyx_softc *sc = (struct onyx_softc *)self;
>  
>   sc->sc_setvolume = onyx_set_volume;
> + sc->sc_setinput = onyx_set_input;
>  
>   i2s_attach(parent, sc, aux);
>   config_defer(self, onyx_defer);
> @@ -169,6 +172,7 @@ onyx_defer(struct device *dev)
>  
>   deq_reset(sc);
>   onyx_set_volume(sc, 192, 192);
> + onyx_set_input(sc, 1);
>  }
>  
>  void
> @@ -186,4 +190,25 @@ onyx_set_volume(struct onyx_softc *sc, i
>   data = 128 + (right >> 1);
>   kiic_write(sc->sc_i2c, PCM3052_I2C_ADDR,
>   PCM3052_REG_RIGHT_VOLUME, , 1);
> +}
> +
> +void
> +onyx_set_input(struct onyx_softc *sc, int mask)
> +{
> + uint8_t data = 0;
> +
> + sc->sc_record_source = mask;
> +
> + switch (mask) {
> + case1 << 0: /* microphone */
> + data = 0x20;
> + break;
> + case1 << 1: /* line in */
> + data = 0;
> + break;
> + }
> + data |= 12; /* bump volume */
> +
> + kiic_write(sc->sc_i2c, PCM3052_I2C_ADDR,
> + PCM3052_REG_ADC_CONTROL, , 1);
>  }
> 



Re: [Diff] Keyboard backlight support for late powerbooks, plus keybindings

2023-07-24 Thread Tobias Heider
On Sun, Jul 23, 2023 at 09:16:40PM +, jon@elytron.openbsd.amsterdam wrote:
> If I'm not mistaken, all wskbd_{get,set}_backlight uses are in the
> following drivers: acpicbkbd, acpithinkpad, asmc, pwmleds, and now
> my implementation in adb. It is my impression that they are roughly
> the same code, I have collected them below to ease their inspection.

This doesn't look too bad actually but I am not quite sure about pwm.
A common way to solve this is adding a task to do the heavy lifiting and
then invoking that from interrupt context.

In wskbd.c there is a wskbd_brightness_task() which you could use as a
reference for a new wskbd_kbd_backlight_task().

> 
> I would also like to acknowledge the kind help of jcs@, whom I
> approached back when I made my first post on this matter (before
> the key shortcuts, wsconsctl only) upon coming across his acpicbkbd(4).
> 
> (installed in dev/acpi/amsc.c:319)
> 
> int
> asmc_get_backlight(struct wskbd_backlight *kbl)
> {
>   struct asmc_softc *sc = asmc_cd.cd_devs[0];
> 
>   KASSERT(sc != NULL);
>   kbl->min = 0;
>   kbl->max = 0xff;
>   kbl->curval = sc->sc_backlight;
>   return 0;
> }
> 
> int
> asmc_set_backlight(struct wskbd_backlight *kbl)
> {
>   struct asmc_softc *sc = asmc_cd.cd_devs[0];
> 
>   KASSERT(sc != NULL);
>   if (kbl->curval > 0xff)
> return EINVAL;
>   sc->sc_backlight = kbl->curval;
>   task_add(systq, >sc_task_backlight);
>   return 0;
> }
> 
> (installed in dev/acpi/acpicbkbd.c:94)
> 
> int
> acpicbkbd_get_backlight(struct wskbd_backlight *kbl)
> {
>   struct acpicbkbd_softc *sc = acpicbkbd_cd.cd_devs[0];
> 
>   KASSERT(sc != NULL);
> 
>   kbl->min = 0;
>   kbl->max = ACPICBKBD_MAX_BACKLIGHT;
>   kbl->curval = sc->sc_backlight;
> 
>   return 0;
> }
> 
> int
> acpicbkbd_set_backlight(struct wskbd_backlight *kbl)
> {
>   struct acpicbkbd_softc *sc = acpicbkbd_cd.cd_devs[0];
> 
>   KASSERT(sc != NULL);
> 
>   if (kbl->curval > ACPICBKBD_MAX_BACKLIGHT)
> return EINVAL;
> 
>   sc->sc_backlight = kbl->curval;
> 
>   acpi_addtask(sc->sc_acpi, acpicbkbd_write_backlight, sc, 0);
>   acpi_wakeup(sc->sc_acpi);
> 
>   return 0;
> }
> 
> (installed in dev/acpi/acpithinkpad.c:347)
> 
> int
> thinkpad_get_kbd_backlight(struct wskbd_backlight *kbl)
> {
>   struct acpithinkpad_softc *sc = acpithinkpad_cd.cd_devs[0];
> 
>   KASSERT(sc != NULL);
> 
>   kbl->min = 0;
>   kbl->max = (sc->sc_thinklight >> 8) & 0x0f;
>   kbl->curval = sc->sc_thinklight & 0x0f;
> 
>   if (kbl->max == 0)
> return (ENOTTY);
> 
>   return 0;
> }
> 
> int
> thinkpad_set_kbd_backlight(struct wskbd_backlight *kbl)
> {
>   struct acpithinkpad_softc *sc = acpithinkpad_cd.cd_devs[0];
>   int maxval;
> 
>   KASSERT(sc != NULL);
> 
>   maxval = (sc->sc_thinklight >> 8) & 0x0f;
> 
>   if (maxval == 0)
> return (ENOTTY);
> 
>   if (kbl->curval > maxval)
> return EINVAL;
> 
>   sc->sc_thinklight &= ~0xff;
>   sc->sc_thinklight |= kbl->curval;
>   acpi_addtask(sc->sc_acpi, thinkpad_set_thinklight, sc, 0);
>   acpi_wakeup(sc->sc_acpi);
>   return 0;
> }
> 
> (installed in pwmleds.c:102)
> int
> pwmleds_get_kbd_backlight(struct wskbd_backlight *kbl)
> {
>   struct pwmleds_softc *sc;
>   struct pwm_state ps;
>   int error;
> 
>   sc = pwmleds_kbd_backlight();
>   if (sc == NULL)
> return ENOTTY;
> 
>   error = pwm_get_state(sc->sc_pwm, );
>   if (error)
> return error;
> 
>   kbl->min = 0;
>   kbl->max = sc->sc_max_brightness;
>   kbl->curval = (ps.ps_enabled) ?
>   ((uint64_t)ps.ps_pulse_width * kbl->max) / ps.ps_period : 0;
>   return 0;
> }
> 
> int
> pwmleds_set_kbd_backlight(struct wskbd_backlight *kbl)
> {
>   struct pwmleds_softc *sc;
>   struct pwm_state ps;
> 
>   sc = pwmleds_kbd_backlight();
>   if (sc == NULL)
> return ENOTTY;
> 
>   if (kbl->curval < 0 || kbl->curval > sc->sc_max_brightness)
> return EINVAL;
> 
>   pwm_init_state(sc->sc_pwm, );
>   ps.ps_pulse_width =
>   ((uint64_t)kbl->curval * ps.ps_period) / sc->sc_max_brightness;
>   ps.ps_enabled = (ps.ps_pulse_width > 0);
>   return pwm_set_state(sc->sc_pwm, );
> }



Make USB WiFi drivers more robust

2023-07-24 Thread Mark Kettenis
Hi All,

I recently committed a change to the xhci(4) driver that fixed an
issue with suspending a machine while it has USB devices plugged in.
Unfortunately this diff had some unintended side effects.  After
looking at the way the USB stack works, I've come to the conclusion
that it is best to try and fix the drivers to self-protect against
events coming in while the device is being detached.  Some drivers
already do this, some drivers only do this partially.  The diff below
makes sure that all of the USB WiFi drivers do this in a consistent
way by checking that we're in the processes of detaching the devices
at the following points:

1. The driver's ioctl function.

2. The driver's USB transfer completion callbacks.

I may have missed some other points that need protection.  So it would
be great if users of USB WiFi devices could test this diff and try to
suspend or hibernate their machine with the WiFi interface active.

My plan is to only commit the xhci.c bit after checking the other USB
drivers for potential problems.

ok?


Index: dev/usb/if_athn_usb.c
===
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.65
diff -u -p -r1.65 if_athn_usb.c
--- dev/usb/if_athn_usb.c   10 Jul 2022 21:13:41 -  1.65
+++ dev/usb/if_athn_usb.c   24 Jul 2023 09:55:08 -
@@ -2131,6 +2131,9 @@ athn_usb_rxeof(struct usbd_xfer *xfer, v
uint16_t pktlen;
int off, len;
 
+   if (usbd_is_dying(usc->sc_udev))
+   return;
+
if (__predict_false(status != USBD_NORMAL_COMPLETION)) {
DPRINTF(("RX status=%d\n", status));
if (status == USBD_STALLED)
@@ -2240,6 +2243,9 @@ athn_usb_txeof(struct usbd_xfer *xfer, v
struct athn_softc *sc = >sc_sc;
struct ifnet *ifp = >sc_ic.ic_if;
int s;
+
+   if (usbd_is_dying(usc->sc_udev))
+   return;
 
s = splnet();
/* Put this Tx buffer back to our free list. */
Index: dev/usb/if_atu.c
===
RCS file: /cvs/src/sys/dev/usb/if_atu.c,v
retrieving revision 1.134
diff -u -p -r1.134 if_atu.c
--- dev/usb/if_atu.c21 Apr 2022 21:03:03 -  1.134
+++ dev/usb/if_atu.c24 Jul 2023 09:55:09 -
@@ -1774,6 +1774,9 @@ atu_txeof(struct usbd_xfer *xfer, void *
c->atu_mbuf = NULL;
}
 
+   if (usbd_is_dying(sc->atu_udev))
+   return;
+
if (status != USBD_NORMAL_COMPLETION) {
if (status == USBD_NOT_STARTED || status == USBD_CANCELLED)
return;
@@ -2109,6 +2112,9 @@ atu_ioctl(struct ifnet *ifp, u_long comm
 {
struct atu_softc*sc = ifp->if_softc;
int err = 0, s;
+
+   if (usbd_is_dying(sc->atu_udev))
+   return ENXIO;
 
s = splnet();
switch (command) {
Index: dev/usb/if_mtw.c
===
RCS file: /cvs/src/sys/dev/usb/if_mtw.c,v
retrieving revision 1.8
diff -u -p -r1.8 if_mtw.c
--- dev/usb/if_mtw.c8 Mar 2023 04:43:08 -   1.8
+++ dev/usb/if_mtw.c24 Jul 2023 09:55:09 -
@@ -2140,6 +2140,9 @@ mtw_rxeof(struct usbd_xfer *xfer, void *
uint32_t dmalen;
int xferlen;
 
+   if (usbd_is_dying(sc->sc_udev))
+   return;
+
if (__predict_false(status != USBD_NORMAL_COMPLETION)) {
DPRINTF(("RX status=%d\n", status));
if (status == USBD_STALLED)
Index: dev/usb/if_otus.c
===
RCS file: /cvs/src/sys/dev/usb/if_otus.c,v
retrieving revision 1.72
diff -u -p -r1.72 if_otus.c
--- dev/usb/if_otus.c   8 Mar 2023 04:43:08 -   1.72
+++ dev/usb/if_otus.c   24 Jul 2023 09:55:09 -
@@ -995,6 +995,9 @@ otus_cmd_rxeof(struct otus_softc *sc, ui
struct ar_cmd_hdr *hdr;
int s;
 
+   if (usbd_is_dying(sc->sc_udev))
+   return;
+
if (__predict_false(len < sizeof (*hdr))) {
DPRINTF(("cmd too small %d\n", len));
return;
@@ -1256,6 +1259,9 @@ otus_txeof(struct usbd_xfer *xfer, void 
struct ieee80211com *ic = >sc_ic;
struct ifnet *ifp = >ic_if;
int s;
+
+   if (usbd_is_dying(sc->sc_udev))
+   return;
 
s = splnet();
sc->tx_queued--;
Index: dev/usb/if_ral.c
===
RCS file: /cvs/src/sys/dev/usb/if_ral.c,v
retrieving revision 1.149
diff -u -p -r1.149 if_ral.c
--- dev/usb/if_ral.c21 Apr 2022 21:03:03 -  1.149
+++ dev/usb/if_ral.c24 Jul 2023 09:55:09 -
@@ -648,6 +648,9 @@ ural_txeof(struct usbd_xfer *xfer, void 
struct ifnet *ifp = >ic_if;
int s;
 
+   if (usbd_is_dying(sc->sc_udev))
+   return;
+
if (status != USBD_NORMAL_COMPLETION) {