NetBSD Problem Report #50810
From gson@gson.org Mon Feb 15 16:52:05 2016
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.NetBSD.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 0221B7ABE7
for <gnats-bugs@gnats.NetBSD.org>; Mon, 15 Feb 2016 16:52:04 +0000 (UTC)
Message-Id: <20160215165158.CE547744199@guava.gson.org>
Date: Mon, 15 Feb 2016 18:51:58 +0200 (EET)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@gnats.NetBSD.org
Subject: Kernel page fault trap in ugenclose()
X-Send-Pr-Version: 3.95
>Number: 50810
>Category: kern
>Synopsis: Kernel page fault trap in ugenclose()
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: skrll
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Feb 15 16:55:00 +0000 2016
>Closed-Date: Mon Mar 07 20:34:31 +0000 2016
>Last-Modified: Mon Mar 07 20:34:31 +0000 2016
>Originator: Andreas Gustafsson
>Release: NetBSD 6.1.5, 7.0, and -current
>Organization:
>Environment:
System: NetBSD guava.gson.org 6.1.5 NetBSD 6.1.5 (GENERIC) amd64
Architecture: x86_64
Machine: amd64
>Description:
When I try to scan a document using the "scanimage" program from
version 1.0.25 of the sane-backends package and a Fujitsu ScanSnap
S1500 scanner, my system crashes with a fatal page fault in
ugenclose().
This happens with 6.1.5, 7.0. and -current. The scanimage program
from earlier versions of sane-backends does not trigger the crash.
From the crash dump, I have determined that the crash happens when
ugenclose() dereferences sce->edesc, which is NULL.
The following patch prevents the crash, but it's probably not a
correct and complete fix since there are other places where sce->edesc
is checked for NULL only within "#ifdef DIAGNOSTIC", so presumably
that condition is not supposed to arise in the first place.
Also, it only prevents the crash, it does not make the scan succeed.
Index: ugen.c
===================================================================
RCS file: /bracket/repo/src/sys/dev/usb/ugen.c,v
retrieving revision 1.126
diff -u -r1.126 ugen.c
--- ugen.c 20 Sep 2014 08:45:23 -0000 1.126
+++ ugen.c 9 Feb 2016 19:56:22 -0000
@@ -544,6 +544,12 @@
usbd_close_pipe(sce->pipeh);
sce->pipeh = NULL;
+ if (sce->edesc == NULL) {
+ printf("ugenclose: endpt=%d dir=%d no edesc\n",
+ endpt, dir);
+ continue;
+ }
+
switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
case UE_INTERRUPT:
ndflush(&sce->q, sce->q.c_cc);
>How-To-Repeat:
If you don't happen to have a ScanSnap S1500, you can ask me to run
tests. The problem is 100% reproducible for me.
>Fix:
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: kern-bug-people->skrll
Responsible-Changed-By: skrll@NetBSD.org
Responsible-Changed-When: Mon, 15 Feb 2016 21:34:52 +0000
Responsible-Changed-Why:
take
From: "John D. Baker" <jdbaker@mylinuxisp.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 15 Feb 2016 16:14:29 -0600 (CST)
May be same as/related to:
kern/50597: using xsane causes my computer to crash
Do you have a crash dump/backtrace?
--
|/"\ John D. Baker, KN5UKS NetBSD Darwin/MacOS X
|\ / jdbaker[snail]mylinuxisp[flyspeck]com OpenBSD FreeBSD
| X No HTML/proprietary data in email. BSD just sits there and works!
|/ \ GPGkeyID: D703 4A7E 479F 63F8 D3F4 BD99 9572 8F23 E4AD 1645
From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 15 Feb 2016 21:42:35 +0000
Can you provide output from a UGEN_DEBUG kernel with ugendebug set to 10
Thanks,
Nick
From: Andreas Gustafsson <gson@gson.org>
To: "John D. Baker" <jdbaker@mylinuxisp.com>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Tue, 16 Feb 2016 10:53:06 +0200
John D. Baker wrote:
> Do you have a crash dump/backtrace?
A backtrace is now at:
http://www.gson.org/netbsd/bugs/50180/backtrace.jpg
--
Andreas Gustafsson, gson@gson.org
From: Andreas Gustafsson <gson@gson.org>
To: skrll@NetBSD.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Tue, 16 Feb 2016 11:26:55 +0200
Nick Hudson wrote:
> Can you provide output from a UGEN_DEBUG kernel with ugendebug set to 10
Included below at the end of the dmesg. This is with my patch, so the
output includes messages saying "no edesc" where the system would
otherwise have crashed.
--
Andreas Gustafsson, gson@gson.org
NetBSD 7.99.26 (GURU) #2: Tue Feb 16 11:05:36 EET 2016
gson@guido.araneus.fi:/ssd/current/amd64/obj/sys/arch/amd64/compile/GURU
total memory = 1977 MB
avail memory = 1895 MB
timecounter: Timecounters tick every 10.000 msec
Kernelized RAIDframe activated
timecounter: Timecounter "i8254" frequency 1193182 Hz quality 100
Hewlett-Packard HP Compaq dc7900 Small Form Factor ( )
mainbus0 (root)
ACPI: RSDP 0x00000000000E5810 000014 (v00 COMPAQ)
ACPI: RSDT 0x000000007B9C5840 000044 (v01 HPQOEM SLIC-BPC 20090305 00000000)
ACPI: FACP 0x000000007B9C58E8 000074 (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI BIOS Warning (bug): Optional FADT field Pm2ControlBlock has zero address or length: 0x0000000000000050/0x0 (20160108/tbfadt-681)
ACPI BIOS Warning (bug): Invalid length for FADT/Pm2ControlBlock: 0, using default 8 (20160108/tbfadt-733)
ACPI: DSDT 0x000000007B9C5E27 00A4B3 (v01 COMPAQ DSDT_PRJ 00000001 MSFT 0100000E)
ACPI: FACS 0x000000007B9C5800 000040
ACPI: APIC 0x000000007B9C595C 000084 (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: ASF! 0x000000007B9C59E0 000063 (v32 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: MCFG 0x000000007B9C5A43 00003C (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: TCPA 0x000000007B9C5A7F 000032 (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: SLIC 0x000000007B9C5AB1 000176 (v01 HPQOEM SLIC-BPC 00000001 00000000)
ACPI: HPET 0x000000007B9C5C27 000038 (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: DMAR 0x000000007B9C5C5F 0001C8 (v01 COMPAQ EAGLLAKE 00000001 00000000)
ACPI: 1 ACPI AML tables successfully acquired and loaded
ioapic0 at mainbus0 apid 1: pa 0xfec00000, version 0x20, 24 pins
cpu0 at mainbus0 apid 0
cpu0: Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz, id 0x1067a
cpu1 at mainbus0 apid 1
cpu1: Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz, id 0x1067a
acpi0 at mainbus0: Intel ACPICA 20160108
acpi0: X/RSDT: OemId <HPQOEM,SLIC-BPC,20090305>, AslId < ,00000000>
acpi0: MCFG: segment 0, bus 0-63, address 0x00000000f4000000
ACPI: Dynamic OEM Table Load:
ACPI: SSDT 0xFFFFFE8043831410 0003AC (v01 COMPAQ CPU_TM2 00000001 MSFT 0100000E)
ACPI: Dynamic OEM Table Load:
ACPI: SSDT 0xFFFFFE804382E990 0002B0 (v01 COMPAQ CST 00000001 MSFT 0100000E)
acpi0: SCI interrupting at int 9
timecounter: Timecounter "ACPI-Fast" frequency 3579545 Hz quality 1000
hpet0 at acpi0: high precision event timer (mem 0xfed00000-0xfed00400)
timecounter: Timecounter "hpet0" frequency 14318180 Hz quality 2000
attimer1 at acpi0 (TIME, PNP0100): io 0x40-0x43 irq 0
pcppi1 at acpi0 (SPKR, PNP0800): io 0x61
midi0 at pcppi1: PC speaker
sysbeep0 at pcppi1
pckbc1 at acpi0 (PS2M, PNP0F13) (aux port): irq 12
pckbc2 at acpi0 (KBD, PNP0303) (kbd port): io 0x60,0x64 irq 1
COM1 (PNP0501) at acpi0 not configured
FDC0 (PNP0700) at acpi0 not configured
APIC (PNP0003) at acpi0 not configured
TPM (IFX0102) at acpi0 not configured
SBD1 (PNP0C02) at acpi0 not configured
SBD3 (PNP0C02) at acpi0 not configured
SBD2 (PNP0C02) at acpi0 not configured
acpivga0 at acpi0 (GFX0): ACPI Display Adapter
MBRD (PNP0C01) at acpi0 not configured
acpibut0 at acpi0 (PBTN, PNP0C0C): ACPI Power Button
acpiwmi0 at acpi0 (WMID, PNP0C14-0): ACPI WMI Interface
wmihp0 at acpiwmi0: HP WMI mappings
ACPI: Enabled 1 GPEs in block 00 to 3F
attimer1: attached to pcppi1
pckbd0 at pckbc2 (kbd slot)
pckbc2: using irq 1 for kbd slot
wskbd0 at pckbd0: console keyboard
pms0 at pckbc2 (aux slot)
pckbc2: using irq 12 for aux slot
wsmouse0 at pms0 mux 0
pci0 at mainbus0 bus 0: configuration mode 1
pci0: i/o space, memory space enabled, rd/line, rd/mult, wr/inv ok
pchb0 at pci0 dev 0 function 0: vendor 8086 product 2e10 (rev. 0x03)
agp0 at pchb0: G4X-family chipset
agp0: detected 32252k stolen memory
agp0: aperture at 0xe0000000, size 0x10000000
i915drmkms0 at pci0 dev 2 function 0: vendor 8086 product 2e12 (rev. 0x03)
drm: Memory usable by graphics device = 512M
drm kern info: [drm] ACPI BIOS requests an excessive sleep of 3499208196 ms, using 1500 ms instead
drm kern info: [drm] ACPI BIOS requests an excessive sleep of 3499208196 ms, using 1500 ms instead
drm: Supports vblank timestamp caching Rev 2 (21.10.2013).
drm: Driver supports precise vblank timestamp query.
i915drmkms0: interrupting at ioapic0 pin 16 (i915)
intelfb0 at i915drmkms0
i915drmkms0: info: registered panic notifier
intelfb0: framebuffer at 0xffff800023d90000, size 1280x1024, depth 32, stride 5120
wsdisplay0 at intelfb0 kbdmux 1: console (default, vt100 emulation), using wskbd0
wsmux1: connecting to wsdisplay0
vendor 8086 product 2e13 (miscellaneous display, revision 0x03) at pci0 dev 2 function 1 not configured
vendor 8086 product 2e14 (miscellaneous communications, revision 0x03) at pci0 dev 3 function 0 not configured
pciide0 at pci0 dev 3 function 2: vendor 8086 product 2e16 (rev. 0x03)
pciide0: bus-master DMA support present, but unused (no driver support)
pciide0: primary channel wired to native-PCI mode
pciide0: using ioapic0 pin 18 for native-PCI interrupt
atabus0 at pciide0 channel 0
pciide0: secondary channel wired to native-PCI mode
atabus1 at pciide0 channel 1
puc0 at pci0 dev 3 function 3: Intel Q45 KT (com)
com2 at puc0 port 0 (16550-compatible): ioaddr 0x1268, interrupting at ioapic0 pin 17
com2: ns16550a, working fifo
wm0 at pci0 dev 25 function 0: 82567LM-3 LAN Controller (rev. 0x02)
wm0: interrupting at msi0 vec 0
wm0: PCI-Express bus
wm0: 2048 words FLASH
wm0: Ethernet address 00:24:81:99:99:1a
makphy0 at wm0 phy 2: Marvell 88E1149 Gigabit PHY, rev. 1
makphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto
uhci0 at pci0 dev 26 function 0: vendor 8086 product 3a67 (rev. 0x02)
uhci0: interrupting at ioapic0 pin 20
usb0 at uhci0: USB revision 1.0
uhci1 at pci0 dev 26 function 1: vendor 8086 product 3a68 (rev. 0x02)
uhci1: interrupting at ioapic0 pin 21
usb1 at uhci1: USB revision 1.0
uhci2 at pci0 dev 26 function 2: vendor 8086 product 3a69 (rev. 0x02)
uhci2: interrupting at ioapic0 pin 22
usb2 at uhci2: USB revision 1.0
ehci0 at pci0 dev 26 function 7: vendor 8086 product 3a6c (rev. 0x02)
ehci0: interrupting at ioapic0 pin 22
ehci0: EHCI version 1.0
ehci0: companion controllers, 2 ports each: uhci0 uhci1 uhci2
usb3 at ehci0: USB revision 2.0
hdaudio0 at pci0 dev 27 function 0: HD Audio Controller
hdaudio0: interrupting at ioapic0 pin 21
hdafg0 at hdaudio0: vendor 11d4 product 184a
hdafg0: DAC00 2ch: Speaker [Jack]
hdafg0: ADC01 2ch: Line In [Jack]
hdafg0: DAC02 2ch: HP Out [Jack]
hdafg0: ADC03 2ch: Mic In [Jack]
hdafg0: 2ch/2ch 8000Hz 11025Hz 16000Hz 22050Hz 32000Hz 44100Hz 48000Hz 88200Hz 96000Hz 192000Hz PCM16 PCM20 PCM24 AC3
audio0 at hdafg0: full duplex, playback, capture, mmap, independent
ppb0 at pci0 dev 28 function 0: vendor 8086 product 3a70 (rev. 0x02)
ppb0: PCI Express capability version 2 <Root Port of PCI-E Root Complex> x4 @ 2.5GT/s
pci1 at ppb0 bus 32
pci1: i/o space, memory space enabled, rd/line, wr/inv ok
ppb1 at pci0 dev 28 function 4: vendor 8086 product 3a78 (rev. 0x02)
ppb1: PCI Express capability version 2 <Root Port of PCI-E Root Complex> x1 @ 2.5GT/s
pci2 at ppb1 bus 48
pci2: i/o space, memory space enabled, rd/line, wr/inv ok
uhci3 at pci0 dev 29 function 0: vendor 8086 product 3a64 (rev. 0x02)
uhci3: interrupting at ioapic0 pin 20
usb4 at uhci3: USB revision 1.0
uhci4 at pci0 dev 29 function 1: vendor 8086 product 3a65 (rev. 0x02)
uhci4: interrupting at ioapic0 pin 21
usb5 at uhci4: USB revision 1.0
uhci5 at pci0 dev 29 function 2: vendor 8086 product 3a66 (rev. 0x02)
uhci5: interrupting at ioapic0 pin 22
usb6 at uhci5: USB revision 1.0
ehci1 at pci0 dev 29 function 7: vendor 8086 product 3a6a (rev. 0x02)
ehci1: interrupting at ioapic0 pin 20
ehci1: EHCI version 1.0
ehci1: companion controllers, 2 ports each: uhci3 uhci4 uhci5
usb7 at ehci1: USB revision 2.0
ppb2 at pci0 dev 30 function 0: vendor 8086 product 244e (rev. 0xa2)
pci3 at ppb2 bus 7
pci3: i/o space, memory space enabled
ichlpcib0 at pci0 dev 31 function 0: vendor 8086 product 3a14 (rev. 0x02)
timecounter: Timecounter "ichlpcib0" frequency 3579545 Hz quality 1000
ichlpcib0: 24-bit timer
tco0 at ichlpcib0: TCO (watchdog) timer configured.
tco0: Min/Max interval 1/367 seconds
gpio0 at ichlpcib0: 64 pins
piixide0 at pci0 dev 31 function 2: Intel 82801JD Serial ATA Controller (ICH10) (rev. 0x02)
piixide0: bus-master DMA support present
piixide0: primary channel configured to compatibility mode
piixide0: primary channel interrupting at ioapic0 pin 14
atabus2 at piixide0 channel 0
piixide0: secondary channel configured to compatibility mode
piixide0: secondary channel interrupting at ioapic0 pin 15
atabus3 at piixide0 channel 1
piixide1 at pci0 dev 31 function 5: Intel 82801JD Serial ATA Controller (ICH10) (rev. 0x02)
piixide1: bus-master DMA support present
piixide1: primary channel wired to native-PCI mode
piixide1: using ioapic0 pin 18 for native-PCI interrupt
atabus4 at piixide1 channel 0
piixide1: secondary channel wired to native-PCI mode
atabus5 at piixide1 channel 1
isa0 at ichlpcib0
tpm0 at isa0 iomem 0xfed40000-0xfed44fff irq 7: IFX SLB 9635 TT 1.2 rev 0x10
com0 at isa0 port 0x3f8-0x3ff irq 4: ns16550a, working fifo
fdc0 at isa0 port 0x3f0-0x3f7 irq 6 drq 2
acpicpu0 at cpu0: ACPI CPU
acpicpu0: C1: FFH, lat 1 us, pow 1000 mW
acpicpu0: C2: FFH, lat 17 us, pow 500 mW
acpicpu0: P0: FFH, lat 10 us, pow 135000 mW, 3000 MHz
acpicpu0: P1: FFH, lat 10 us, pow 68432 mW, 1998 MHz
acpicpu0: T0: I/O, lat 1 us, pow 0 mW, 100 %
acpicpu0: T1: I/O, lat 1 us, pow 0 mW, 88 %
acpicpu0: T2: I/O, lat 1 us, pow 0 mW, 76 %
acpicpu0: T3: I/O, lat 1 us, pow 0 mW, 64 %
acpicpu0: T4: I/O, lat 1 us, pow 0 mW, 52 %
acpicpu0: T5: I/O, lat 1 us, pow 0 mW, 40 %
acpicpu0: T6: I/O, lat 1 us, pow 0 mW, 28 %
acpicpu0: T7: I/O, lat 1 us, pow 0 mW, 16 %
coretemp0 at cpu0: thermal sensor, 1 C resolution, Tjmax=100
acpicpu1 at cpu1: ACPI CPU
coretemp1 at cpu1: thermal sensor, 1 C resolution, Tjmax=100
timecounter: Timecounter "clockinterrupt" frequency 100 Hz quality 0
IPsec: Initialized Security Association Processing.
uhub0 at usb0: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub0: 2 ports with 2 removable, self powered
uhub1 at usb2: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub1: 2 ports with 2 removable, self powered
uhub2 at usb4: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub2: 2 ports with 2 removable, self powered
uhub3 at usb6: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub3: 2 ports with 2 removable, self powered
uhub4 at usb1: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub4: 2 ports with 2 removable, self powered
uhub5 at usb3: vendor 8086 EHCI root hub, class 9/0, rev 2.00/1.00, addr 1
uhub5: 6 ports with 6 removable, self powered
uhub6 at usb5: vendor 8086 UHCI root hub, class 9/0, rev 1.00/1.00, addr 1
uhub6: 2 ports with 2 removable, self powered
uhub7 at usb7: vendor 8086 EHCI root hub, class 9/0, rev 2.00/1.00, addr 1
uhub7: 6 ports with 6 removable, self powered
wd0 at atabus2 drive 0
wd0: <ST3500630AS>
wd0: drive supports 16-sector PIO transfers, LBA48 addressing
wd0: 465 GB, 969021 cyl, 16 head, 63 sec, 512 bytes/sect x 976773168 sectors
wd0: 32-bit data port
wd0: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133)
wd0(piixide0:0:0): using PIO mode 4, Ultra-DMA mode 6 (Ultra/133) (using DMA)
atapibus0 at atabus3: 2 targets
cd0 at atapibus0 drive 0: <ATAPI DVD A DH16A6L, 359919403324, ZH39> cdrom removable
cd0: 32-bit data port
cd0: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 5 (Ultra/100)
cd0(piixide0:1:0): using PIO mode 4, Ultra-DMA mode 5 (Ultra/100) (using DMA)
pad0: outputs: 44100Hz, 16-bit, stereo
audio1 at pad0: half duplex, playback, capture
boot device: wd0
root on wd0a dumps on wd0b
root file system type: ffs
kern.module.path=/stand/amd64/7.99.26/modules
wsdisplay0: screen 1 added (default, vt100 emulation)
wsdisplay0: screen 2 added (default, vt100 emulation)
wsdisplay0: screen 3 added (default, vt100 emulation)
wsdisplay0: screen 4 added (default, vt100 emulation)
ugen0 at uhub5 port 5
ugen0: Fujitsu ScanSnap S1500, rev 2.00/1.00, addr 2
ugen_set_config: ugen0 to configno 1, sc=0xffff8000037e3000
ugen_set_config: ifaceno 0
ugen_set_config: endptno 0, endpt=0x81(1,128), sce=0xffff8000037e34d8
ugen_set_config: endptno 1, endpt=0x02(2,0), sce=0xffff8000037e3660
ugenopen: flag=3, mode=8192, unit=0 endpt=0
ugenclose: flag=3, mode=8192, unit=0, endpt=0
ugenclose: close control
ugenopen: flag=3, mode=8192, unit=0 endpt=0
ugenopen: flag=3, mode=8192, unit=0 endpt=2
ugenopen: flag=2, mode=8192, unit=0 endpt=2
ugenopen: sc=0xffff8000037e3000, endpt=2, dir=0, sce=0xffff8000037e3660
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenopen: flag=3, mode=8192, unit=0 endpt=1
ugenopen: flag=1, mode=8192, unit=0 endpt=1
ugenopen: sc=0xffff8000037e3000, endpt=1, dir=1, sce=0xffff8000037e34d8
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 96 bytes
ugenread: got 96 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 204 bytes
ugenread: got 204 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 72 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 16 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=80045572
ugen0: ugenwrite: 2
ugenwrite: transfer 31 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 10 bytes
ugenread: got 10 bytes
ugenioctl: cmd=80045572
ugenioctl: cmd=80045571
ugen0: ugenread: 1
ugenread: start transfer 13 bytes
ugenread: got 13 bytes
ugenioctl: cmd=c00c5567
ugenclose: flag=1, mode=8192, unit=0, endpt=1
ugenclose: endpt=1 dir=1 sce=0xffff8000037e34d8
ugenclose: endpt=1 dir=1 no edesc
ugenclose: flag=2, mode=8192, unit=0, endpt=2
ugenclose: endpt=2 dir=0 sce=0xffff8000037e3660
ugenclose: endpt=2 dir=0 no edesc
ugenclose: flag=3, mode=8192, unit=0, endpt=0
ugenclose: close control
ugenopen: flag=3, mode=8192, unit=0 endpt=0
ugenopen: flag=3, mode=8192, unit=0 endpt=2
ugenopen: flag=2, mode=8192, unit=0 endpt=2
ugenopen: flag=3, mode=8192, unit=0 endpt=2
ugenopen: flag=2, mode=8192, unit=0 endpt=2
ugenopen: flag=3, mode=8192, unit=0 endpt=2
ugenopen: flag=2, mode=8192, unit=0 endpt=2
ugenioctl: cmd=c00c5567
ugenclose: flag=3, mode=8192, unit=0, endpt=0
ugenclose: close control
From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
Andreas Gustafsson <gson@gson.org>
Cc:
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sat, 20 Feb 2016 09:56:39 +0000
On 02/16/16 09:30, Andreas Gustafsson wrote:
> ugenioctl: cmd=c00c5567
> ugenclose: flag=1, mode=8192, unit=0, endpt=1
> ugenclose: endpt=1 dir=1 sce=0xffff8000037e34d8
> ugenclose: endpt=1 dir=1 no edesc
> ugenclose: flag=2, mode=8192, unit=0, endpt=2
> ugenclose: endpt=2 dir=0 sce=0xffff8000037e3660
> ugenclose: endpt=2 dir=0 no edesc
> ugenclose: flag=3, mode=8192, unit=0, endpt=0
> ugenclose: close control
The ioctl (USB_SET_ALTINTERFACE) is blatting edesc and making it fail
Not sure why it's doing this before closing the endpoints.
Maybe the ioctl should fail here?
Nick
From: Andreas Gustafsson <gson@gson.org>
To: skrll@NetBSD.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sat, 20 Feb 2016 13:11:39 +0200
Nick Hudson wrote:
> The ioctl (USB_SET_ALTINTERFACE) is blatting edesc and making it fail
>
> Not sure why it's doing this before closing the endpoints.
>
> Maybe the ioctl should fail here?
I'm having some trouble following your comment.
- What do you mean by "blatting"? The dictionary
I consulted was not helpful.
- In "why it's doing this", what does "it" refer to?
- At what point should the ioctl fail, and why would it make
a difference?
I see there is a comment in ugen_set_interface() saying /* XXX should
only do this after setting new altno has succeeded */, which seems to
point at a possible cause of the bug: If setting the new altno fails,
sce->edesc will be NULL, but sce->pipeh has not been cleared, so if it
was not NULL to begin, with we have the exact combination of
conditions (sce->pipeh != NULL && sce->edesc == NULL) that triggers
the crash in ugenclose().
Also, there are several places checking for sce == NULL, for example:
sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
if (sce == NULL)
None of these make any sense - sce is a pointer into the middle of a
the ugen_softc struct, so it can *never* be NULL by definition.
Should they all say "sce->sc == NULL"? What is the canonical way
of distinguishing a valid endpoint from an invalid one?
--
Andreas Gustafsson, gson@gson.org
From: Nick Hudson <skrll@netbsd.org>
To: Andreas Gustafsson <gson@gson.org>, skrll@NetBSD.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sat, 20 Feb 2016 17:12:24 +0000
On 02/20/16 11:11, Andreas Gustafsson wrote:
> Nick Hudson wrote:
>> The ioctl (USB_SET_ALTINTERFACE) is blatting edesc and making it fail
>>
>> Not sure why it's doing this before closing the endpoints.
>>
>> Maybe the ioctl should fail here?
> I'm having some trouble following your comment.
Sorry for being unclear....
> - What do you mean by "blatting"? The dictionary
> I consulted was not helpful.
overwriting / erasing
>
> - In "why it's doing this", what does "it" refer to?
sane-backends
>
> - At what point should the ioctl fail, and why would it make
> a difference?
>
> I see there is a comment in ugen_set_interface() saying /* XXX should
> only do this after setting new altno has succeeded */, which seems to
> point at a possible cause of the bug: If setting the new altno fails,
> sce->edesc will be NULL, but sce->pipeh has not been cleared, so if it
> was not NULL to begin, with we have the exact combination of
> conditions (sce->pipeh != NULL && sce->edesc == NULL) that triggers
> the crash in ugenclose().
You got what I meant here.
>
> Also, there are several places checking for sce == NULL, for example:
>
> sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
> if (sce == NULL)
>
> None of these make any sense - sce is a pointer into the middle of a
> the ugen_softc struct, so it can *never* be NULL by definition.
> Should they all say "sce->sc == NULL"? What is the canonical way
> of distinguishing a valid endpoint from an invalid one?
I guess edesc and/or iface get populated with something after
ugen_set_interface/ugen_set_config
Nick
From: Andreas Gustafsson <gson@gson.org>
To: skrll@NetBSD.org
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sat, 20 Feb 2016 22:53:52 +0200
Nick,
You wrote:
> > Also, there are several places checking for sce == NULL, for example:
> >
> > sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
> > if (sce == NULL)
> >
> > None of these make any sense - sce is a pointer into the middle of a
> > the ugen_softc struct, so it can *never* be NULL by definition.
> > Should they all say "sce->sc == NULL"? What is the canonical way
> > of distinguishing a valid endpoint from an invalid one?
>
> I guess edesc and/or iface get populated with something after
> ugen_set_interface/ugen_set_config
I'm sorry, but I fail to see the connection between my question and
your answer.
--
Andreas Gustafsson, gson@gson.org
From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
Andreas Gustafsson <gson@gson.org>
Cc:
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sun, 21 Feb 2016 10:05:36 +0000
This is a multi-part message in MIME format.
--------------050805080502070203020306
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
On 02/20/16 20:55, Andreas Gustafsson wrote:
> The following reply was made to PR kern/50810; it has been noted by GNATS.
>
> From: Andreas Gustafsson<gson@gson.org>
> To:skrll@NetBSD.org
> Cc:gnats-bugs@netbsd.org
> Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
> Date: Sat, 20 Feb 2016 22:53:52 +0200
>
> Nick,
>
> You wrote:
> > > Also, there are several places checking for sce == NULL, for example:
> > >
> > > sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
> > > if (sce == NULL)
> > >
> > > None of these make any sense - sce is a pointer into the middle of a
> > > the ugen_softc struct, so it can *never* be NULL by definition.
> > > Should they all say "sce->sc == NULL"?
I got rid of these pointless conditionals
> > > What is the canonical way
> > > of distinguishing a valid endpoint from an invalid one?
> >
> > I guess edesc and/or iface get populated with something after
> > ugen_set_interface/ugen_set_config
>
> I'm sorry, but I fail to see the connection between my question and
> your answer.
I was really answering your second question and not the first. Both
edesc and iface will
be non-null if the endpoint is valid.
> --
> Andreas Gustafsson,gson@gson.org
>
>
Does this diff help? I'm not sure of retaining selinfo and the cv is
correct for ugen_set_interface.
Nick
--------------050805080502070203020306
Content-Type: text/x-patch;
name="ugen.c.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="ugen.c.diff"
Index: sys/dev/usb/ugen.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ugen.c,v
retrieving revision 1.129
diff -u -p -r1.129 ugen.c
--- sys/dev/usb/ugen.c 21 Feb 2016 09:50:10 -0000 1.129
+++ sys/dev/usb/ugen.c 21 Feb 2016 10:04:19 -0000
@@ -270,6 +270,19 @@ ugen_attach(device_t parent, device_t se
return;
}
+Static void
+ugen_clear_endpoints(struct ugen_softc *sc)
+{
+
+ /* Clear out the old info, but leave the selinfo and cv initialised. */
+ for (int i = 0; i < USB_MAX_ENDPOINTS; i++) {
+ for (int dir = OUT; dir <= IN; dir++) {
+ struct ugen_endpoint *sce = &sc->sc_endpoints[i][dir];
+ memset(sce, 0, UGEN_ENDPOINT_NONZERO_CRUFT);
+ }
+ }
+}
+
Static int
ugen_set_config(struct ugen_softc *sc, int configno)
{
@@ -281,7 +294,7 @@ ugen_set_config(struct ugen_softc *sc, i
u_int8_t niface, nendpt;
int ifaceno, endptno, endpt;
usbd_status err;
- int dir, i;
+ int dir;
DPRINTFN(1,("ugen_set_config: %s to configno %d, sc=%p\n",
device_xname(sc->sc_dev), configno, sc));
@@ -310,13 +323,7 @@ ugen_set_config(struct ugen_softc *sc, i
if (err)
return (err);
- /* Clear out the old info, but leave the selinfo and cv initialised. */
- for (i = 0; i < USB_MAX_ENDPOINTS; i++) {
- for (dir = OUT; dir <= IN; dir++) {
- sce = &sc->sc_endpoints[i][dir];
- memset(sce, 0, UGEN_ENDPOINT_NONZERO_CRUFT);
- }
- }
+ ugen_clear_endpoints(sc);
for (ifaceno = 0; ifaceno < niface; ifaceno++) {
DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
@@ -1336,16 +1343,6 @@ ugen_set_interface(struct ugen_softc *sc
err = usbd_endpoint_count(iface, &nendpt);
if (err)
return (err);
- /* XXX should only do this after setting new altno has succeeded */
- for (endptno = 0; endptno < nendpt; endptno++) {
- ed = usbd_interface2endpoint_descriptor(iface,endptno);
- endpt = ed->bEndpointAddress;
- dir = UE_GET_DIR(endpt) == UE_DIR_IN ? IN : OUT;
- sce = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
- sce->sc = NULL;
- sce->edesc = NULL;
- sce->iface = NULL;
- }
/* change setting */
err = usbd_set_interface(iface, altno);
@@ -1355,6 +1352,9 @@ ugen_set_interface(struct ugen_softc *sc
err = usbd_endpoint_count(iface, &nendpt);
if (err)
return (err);
+
+ ugen_clear_endpoints(sc);
+
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
KASSERT(ed != NULL);
--------------050805080502070203020306--
From: Ryo ONODERA <ryo_on@yk.rim.or.jp>
To: skrll@netbsd.org, gnats-bugs@NetBSD.org, gson@gson.org
Cc:
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sun, 21 Feb 2016 19:32:37 +0900 (JST)
Hi,
From: Nick Hudson <skrll@netbsd.org>, Date: Sun, 21 Feb 2016 10:05:36 +0000
> Does this diff help? I'm not sure of retaining selinfo and the cv is
> correct for ugen_set_interface.
I have same problem with Canon CanoScan LiDE40 scanner and sane scanimage.
With your patch, my problem is resolved.
Thank you.
--
Ryo ONODERA // ryo_on@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50810 CVS commit: src/sys/dev/usb
Date: Mon, 22 Feb 2016 07:46:00 +0000
Module Name: src
Committed By: skrll
Date: Mon Feb 22 07:46:00 UTC 2016
Modified Files:
src/sys/dev/usb: ugen.c
Log Message:
Only clear the endpoint information in ugen_set_interface only if setting
the new altno suceeds.
Avoids the null de-ref in PR/50597 and PR/50810
To generate a diff of this commit:
cvs rdiff -u -r1.129 -r1.130 src/sys/dev/usb/ugen.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->pending-pullups
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Mon, 22 Feb 2016 07:51:38 +0000
State-Changed-Why:
[pullup-7 #1124] ugen(4) fix for PR/50597 and PR/50810
From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <skrll@netbsd.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 22 Feb 2016 09:55:05 +0200
Nick Hudson wrote:
> I got rid of these pointless conditionals
I'm just wondering if whoever put them in in the first place intended
to check for a valid endpoint, in which case some of them might need to
be replaced by working checks for a valid endpoint rather than just
removed.
> > > > What is the canonical way
> > > > of distinguishing a valid endpoint from an invalid one?
> > >
> > > I guess edesc and/or iface get populated with something after
> > > ugen_set_interface/ugen_set_config
> >
> > I'm sorry, but I fail to see the connection between my question and
> > your answer.
>
> I was really answering your second question and not the first. Both
> edesc and iface will
> be non-null if the endpoint is valid.
That still doesn't tell me which one the code should be checking.
I guess the answer is that there is no canonical way, and that
each function is just checking whatever field it is going to
dereference (if it is checking at all).
> Does this diff help?
Building with UGENDEBUG enabled, I got:
/ssd/current/src/sys/dev/usb/ugen.c:274:1: error: no previous prototype for 'ugen_clear_endpoints' [-Werror=missing-prototypes]
ugen_clear_endpoints(struct ugen_softc *sc)
^
cc1: all warnings being treated as errors
*** [ugen.o] Error code 1
After I worked around that, the patch did prevent the crash, and not
only that, the scan even succeeded, which it didn't with my own
initial patch. Thank you!
> I'm not sure of retaining selinfo and the cv is correct for
> ugen_set_interface.
I'm not sure, either.
--
Andreas Gustafsson, gson@gson.org
From: Nick Hudson <skrll@netbsd.org>
To: Andreas Gustafsson <gson@gson.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 22 Feb 2016 08:11:58 +0000
On 02/22/16 07:55, Andreas Gustafsson wrote:
> Nick Hudson wrote:
>> I got rid of these pointless conditionals
> I'm just wondering if whoever put them in in the first place intended
> to check for a valid endpoint, in which case some of them might need to
> be replaced by working checks for a valid endpoint rather than just
> removed.
>
>>> > > What is the canonical way
>>> > > of distinguishing a valid endpoint from an invalid one?
>>> >
>>> > I guess edesc and/or iface get populated with something after
>>> > ugen_set_interface/ugen_set_config
>>>
>>> I'm sorry, but I fail to see the connection between my question and
>>> your answer.
>> I was really answering your second question and not the first. Both
>> edesc and iface will
>> be non-null if the endpoint is valid.
> That still doesn't tell me which one the code should be checking.
> I guess the answer is that there is no canonical way, and that
> each function is just checking whatever field it is going to
> dereference (if it is checking at all).
Huh? check either or both...
>
>> Does this diff help?
> Building with UGENDEBUG enabled, I got:
>
> /ssd/current/src/sys/dev/usb/ugen.c:274:1: error: no previous prototype for 'ugen_clear_endpoints' [-Werror=missing-prototypes]
> ugen_clear_endpoints(struct ugen_softc *sc)
> ^
> cc1: all warnings being treated as errors
> *** [ugen.o] Error code 1
>
> After I worked around that, the patch did prevent the crash, and not
> only that, the scan even succeeded, which it didn't with my own
> initial patch. Thank you!
>
>> I'm not sure of retaining selinfo and the cv is correct for
>> ugen_set_interface.
> I'm not sure, either.
Can you work out where in sane it's doing the
ioctl(USB_SET_ALTINTERFACE) and
why it's choosing to do it before closing the endpoints of the existing
interface? #
I'm curious what behaviour it's expecting.
Thanks,
Nick
From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <skrll@netbsd.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 22 Feb 2016 11:01:56 +0200
Nick Hudson wrote:
> > I guess the answer is that there is no canonical way, and that
> > each function is just checking whatever field it is going to
> > dereference (if it is checking at all).
>
> Huh? check either or both...
That's what I'm saying - if all choices are equally correct, then by
definition, none of them is canonical.
> Can you work out where in sane it's doing the
> ioctl(USB_SET_ALTINTERFACE) and
> why it's choosing to do it before closing the endpoints of the existing
> interface? #
> I'm curious what behaviour it's expecting.
From sane-backends-1.0.25/sanei/sanei_usb.c, around line 2154:
/* This call seems to be required by Linux xhci driver
* even though it should be a no-op. Without it, the
* host or driver does not reset it's data toggle bit.
* We intentionally ignore the return val */
=> sanei_usb_set_altinterface (dn, devices[dn].alt_setting);
libusb_release_interface (devices[dn].lu_handle,
devices[dn].interface_nr);
libusb_close (devices[dn].lu_handle);
--
Andreas Gustafsson, gson@gson.org
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50810 CVS commit: [netbsd-7] src/sys/dev/usb
Date: Sun, 6 Mar 2016 18:08:04 +0000
Module Name: src
Committed By: martin
Date: Sun Mar 6 18:08:04 UTC 2016
Modified Files:
src/sys/dev/usb [netbsd-7]: ugen.c
Log Message:
Pull up following revision(s) (requested by skrll in ticket #1124):
sys/dev/usb/ugen.c: revision 1.127
sys/dev/usb/ugen.c: revision 1.128
sys/dev/usb/ugen.c: revision 1.129
sys/dev/usb/ugen.c: revision 1.130
s/0/NULL/
One more s/0/NULL/
Remove always true conditional
Only clear the endpoint information in ugen_set_interface only if setting
the new altno suceeds.
Avoids the null de-ref in PR/50597 and PR/50810
To generate a diff of this commit:
cvs rdiff -u -r1.124 -r1.124.2.1 src/sys/dev/usb/ugen.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50810 CVS commit: [netbsd-7-0] src/sys/dev/usb
Date: Sun, 6 Mar 2016 18:10:20 +0000
Module Name: src
Committed By: martin
Date: Sun Mar 6 18:10:20 UTC 2016
Modified Files:
src/sys/dev/usb [netbsd-7-0]: ugen.c
Log Message:
Pull up following revision(s) (requested by skrll in ticket #1124):
sys/dev/usb/ugen.c: revision 1.127
sys/dev/usb/ugen.c: revision 1.128
sys/dev/usb/ugen.c: revision 1.129
sys/dev/usb/ugen.c: revision 1.130
s/0/NULL/
One more s/0/NULL/
Remove always true conditional
Only clear the endpoint information in ugen_set_interface only if setting
the new altno suceeds.
Avoids the null de-ref in PR/50597 and PR/50810
To generate a diff of this commit:
cvs rdiff -u -r1.124 -r1.124.4.1 src/sys/dev/usb/ugen.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: gson@NetBSD.org
State-Changed-When: Mon, 07 Mar 2016 20:34:31 +0000
State-Changed-Why:
Pullups done
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home