Re: upd(4) page fault since 7.0
On Fri, Oct 29, 2021 at 08:56:36AM +0200, Damien Couderc wrote:
> Le 28/10/2021 à 21:09, Anton Lindqvist a écrit :
>
>
> Hi Anton,
>
> It works great!
>
> See below the resulting dmesg with option UPD_DEBUG.
>
> Any chance this fix will come through syspatch?
I just committed a slightly tweaked version as
UHIDEV_CLAIM_MULTIPLE_REPORTID can be defined to 256 to avoid the
conflict according to the USB HID specification.
This will soon be available in -current snapshots.
Index: dev/usb/uhidev.h
===
RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.32
diff -u -p -r1.32 uhidev.h
--- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
+++ dev/usb/uhidev.h29 Oct 2021 15:35:43 -
@@ -80,9 +80,9 @@ struct uhidev {
struct uhidev_attach_arg {
struct usb_attach_arg *uaa;
struct uhidev_softc *parent;
- uint8_t reportid;
-#defineUHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+ u_intreportid;
+#defineUHIDEV_CLAIM_MULTIPLE_REPORTID 256
+ u_intnreports;
uint8_t *claimed;
};
Re: upd(4) page fault since 7.0
Le 28/10/2021 à 21:09, Anton Lindqvist a écrit : Hi Anton, It works great! See below the resulting dmesg with option UPD_DEBUG. Any chance this fix will come through syspatch? OpenBSD 7.0-stable (UPD.MP) #20: Fri Oct 29 08:41:57 CEST 2021 damien@gageac.petrolan:/usr/src/sys/arch/amd64/compile/UPD.MP real mem = 6283997184 (5992MB) avail mem = 6077521920 (5795MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xc8889000 (57 entries) bios0: vendor LENOVO version "R19ET36W (1.20 )" date 07/12/2021 bios0: LENOVO 20U70003FR acpi0 at bios0: ACPI 6.3 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SSDT SSDT IVRS SSDT MSDM BATB HPET APIC MCFG SBST WSMT SSDT SSDT CRAT CDIT FPDT SSDT SSDT SSDT UEFI SSDT SSDT acpi0: wakeup devices GPP0(S4) GPP3(S4) GPP4(S4) GPP5(S4) GP17(S4) XHC0(S3) XHC1(S3) GP19(S4) LID_(S4) SLPB(S3) acpitimer0 at acpi0: 3579545 Hz, 32 bits acpihpet0 at acpi0: 14318180 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD Ryzen 5 4500U with Radeon Graphics, 2370.90 MHz, 17-60-01 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,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=1.1, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 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,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu1: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu1: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu2: 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,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu2: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu2: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu2: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 4 (application processor) cpu3: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu3: 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,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu3: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache cpu3: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu3: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu3: disabling user TSC (skew=226) cpu3: smt 0, core 4, package 0 cpu4 at mainbus0: apid 5 (application processor) cpu4: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu4:
Re: upd(4) page fault since 7.0
On Thu, Oct 28, 2021 at 08:40:03PM +0200, Anton Lindqvist wrote:
> On Thu, Oct 28, 2021 at 08:08:36PM +0200, Damien Couderc wrote:
> > Le 26/10/2021 à 19:03, Anton Lindqvist a écrit :
> > > On Tue, Oct 26, 2021 at 05:58:12PM +0200, Damien Couderc wrote:
> > > > Le 26/10/2021 à 16:11, Anton Lindqvist a écrit :
> > > > > On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
> > > > > > Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
> > > > > > > On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
> > > > > > > > Hi,
> > > > > > > > I got a page fault with upd(4) since 7.0.
> > > > > > > >
> > > > > > > > The problem began with the last revision of upd.c (1.30):
> > > > > > > >
> > > > > > > > ===
> > > > > > > > RCS file: /cvs/src/sys/dev/usb/upd.c,v
> > > > > > > > retrieving revision 1.29
> > > > > > > > retrieving revision 1.30
> > > > > > > > diff -u -r1.29 -r1.30
> > > > > > > > --- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
> > > > > > > > +++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
> > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > -/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
> > > > > > > > +/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp
> > > > > > > > $ */
> > > > > > > >
> > > > > > > > /*
> > > > > > > > * Copyright (c) 2015 David Higgs
> > > > > > > > @@ -167,7 +167,7 @@
> > > > > > > > if (upd_lookup_usage_entry(desc, size,
> > > > > > > > upd_usage_roots + i, )) {
> > > > > > > > ret = UMATCH_VENDOR_PRODUCT;
> > > > > > > > - break;
> > > > > > > > + uha->claimed[item.report_ID] = 1;
> > > > > > > > }
> > > > > > > >
> > > > > > > > return (ret);
> > > > > > > >
> > > > > > > > ===
> > > > > > > >
> > > > > > > > The uha.claimed array is allocated using uha.nreports as its
> > > > > > > > size while
> > > > > > > > upd_match() is looping through the number of items of
> > > > > > > > upd_usage_roots.
> > > > > > > >
> > > > > > > > In my case uha.nreports is equal to zero so uha.claimed is
> > > > > > > > null, hence
> > > > > > > > setting uha->claimed[n] is failing.
> > > > > > > >
> > > > > > > > As I'm not familiar with the HID code I did not yet understood
> > > > > > > > the
> > > > > > > > relation between upd_usage_roots and the claimed array but as
> > > > > > > > we're
> > > > > > > > talking about UPS attached computers I though the issue would
> > > > > > > > sensible
> > > > > > > > enough to make a quick reporting.
> > > > > > > >
> > > > > > > > You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set
> > > > > > > > and the
> > > > > > > > following patch applied to circumvent the page fault and
> > > > > > > > provide some debug:
> > > > > > > Could you try the following diff, looks like an unsigned wrap
> > > > > > > around.
> > > > > > >
> > > > > > > Index: dev/usb/uhidev.h
> > > > > > > ===
> > > > > > > RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
> > > > > > > retrieving revision 1.32
> > > > > > > diff -u -p -r1.32 uhidev.h
> > > > > > > --- dev/usb/uhidev.h 12 Sep 2021 06:58:08 - 1.32
> > > > > > > +++ dev/usb/uhidev.h 24 Oct 2021 19:44:52 -
> > > > > > > @@ -82,7 +82,7 @@ struct uhidev_attach_arg {
> > > > > > > struct uhidev_softc *parent;
> > > > > > > uint8_t reportid;
> > > > > > > #define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
> > > > > > > - uint8_t nreports;
> > > > > > > + u_intnreports;
> > > > > > > uint8_t *claimed;
> > > > > > > };
> > > > > > >
> > > > > > Hello Anton,
> > > > > >
> > > > > > I made a quick test and nreports is now set with 256 but I still
> > > > > > get the
> > > > > > page fault.
> > > > > >
> > > > > > I'll check the details ASAP.
> > > > >
> > > > > Do you have a backtrace?
> > > > >
> > > >
> > > > I only have screenshots of ddb.
> > > >
> > > > With the default kernel :
> > > > http://cromagnon.petrocore.eu/ss_upd_ddb_default.jpg
> > > >
> > > > With a custom kernel for a bit more data :
> > > > http://cromagnon.petrocore.eu/ss_upd_ddb_custom.jpg
> > >
> > > Looks like a NULL deref. My guess would be item.report_ID going out of
> > > bounds on uha->claimed. Inspecting item.report_ID could provide some
> > > hints. This in turn could be caused by uhidev_maxrepid() looking for
> > > hid_none items whereas upd_lookup_usage_entry() uses hid_feature.
> > >
> > > Also, it would be interesting to see the descriptor of this device. If
> > > you make upd_match() unconditionally return 0 it should hopefully^W
> > > attach as uhid device; making it possible to
Re: upd(4) page fault since 7.0
On Thu, Oct 28, 2021 at 08:08:36PM +0200, Damien Couderc wrote:
> Le 26/10/2021 à 19:03, Anton Lindqvist a écrit :
> > On Tue, Oct 26, 2021 at 05:58:12PM +0200, Damien Couderc wrote:
> > > Le 26/10/2021 à 16:11, Anton Lindqvist a écrit :
> > > > On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
> > > > > Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
> > > > > > On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
> > > > > > > Hi,
> > > > > > > I got a page fault with upd(4) since 7.0.
> > > > > > >
> > > > > > > The problem began with the last revision of upd.c (1.30):
> > > > > > >
> > > > > > > ===
> > > > > > > RCS file: /cvs/src/sys/dev/usb/upd.c,v
> > > > > > > retrieving revision 1.29
> > > > > > > retrieving revision 1.30
> > > > > > > diff -u -r1.29 -r1.30
> > > > > > > --- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
> > > > > > > +++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
> > > > > > > @@ -1,4 +1,4 @@
> > > > > > > -/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
> > > > > > > +/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp
> > > > > > > $ */
> > > > > > >
> > > > > > > /*
> > > > > > > * Copyright (c) 2015 David Higgs
> > > > > > > @@ -167,7 +167,7 @@
> > > > > > > if (upd_lookup_usage_entry(desc, size,
> > > > > > > upd_usage_roots + i, )) {
> > > > > > > ret = UMATCH_VENDOR_PRODUCT;
> > > > > > > - break;
> > > > > > > + uha->claimed[item.report_ID] = 1;
> > > > > > > }
> > > > > > >
> > > > > > > return (ret);
> > > > > > >
> > > > > > > ===
> > > > > > >
> > > > > > > The uha.claimed array is allocated using uha.nreports as its size
> > > > > > > while
> > > > > > > upd_match() is looping through the number of items of
> > > > > > > upd_usage_roots.
> > > > > > >
> > > > > > > In my case uha.nreports is equal to zero so uha.claimed is null,
> > > > > > > hence
> > > > > > > setting uha->claimed[n] is failing.
> > > > > > >
> > > > > > > As I'm not familiar with the HID code I did not yet understood the
> > > > > > > relation between upd_usage_roots and the claimed array but as
> > > > > > > we're
> > > > > > > talking about UPS attached computers I though the issue would
> > > > > > > sensible
> > > > > > > enough to make a quick reporting.
> > > > > > >
> > > > > > > You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set
> > > > > > > and the
> > > > > > > following patch applied to circumvent the page fault and provide
> > > > > > > some debug:
> > > > > > Could you try the following diff, looks like an unsigned wrap
> > > > > > around.
> > > > > >
> > > > > > Index: dev/usb/uhidev.h
> > > > > > ===
> > > > > > RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
> > > > > > retrieving revision 1.32
> > > > > > diff -u -p -r1.32 uhidev.h
> > > > > > --- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
> > > > > > +++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
> > > > > > @@ -82,7 +82,7 @@ struct uhidev_attach_arg {
> > > > > > struct uhidev_softc *parent;
> > > > > > uint8_t reportid;
> > > > > > #define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
> > > > > > - uint8_t nreports;
> > > > > > + u_intnreports;
> > > > > > uint8_t *claimed;
> > > > > > };
> > > > > >
> > > > > Hello Anton,
> > > > >
> > > > > I made a quick test and nreports is now set with 256 but I still get
> > > > > the
> > > > > page fault.
> > > > >
> > > > > I'll check the details ASAP.
> > > >
> > > > Do you have a backtrace?
> > > >
> > >
> > > I only have screenshots of ddb.
> > >
> > > With the default kernel :
> > > http://cromagnon.petrocore.eu/ss_upd_ddb_default.jpg
> > >
> > > With a custom kernel for a bit more data :
> > > http://cromagnon.petrocore.eu/ss_upd_ddb_custom.jpg
> >
> > Looks like a NULL deref. My guess would be item.report_ID going out of
> > bounds on uha->claimed. Inspecting item.report_ID could provide some
> > hints. This in turn could be caused by uhidev_maxrepid() looking for
> > hid_none items whereas upd_lookup_usage_entry() uses hid_feature.
> >
> > Also, it would be interesting to see the descriptor of this device. If
> > you make upd_match() unconditionally return 0 it should hopefully^W
> > attach as uhid device; making it possible to extract the report using
> > usbhidctl replacing X with the attached device:
> >
> > # usbhidctl -R -f /dev/uhidX
> >
>
> Hi !
>
> I set uhidevdebug to enable more debug messages and it gets more
> interesting as we can see that we enter twice in upd_match() but the
> second time
Re: upd(4) page fault since 7.0
Le 26/10/2021 à 19:03, Anton Lindqvist a écrit :
On Tue, Oct 26, 2021 at 05:58:12PM +0200, Damien Couderc wrote:
Le 26/10/2021 à 16:11, Anton Lindqvist a écrit :
On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
Hi,
I got a page fault with upd(4) since 7.0.
The problem began with the last revision of upd.c (1.30):
===
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
+++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
@@ -1,4 +1,4 @@
-/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
+/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
/*
* Copyright (c) 2015 David Higgs
@@ -167,7 +167,7 @@
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
- break;
+ uha->claimed[item.report_ID] = 1;
}
return (ret);
===
The uha.claimed array is allocated using uha.nreports as its size while
upd_match() is looping through the number of items of upd_usage_roots.
In my case uha.nreports is equal to zero so uha.claimed is null, hence
setting uha->claimed[n] is failing.
As I'm not familiar with the HID code I did not yet understood the
relation between upd_usage_roots and the claimed array but as we're
talking about UPS attached computers I though the issue would sensible
enough to make a quick reporting.
You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
following patch applied to circumvent the page fault and provide some debug:
Could you try the following diff, looks like an unsigned wrap around.
Index: dev/usb/uhidev.h
===
RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.32
diff -u -p -r1.32 uhidev.h
--- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
+++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
@@ -82,7 +82,7 @@ struct uhidev_attach_arg {
struct uhidev_softc *parent;
uint8_t reportid;
#define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+ u_intnreports;
uint8_t *claimed;
};
Hello Anton,
I made a quick test and nreports is now set with 256 but I still get the
page fault.
I'll check the details ASAP.
Do you have a backtrace?
I only have screenshots of ddb.
With the default kernel :
http://cromagnon.petrocore.eu/ss_upd_ddb_default.jpg
With a custom kernel for a bit more data :
http://cromagnon.petrocore.eu/ss_upd_ddb_custom.jpg
Looks like a NULL deref. My guess would be item.report_ID going out of
bounds on uha->claimed. Inspecting item.report_ID could provide some
hints. This in turn could be caused by uhidev_maxrepid() looking for
hid_none items whereas upd_lookup_usage_entry() uses hid_feature.
Also, it would be interesting to see the descriptor of this device. If
you make upd_match() unconditionally return 0 it should hopefully^W
attach as uhid device; making it possible to extract the report using
usbhidctl replacing X with the attached device:
# usbhidctl -R -f /dev/uhidX
Hi !
I set uhidevdebug to enable more debug messages and it gets more
interesting as we can see that we enter twice in upd_match() but the
second time uha.claimed is not allocated.
I'll dig for more ASAP.
Which uhid device would you want to get the report for ?
Below the diff of the changes I made yet and the corresponding dmesg.
Index: uhidev.c
===
RCS file: /home/cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.95
diff -u -p -r1.95 uhidev.c
--- uhidev.c12 Sep 2021 06:58:08 - 1.95
+++ uhidev.c28 Oct 2021 17:53:55 -
@@ -72,7 +72,7 @@ int uhidev_use_rdesc(struct uhidev_softc
#ifdef UHIDEV_DEBUG
#define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
#define DPRINTFN(n,x) do { if (uhidevdebug>(n)) printf x; } while (0)
-intuhidevdebug = 0;
+intuhidevdebug = 1;
#else
#define DPRINTF(x)
#define DPRINTFN(n,x)
@@ -252,6 +252,7 @@ uhidev_attach(struct device *parent, str
uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
uha.nreports = nrepid;
uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
+DPRINTF(("uhidev_attach: uha.claimed allocated\n"));
/* Look for a driver claiming multiple report IDs first. */
dev = config_found_sm(self, , NULL, uhidevsubmatch);
@@ -270,6 +271,7 @@ uhidev_attach(struct device *parent, str
Re: upd(4) page fault since 7.0
On Tue, Oct 26, 2021 at 05:58:12PM +0200, Damien Couderc wrote:
> Le 26/10/2021 à 16:11, Anton Lindqvist a écrit :
> > On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
> > > Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
> > > > On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
> > > > > Hi,
> > > > > I got a page fault with upd(4) since 7.0.
> > > > >
> > > > > The problem began with the last revision of upd.c (1.30):
> > > > >
> > > > > ===
> > > > > RCS file: /cvs/src/sys/dev/usb/upd.c,v
> > > > > retrieving revision 1.29
> > > > > retrieving revision 1.30
> > > > > diff -u -r1.29 -r1.30
> > > > > --- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
> > > > > +++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
> > > > > @@ -1,4 +1,4 @@
> > > > > -/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
> > > > > +/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
> > > > >
> > > > >/*
> > > > > * Copyright (c) 2015 David Higgs
> > > > > @@ -167,7 +167,7 @@
> > > > > if (upd_lookup_usage_entry(desc, size,
> > > > > upd_usage_roots + i, )) {
> > > > > ret = UMATCH_VENDOR_PRODUCT;
> > > > > - break;
> > > > > + uha->claimed[item.report_ID] = 1;
> > > > > }
> > > > >
> > > > > return (ret);
> > > > >
> > > > > ===
> > > > >
> > > > > The uha.claimed array is allocated using uha.nreports as its size
> > > > > while
> > > > > upd_match() is looping through the number of items of upd_usage_roots.
> > > > >
> > > > > In my case uha.nreports is equal to zero so uha.claimed is null, hence
> > > > > setting uha->claimed[n] is failing.
> > > > >
> > > > > As I'm not familiar with the HID code I did not yet understood the
> > > > > relation between upd_usage_roots and the claimed array but as we're
> > > > > talking about UPS attached computers I though the issue would sensible
> > > > > enough to make a quick reporting.
> > > > >
> > > > > You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and
> > > > > the
> > > > > following patch applied to circumvent the page fault and provide some
> > > > > debug:
> > > > Could you try the following diff, looks like an unsigned wrap around.
> > > >
> > > > Index: dev/usb/uhidev.h
> > > > ===
> > > > RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
> > > > retrieving revision 1.32
> > > > diff -u -p -r1.32 uhidev.h
> > > > --- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
> > > > +++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
> > > > @@ -82,7 +82,7 @@ struct uhidev_attach_arg {
> > > > struct uhidev_softc *parent;
> > > > uint8_t reportid;
> > > >#define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
> > > > - uint8_t nreports;
> > > > + u_intnreports;
> > > > uint8_t *claimed;
> > > >};
> > > >
> > > Hello Anton,
> > >
> > > I made a quick test and nreports is now set with 256 but I still get the
> > > page fault.
> > >
> > > I'll check the details ASAP.
> >
> > Do you have a backtrace?
> >
>
> I only have screenshots of ddb.
>
> With the default kernel :
> http://cromagnon.petrocore.eu/ss_upd_ddb_default.jpg
>
> With a custom kernel for a bit more data :
> http://cromagnon.petrocore.eu/ss_upd_ddb_custom.jpg
Looks like a NULL deref. My guess would be item.report_ID going out of
bounds on uha->claimed. Inspecting item.report_ID could provide some
hints. This in turn could be caused by uhidev_maxrepid() looking for
hid_none items whereas upd_lookup_usage_entry() uses hid_feature.
Also, it would be interesting to see the descriptor of this device. If
you make upd_match() unconditionally return 0 it should hopefully^W
attach as uhid device; making it possible to extract the report using
usbhidctl replacing X with the attached device:
# usbhidctl -R -f /dev/uhidX
Re: upd(4) page fault since 7.0
Le 26/10/2021 à 16:11, Anton Lindqvist a écrit :
On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
Hi,
I got a page fault with upd(4) since 7.0.
The problem began with the last revision of upd.c (1.30):
===
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
+++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
@@ -1,4 +1,4 @@
-/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
+/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
/*
* Copyright (c) 2015 David Higgs
@@ -167,7 +167,7 @@
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
- break;
+ uha->claimed[item.report_ID] = 1;
}
return (ret);
===
The uha.claimed array is allocated using uha.nreports as its size while
upd_match() is looping through the number of items of upd_usage_roots.
In my case uha.nreports is equal to zero so uha.claimed is null, hence
setting uha->claimed[n] is failing.
As I'm not familiar with the HID code I did not yet understood the
relation between upd_usage_roots and the claimed array but as we're
talking about UPS attached computers I though the issue would sensible
enough to make a quick reporting.
You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
following patch applied to circumvent the page fault and provide some debug:
Could you try the following diff, looks like an unsigned wrap around.
Index: dev/usb/uhidev.h
===
RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.32
diff -u -p -r1.32 uhidev.h
--- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
+++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
@@ -82,7 +82,7 @@ struct uhidev_attach_arg {
struct uhidev_softc *parent;
uint8_t reportid;
#define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+ u_intnreports;
uint8_t *claimed;
};
Hello Anton,
I made a quick test and nreports is now set with 256 but I still get the
page fault.
I'll check the details ASAP.
Do you have a backtrace?
I only have screenshots of ddb.
With the default kernel :
http://cromagnon.petrocore.eu/ss_upd_ddb_default.jpg
With a custom kernel for a bit more data :
http://cromagnon.petrocore.eu/ss_upd_ddb_custom.jpg
With luck I'll have some time to go deeper this evening.
Thanks,
Damien
Re: upd(4) page fault since 7.0
On Tue, Oct 26, 2021 at 09:50:41AM +0200, Damien Couderc wrote:
> Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
> > On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
> > > Hi,
> > > I got a page fault with upd(4) since 7.0.
> > >
> > > The problem began with the last revision of upd.c (1.30):
> > >
> > > ===
> > > RCS file: /cvs/src/sys/dev/usb/upd.c,v
> > > retrieving revision 1.29
> > > retrieving revision 1.30
> > > diff -u -r1.29 -r1.30
> > > --- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
> > > +++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
> > > @@ -1,4 +1,4 @@
> > > -/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
> > > +/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
> > >
> > > /*
> > >* Copyright (c) 2015 David Higgs
> > > @@ -167,7 +167,7 @@
> > > if (upd_lookup_usage_entry(desc, size,
> > > upd_usage_roots + i, )) {
> > > ret = UMATCH_VENDOR_PRODUCT;
> > > - break;
> > > + uha->claimed[item.report_ID] = 1;
> > > }
> > >
> > > return (ret);
> > >
> > > ===
> > >
> > > The uha.claimed array is allocated using uha.nreports as its size while
> > > upd_match() is looping through the number of items of upd_usage_roots.
> > >
> > > In my case uha.nreports is equal to zero so uha.claimed is null, hence
> > > setting uha->claimed[n] is failing.
> > >
> > > As I'm not familiar with the HID code I did not yet understood the
> > > relation between upd_usage_roots and the claimed array but as we're
> > > talking about UPS attached computers I though the issue would sensible
> > > enough to make a quick reporting.
> > >
> > > You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
> > > following patch applied to circumvent the page fault and provide some
> > > debug:
> > Could you try the following diff, looks like an unsigned wrap around.
> >
> > Index: dev/usb/uhidev.h
> > ===
> > RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
> > retrieving revision 1.32
> > diff -u -p -r1.32 uhidev.h
> > --- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
> > +++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
> > @@ -82,7 +82,7 @@ struct uhidev_attach_arg {
> > struct uhidev_softc *parent;
> > uint8_t reportid;
> > #define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
> > - uint8_t nreports;
> > + u_intnreports;
> > uint8_t *claimed;
> > };
> >
> Hello Anton,
>
> I made a quick test and nreports is now set with 256 but I still get the
> page fault.
>
> I'll check the details ASAP.
Do you have a backtrace?
Re: upd(4) page fault since 7.0
Le 24/10/2021 à 21:45, Anton Lindqvist a écrit :
On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
Hi,
I got a page fault with upd(4) since 7.0.
The problem began with the last revision of upd.c (1.30):
===
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
+++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
@@ -1,4 +1,4 @@
-/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
+/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
/*
* Copyright (c) 2015 David Higgs
@@ -167,7 +167,7 @@
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
- break;
+ uha->claimed[item.report_ID] = 1;
}
return (ret);
===
The uha.claimed array is allocated using uha.nreports as its size while
upd_match() is looping through the number of items of upd_usage_roots.
In my case uha.nreports is equal to zero so uha.claimed is null, hence
setting uha->claimed[n] is failing.
As I'm not familiar with the HID code I did not yet understood the
relation between upd_usage_roots and the claimed array but as we're
talking about UPS attached computers I though the issue would sensible
enough to make a quick reporting.
You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
following patch applied to circumvent the page fault and provide some debug:
Could you try the following diff, looks like an unsigned wrap around.
Index: dev/usb/uhidev.h
===
RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.32
diff -u -p -r1.32 uhidev.h
--- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
+++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
@@ -82,7 +82,7 @@ struct uhidev_attach_arg {
struct uhidev_softc *parent;
uint8_t reportid;
#define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+ u_intnreports;
uint8_t *claimed;
};
Hello Anton,
I made a quick test and nreports is now set with 256 but I still get the
page fault.
I'll check the details ASAP.
Thank you,
Damien
Re: upd(4) page fault since 7.0
On Sun, Oct 24, 2021 at 03:03:01PM +0200, Damien Couderc wrote:
> Hi,
> I got a page fault with upd(4) since 7.0.
>
> The problem began with the last revision of upd.c (1.30):
>
> ===
> RCS file: /cvs/src/sys/dev/usb/upd.c,v
> retrieving revision 1.29
> retrieving revision 1.30
> diff -u -r1.29 -r1.30
> --- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
> +++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
> +/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
>
> /*
> * Copyright (c) 2015 David Higgs
> @@ -167,7 +167,7 @@
> if (upd_lookup_usage_entry(desc, size,
> upd_usage_roots + i, )) {
> ret = UMATCH_VENDOR_PRODUCT;
> - break;
> + uha->claimed[item.report_ID] = 1;
> }
>
> return (ret);
>
> ===
>
> The uha.claimed array is allocated using uha.nreports as its size while
> upd_match() is looping through the number of items of upd_usage_roots.
>
> In my case uha.nreports is equal to zero so uha.claimed is null, hence
> setting uha->claimed[n] is failing.
>
> As I'm not familiar with the HID code I did not yet understood the
> relation between upd_usage_roots and the claimed array but as we're
> talking about UPS attached computers I though the issue would sensible
> enough to make a quick reporting.
>
> You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
> following patch applied to circumvent the page fault and provide some debug:
Could you try the following diff, looks like an unsigned wrap around.
Index: dev/usb/uhidev.h
===
RCS file: /cvs/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.32
diff -u -p -r1.32 uhidev.h
--- dev/usb/uhidev.h12 Sep 2021 06:58:08 - 1.32
+++ dev/usb/uhidev.h24 Oct 2021 19:44:52 -
@@ -82,7 +82,7 @@ struct uhidev_attach_arg {
struct uhidev_softc *parent;
uint8_t reportid;
#defineUHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+ u_intnreports;
uint8_t *claimed;
};
upd(4) page fault since 7.0
Hi,
I got a page fault with upd(4) since 7.0.
The problem began with the last revision of upd.c (1.30):
===
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- src/sys/dev/usb/upd.c 2021/03/08 14:35:57 1.29
+++ src/sys/dev/usb/upd.c 2021/08/06 17:46:45 1.30
@@ -1,4 +1,4 @@
-/* $OpenBSD: upd.c,v 1.29 2021/03/08 14:35:57 jcs Exp $ */
+/* $OpenBSD: upd.c,v 1.30 2021/08/06 17:46:45 abieber Exp $ */
/*
* Copyright (c) 2015 David Higgs
@@ -167,7 +167,7 @@
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
- break;
+ uha->claimed[item.report_ID] = 1;
}
return (ret);
===
The uha.claimed array is allocated using uha.nreports as its size while
upd_match() is looping through the number of items of upd_usage_roots.
In my case uha.nreports is equal to zero so uha.claimed is null, hence
setting uha->claimed[n] is failing.
As I'm not familiar with the HID code I did not yet understood the
relation between upd_usage_roots and the claimed array but as we're
talking about UPS attached computers I though the issue would sensible
enough to make a quick reporting.
You'll find a dmesg with options UPD_DEBUG and UHIDEV_DEBUG set and the
following patch applied to circumvent the page fault and provide some debug:
===
RCS file: /home/cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.30
diff -u -p -r1.30 upd.c
--- upd.c 6 Aug 2021 17:46:45 - 1.30
+++ upd.c 24 Oct 2021 11:43:59 -
@@ -163,11 +163,18 @@ upd_match(struct device *parent, void *m
/* need at least one sensor from root of tree */
uhidev_get_report_desc(uha->parent, , );
+
+ DPRINTF(("upd: nitems=%zu\n", nitems(upd_usage_roots)));
+ DPRINTF(("upd: nreports=%i\n", uha->nreports));
+
for (i = 0; i < nitems(upd_usage_roots); i++)
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
- uha->claimed[item.report_ID] = 1;
+ DPRINTF(("upd: item.report_ID=%i\n", item.report_ID));
+ if (item.report_ID < uha->nreports) {
+ uha->claimed[item.report_ID] = 1;
+ }
}
return (ret);
===
OpenBSD 7.0-stable (UPD.MP) #5: Sun Oct 24 13:44:04 CEST 2021
damien@gageac.petrolan:/usr/src/sys/arch/amd64/compile/UPD.MP
real mem = 6283997184 (5992MB)
avail mem = 6077521920 (5795MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xc8889000 (57 entries)
bios0: vendor LENOVO version "R19ET36W (1.20 )" date 07/12/2021
bios0: LENOVO 20U70003FR
acpi0 at bios0: ACPI 6.3
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SSDT SSDT IVRS SSDT MSDM BATB HPET APIC MCFG SBST WSMT
SSDT SSDT CRAT CDIT FPDT SSDT SSDT SSDT UEFI SSDT SSDT
acpi0: wakeup devices GPP0(S4) GPP3(S4) GPP4(S4) GPP5(S4) GP17(S4) XHC0(S3)
XHC1(S3) GP19(S4) LID_(S4) SLPB(S3)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 5 4500U with Radeon Graphics, 2370.89 MHz, 17-60-01
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,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line
8-way L2 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01
cpu1:
