kernel crash when connecting MP3-Player (Sansa Clip Zip)
Hi bugs@, whenever i connect my turned-off Sansa Clip Zip to my openbsd desktop machine, the kernel hangs and throws me into dbb. It's 100% reproducible. The output was generated from the latest amd64-current-snapshot (23.02., dmesg below), connecting the player immediately after boot. I replaced the firmware on the player with rockbox 3.13, which crashes from time to time. On debian wheezy i had similar problems (i.e. the *player* crashed and was not detected when i connected it), but debian never crashed. Something else i noticed while looking at the ps output: there is a 'usbtask' and a 'usbatsk' command running, is that correct or a typo somewhere? Best, N. kernel: protection fault trap, code=0 Stopped at usb_allocmem+0x166: cmpq%rbx,0(%rax) ddb{0}> trace usb_allocmem() at usb_allocmem+0x166 usbd_transfer() at usbd_transfer+0xe6 usbd_do_request_flags() at usbd_do_request_flags+0xcc usbd_get_desc() at usbd_get_desc+0x2d usbd_new_device() at usbd_new_device+0x1c7 uhub_explore() at uhub_explore+0x1c9 usb_explore() at usb_explore+0xcf usb_task_thread() at usb_task_thread+0xb2 end trace frame: 0x0, count: -8 ddb{0}> ps PID PPID PGRPUID S FLAGS WAIT COMMAND 2487 1 2487 0 30x83 ttyingetty 74 1 74 0 30x83 ttyingetty 11485 1 11485 0 30x83 ttyingetty 17765 1 17765 0 30x83 ttyingetty 8940 1 8940 0 30x83 ttyingetty 12835 1 12835 0 30x80 select cron 15782 1 15782 0 30x80 kqread apmd 5226 1 5226 99 30x90 poll sndiod 10951 1 10951 0 30xb0 select sendmail 32479 1 32479 0 30x80 select sshd 1841 4987118 83 30x90 poll ntpd 4987118118 83 30x90 poll ntpd 118 1118 0 30x80 poll ntpd 11290 6871 6871 74 30x90 bpf pflogd 6871 1 6871 0 30x80 netiopflogd 4023 18959 18959 73 30x90 poll syslogd 18959 1 18959 0 30x80 netiosyslogd 25941 1 25941 77 30x90 poll dhclient 24059 1 24059 0 30x80 poll dhclient 22870 0 0 0 3 0x4200 boredttm_swap 30340 0 0 0 3 0x4200 aiodoned aiodoned 8303 0 0 0 3 0x4200 syncer update 32489 0 0 0 3 0x4200 cleaner cleaner 3256 0 0 0 3 0x4200 reaper reaper 27232 0 0 0 3 0x4200 pgdaemon pagedaemon 15293 0 0 0 3 0x4200 boredcrypto 3835 0 0 0 3 0x4200 pftm pfpurge 13053 0 0 0 3 0x4200 boredsensors * 5362 0 0 0 7 0x4200 ; usbtask 18010 0 0 0 3 0x4200 usbatsk usbatsk 30029 0 0 0 3 0x40004200 acpi0apci0 17126 0 0 0 7 0x40004200 & nbsp; idle1 16167 0 0 0 3 0x4200 boredsystq 15255 0 0 0 3 0x4200 boredsyswq 25236 0 0 0 3 0x40004200 & nbsp; idle0 1 0 1 0 30x82 wait init 0 -1 0 0 3 0x200 schedulerswapper ddb{0}> *dmesg OpenBSD 5.5 (GENERIC.MP) #296: Sun Feb 23 11:54:46 MST 2014 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 3738107904 (3564MB) avail mem = 3629985792 (3461MB) mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf0100 (61 entries) bios0: vendor Award Software International, Inc. version "F4" date 07/28/2010 bios0: Gigabyte Technology Co., Ltd. GA-880GMA-UD2H acpi0 at bios0: rev 0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SSDT HPET MCFG MATS APIC acpi0: wakeup devices USB0(S3) USB1(S3) USB2(S3) USB3(S3) USB4(S3) USB5(S3) USB6(S3) SBAZ(S4) PEX0(S5) PEX1(S5) PEX2(S5) PEX3(S5) P2P_(S5) PCE2(S4) PCE3(S4) PCE4(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 32 bits acpihpet0 at acpi0: 14318180 Hz acpimcfg0 at acpi0 addr 0xe000, bus 0-255 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD Athlon(tm) II X2 240e Processor, 11252.14 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUS H,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,CX16,POPCNT,NXE,MMXX,FFXSR,LONG,3DNOW2,3DN OW,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,ITSC< br> cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 1MB 64b/line 16-way L2 cache cpu0: ITLB 32 4KB entries fully associative, 16 4MB entries fully associative cpu0: DTLB 48 4KB entries fully associative, 48 4MB entries fully a
Re: kernel crash when connecting MP3-Player (Sansa Clip Zip)
Hello, On 24/02/14(Mon) 13:01, n.reu...@hxgn.net wrote: > Hi bugs@, > > whenever i connect my turned-off Sansa Clip Zip to my openbsd desktop > machine, > the kernel hangs and throws me into dbb. It's 100% reproducible. The > output > was generated from the latest amd64-current-snapshot (23.02., dmesg > below), > connecting the player immediately after boot. This looks like one of the DMA segment has been written past its size or after being freed. I see that you have various USB devices on your system, does it happen if you only have your music player connected? I assume it attaches at ehci, could you confirm that? If that's the case can you try to disable ehci at boot (by doing "boot -c" then "disable ehci" and "quit") and see if the problem is still there? Finally could you try to following diff and send me its output? > > Something else i noticed while looking at the ps output: there is a > 'usbtask' > and a 'usbatsk' command running, is that correct or a typo somewhere? It's correct, the first one is the kernel thread executing most of the USB operations (USB task) and the second one only do aborts (USB abort task). Thanks for you report, M. Index: usb_mem.c === RCS file: /home/ncvs/src/sys/dev/usb/usb_mem.c,v retrieving revision 1.25 diff -u -p -r1.25 usb_mem.c --- usb_mem.c 15 Apr 2013 09:23:02 - 1.25 +++ usb_mem.c 24 Feb 2014 15:06:12 - @@ -52,10 +52,13 @@ #include /* just for struct usb_dma */ #include +#undef USB_DEBUG +#define USB_DEBUG + #ifdef USB_DEBUG #define DPRINTF(x) do { if (usbdebug) printf x; } while (0) #define DPRINTFN(n,x) do { if (usbdebug>(n)) printf x; } while (0) -extern int usbdebug; +int usbdebug = 7; #else #define DPRINTF(x) #define DPRINTFN(n,x)
Re: Bad return value for getpwnam_r et al
Hi Stuart, Stuart Henderson wrote on Sun, Feb 23, 2014 at 02:22:47PM +: > btw, here are results from a search for getpw(nam|uid)_r in unpacked > ports source, after removing a bunch of autoconf checks etc. Wow, these are nearly a hundred ports. It is not feasible to audit all that. Besides, my patches also imply slight changes to the behaviour of getgr{nam,gid}_r() (when called with errno already set) and to getpw{nam,uid}() (which may now set errno even when finding a match and will no longer set errno with the third patch in that case). So your list is too long for a complete audit even though it is incomplete. Anyway, i looked at seven examples from your list, choosing ports from different categories. The results are, if i read the code correctly: - I found no cases where my patches might break something. - One of the seven ports would be significantly improved (postfix). - Three ports get minor improvements (gdm, glib2, ruby). - For three ports, it makes no difference (gmake, postgres, gnutls). - Nearly all changes in behaviour are due to the first patch - change of the return values - and among these, nearly all are due to changing the return value from 1 to 0 when not finding a match, but when there is no error. - The errno variable and the exact return value (as opposed to merely whether it's 0 or non-0) are rarely looked at. - For detailed results, look at the very end of this mail. Even after unlock, i don't think it would be reasonable to try to evaluate all the consequences of the changes. After unlock, we should just commit all the changes such that libc behaves reasonably, then watch out for any fallout. What do we commit now? After this review, i'd say: * PLEASE somebody give me an OK just for the first patch. It actually improves various ports. It doesn't look like it's going to break stuff. (Theo said he is not opposed to fixing part of this, but cannot look at the details right now.) * Let's not commit the second patch (avoid errno clobbering as suggested by guenther@). It doesn't seem to matter much, so let's avoid any risk. * Let's not commit the third patch (let getpw*_r() not touch errno as suggested by kettenis@). Even though i didn't find any port where this might matter, i still consider it more risky than the first two patches. Here is the first patch, again. Yours, Ingo Index: getpwent.c === RCS file: /cvs/src/lib/libc/gen/getpwent.c,v retrieving revision 1.48 diff -u -p -r1.48 getpwent.c --- getpwent.c 15 Nov 2013 22:32:55 - 1.48 +++ getpwent.c 19 Feb 2014 16:36:48 - @@ -708,8 +708,12 @@ getpwnam_r(const char *name, struct pass { struct passwd *pwret = NULL; int flags = 0, *flagsp; + int my_errno = 0; + int saved_errno; _THREAD_PRIVATE_MUTEX_LOCK(pw); + saved_errno = errno; + errno = 0; if (!_pw_db && !__initdb()) goto fail; @@ -733,8 +737,12 @@ getpwnam_r(const char *name, struct pass fail: if (pwretp) *pwretp = pwret; + if (pwret == NULL) + my_errno = errno; + if (!errno) + errno = saved_errno; _THREAD_PRIVATE_MUTEX_UNLOCK(pw); - return (pwret ? 0 : 1); + return (my_errno); } struct passwd * @@ -753,8 +761,12 @@ getpwuid_r(uid_t uid, struct passwd *pw, { struct passwd *pwret = NULL; int flags = 0, *flagsp; + int my_errno = 0; + int saved_errno; _THREAD_PRIVATE_MUTEX_LOCK(pw); + saved_errno = errno; + errno = 0; if (!_pw_db && !__initdb()) goto fail; @@ -778,8 +790,12 @@ getpwuid_r(uid_t uid, struct passwd *pw, fail: if (pwretp) *pwretp = pwret; + if (pwret == NULL) + my_errno = errno; + if (!errno) + errno = saved_errno; _THREAD_PRIVATE_MUTEX_UNLOCK(pw); - return (pwret ? 0 : 1); + return (my_errno); } struct passwd * Index: getgrent.c === RCS file: /cvs/src/lib/libc/gen/getgrent.c,v retrieving revision 1.38 diff -u -p -r1.38 getgrent.c --- getgrent.c 17 Apr 2013 17:40:35 - 1.38 +++ getgrent.c 19 Feb 2014 16:36:49 - @@ -134,6 +134,7 @@ getgrnam_r(const char *name, struct grou if (bufsize < GETGR_R_SIZE_MAX) return ERANGE; errnosave = errno; + errno = 0; *result = getgrnam_gs(name, grp, (struct group_storage *)buffer); if (*result == NULL) ret = errno; @@ -180,6 +181,7 @@ getgrgid_r(gid_t gid, struct group *grp, if (bufsize < GETGR_R_SIZE_MAX) return ERANGE; errnosave = errno; + errno = 0; *result = getgrgid_gs(gid, grp, (struct group_storage *)buffer); if (*result == NULL) ret
Re: Bad return value for getpwnam_r et al
> Date: Mon, 24 Feb 2014 16:51:42 +0100 > From: Ingo Schwarze > > Hi Stuart, > > Stuart Henderson wrote on Sun, Feb 23, 2014 at 02:22:47PM +: > > > btw, here are results from a search for getpw(nam|uid)_r in unpacked > > ports source, after removing a bunch of autoconf checks etc. > > Wow, these are nearly a hundred ports. It is not feasible to audit > all that. Besides, my patches also imply slight changes to the > behaviour of getgr{nam,gid}_r() (when called with errno already set) > and to getpw{nam,uid}() (which may now set errno even when finding > a match and will no longer set errno with the third patch in that > case). So your list is too long for a complete audit even though > it is incomplete. > > Anyway, i looked at seven examples from your list, choosing ports > from different categories. The results are, if i read the code > correctly: > > - I found no cases where my patches might break something. > - One of the seven ports would be significantly improved (postfix). > - Three ports get minor improvements (gdm, glib2, ruby). > - For three ports, it makes no difference (gmake, postgres, gnutls). > - Nearly all changes in behaviour are due to the first patch - >change of the return values - and among these, nearly all >are due to changing the return value from 1 to 0 when not >finding a match, but when there is no error. > - The errno variable and the exact return value (as opposed to >merely whether it's 0 or non-0) are rarely looked at. > - For detailed results, look at the very end of this mail. > > Even after unlock, i don't think it would be reasonable to try to > evaluate all the consequences of the changes. After unlock, we > should just commit all the changes such that libc behaves reasonably, > then watch out for any fallout. > > What do we commit now? > > After this review, i'd say: > > * PLEASE somebody give me an OK just for the first patch. >It actually improves various ports. >It doesn't look like it's going to break stuff. > (Theo said he is not opposed to fixing part of this, > but cannot look at the details right now.) > > * Let's not commit the second patch > (avoid errno clobbering as suggested by guenther@). >It doesn't seem to matter much, so let's avoid any risk. > > * Let's not commit the third patch > (let getpw*_r() not touch errno as suggested by kettenis@). >Even though i didn't find any port where this might matter, >i still consider it more risky than the first two patches. > > Here is the first patch, again. While I agree this is a step in the right direction, and probably not going to break stuff, I'm not sure this is the time to change this. It does change the behaviour, and some ports will end up in codepaths that have not been tested on OpenBSD. > Index: getpwent.c > === > RCS file: /cvs/src/lib/libc/gen/getpwent.c,v > retrieving revision 1.48 > diff -u -p -r1.48 getpwent.c > --- getpwent.c15 Nov 2013 22:32:55 - 1.48 > +++ getpwent.c19 Feb 2014 16:36:48 - > @@ -708,8 +708,12 @@ getpwnam_r(const char *name, struct pass > { > struct passwd *pwret = NULL; > int flags = 0, *flagsp; > + int my_errno = 0; > + int saved_errno; > > _THREAD_PRIVATE_MUTEX_LOCK(pw); > + saved_errno = errno; > + errno = 0; > if (!_pw_db && !__initdb()) > goto fail; > > @@ -733,8 +737,12 @@ getpwnam_r(const char *name, struct pass > fail: > if (pwretp) > *pwretp = pwret; > + if (pwret == NULL) > + my_errno = errno; > + if (!errno) > + errno = saved_errno; > _THREAD_PRIVATE_MUTEX_UNLOCK(pw); > - return (pwret ? 0 : 1); > + return (my_errno); > } > > struct passwd * > @@ -753,8 +761,12 @@ getpwuid_r(uid_t uid, struct passwd *pw, > { > struct passwd *pwret = NULL; > int flags = 0, *flagsp; > + int my_errno = 0; > + int saved_errno; > > _THREAD_PRIVATE_MUTEX_LOCK(pw); > + saved_errno = errno; > + errno = 0; > if (!_pw_db && !__initdb()) > goto fail; > > @@ -778,8 +790,12 @@ getpwuid_r(uid_t uid, struct passwd *pw, > fail: > if (pwretp) > *pwretp = pwret; > + if (pwret == NULL) > + my_errno = errno; > + if (!errno) > + errno = saved_errno; > _THREAD_PRIVATE_MUTEX_UNLOCK(pw); > - return (pwret ? 0 : 1); > + return (my_errno); > } > > struct passwd * > Index: getgrent.c > === > RCS file: /cvs/src/lib/libc/gen/getgrent.c,v > retrieving revision 1.38 > diff -u -p -r1.38 getgrent.c > --- getgrent.c17 Apr 2013 17:40:35 - 1.38 > +++ getgrent.c19 Feb 2014 16:36:49 - > @@ -134,6 +134,7 @@ getgrnam_r(const char *name, struct grou > if (bufsize < GETGR_R_SIZE_MAX) >
Re: Bad return value for getpwnam_r et al
> While I agree this is a step in the right direction, and probably not > going to break stuff, I'm not sure this is the time to change this. > It does change the behaviour, and some ports will end up in codepaths Agreed. This is postponed to after release. It didn't make the cutoff.