NetBSD Problem Report #46338

From www@NetBSD.org  Mon Apr 16 07:07:26 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 5096F63B915
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 16 Apr 2012 07:07:26 +0000 (UTC)
Message-Id: <20120416070725.635D763B86B@www.NetBSD.org>
Date: Mon, 16 Apr 2012 07:07:25 +0000 (UTC)
From: nathanialsloss@yahoo.com.au
Reply-To: nathanialsloss@yahoo.com.au
To: gnats-bugs@NetBSD.org
Subject: memcpy/memmove/bcopy kernel panics when hitting page boundary on i386
X-Send-Pr-Version: www-1.0

>Number:         46338
>Category:       kern
>Synopsis:       ubt(4) crash
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 16 07:10:00 +0000 2012
>Closed-Date:    Sun Sep 09 01:08:03 +0000 2012
>Last-Modified:  Sun Sep 09 01:08:03 +0000 2012
>Originator:     Nat Sloss
>Release:        NetBSD Current 6.99.4
>Organization:
>Environment:
NetBSD beast 6.99.4 NetBSD 6.99.4 (LOCKDEBUG) #81: Mon Apr 16 12:31:11 EST 2012  build@beast:/home/build/src/sys/arch/i386/compile/obj/LOCKDEBUG i386
>Description:
Hi I've experienced kernel crashes with iosynchronous usb transfers and this problem could be the cause of kern/38432.

Occasionally the kernel will panic trap type 6 code 0 or code 2 in memcpy+0x14 repe movsl "registers".

I have discovered that the source file memcpy.S doesn't handle page boundaries as the error code 0 is read error, page not loaded supervisor access, and code 2 for write.

The kernel panics whenever an address is accessed on a page boundary because of the use of MOVS.

This type of panic will occur when either the source address or destination address + copy length crosses a page boundary.

>How-To-Repeat:
This problem will occur when ever source address + copy length is greater than 65536 or destination address + copy length is greater than 65536.

ie memcpy( 0xXXXXFFF0, 0xXXXXFFEF, 0x30);

It effects all drivers using memcpy although I notice it consistently with usb and in particular audio.

Could you please pull up NetBSD 5 and 6 when a solution is found.

And just a question will I have to recompile all binaries on my system or just the kernel.
>Fix:
Memcopy should copy to in page size segments due to the use of movs.

copy:
if dist + length > 0xFFFF then
        copy_length = 0xFFFF - dist

if src + copy_length > 0xFFFF then
        copy_length = 0xFFFF - src

cx register = copy_length
length -= copy_length
rep
movsl

if length > 0 then
        goto copy


>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-i386/46338: memcpy/memmove/bcopy kernel panics when hitting page boundary on i386
Date: Mon, 16 Apr 2012 09:17:26 +0200

 On Mon, Apr 16, 2012 at 07:10:01AM +0000, nathanialsloss@yahoo.com.au wrote:
 > I have discovered that the source file memcpy.S doesn't handle page
 > boundaries

 Why should it? It is not memcopy's task to make sure the pages are mapped.
 Can you provide a backtrace from such a panic?

 Martin

From: Nat Sloss <nathanialsloss@yahoo.com.au>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-i386/46338: memcpy/memmove/bcopy kernel panics when hitting page boundary on i386
Date: Tue, 24 Apr 2012 12:20:00 +1000

 Hi.

 I had made a mistake the kernel paniced and worst yet so did I.

 There is nothing wrong with memcpy, bcopy, or memmove.  What causes the crash 
 is not limited to the i386 but to all platforms that support the ubt usb 
 bluetooth controller.

 I have successfully analyzed and fixed the problem so here's what I've found:

 To cause the crash.  Whilst using a ubt supported controller attach and open a 
 bluetooth audio sco connection.  Play music/ audio on the sco audio device.

 Whilst music is playing on the sco device, switch between X and text sessions, 
 keep doing this and in about six or so attempts it will crash.  Sometimes 
 control-c interrupting the audio process will cause the crash.

 You should then have the following from ddb:

 fatal page fault in supervisor mode
 trap type 6 code 2 eip c08c9da4 cs 8 eflags 210213 cr2 c52d0000 ilevel 4
 kernel: supervisor trap page fault, code=0
 Stopped in pid 0.3 (system) at  netbsd:memcpy+0x14:     repe movsl      
 (%esi),%
 es:(%edi)
 db{0}> show registers
 ds          c0c50010    pv_hash_heads+0x1b3d0
 es          10
 fs          c08c0030    null_extant+0xf8
 gs          dbe30010
 edi         c52cfffd
 esi         dd04792b
 ebp         dbe3dc50
 ebx         11
 edx         de
 ecx         3fff5dc0
 eax         fffffffb
 eip         c08c9da4    memcpy+0x14
 cs          8
 eflags      210213
 esp         dbe3dbd0
 ss          10
 netbsd:memcpy+0x14:     repe movsl      (%esi),%es:(%edi)
 db{0}> dmesg 200
 emulation)
 wsdisplay0: screen 2 added (80x25, vt100 emulation)
 wsdisplay0: screen 3 added (80x25, vt100 emulation)
 wsdisplay0: screen 4 added (80x25, vt100 emulation)
 bthub0 at ubt0 local-bdaddr 00:03:7a:c8:7f:e4
 btsco0 at bthub0 remote-bdaddr 50:c9:71:2b:10:6f channel 2
 audio2 at btsco0: full duplex, playback, capture
 i915drm0: interrupting at ioapic0 pin 16
 i915drm0: interrupting at ioapic0 pin 16
 fatal page fault in supervisor mode
 trap type 6 code 2 eip c08c9da4 cs 8 eflags 210213 cr2 c52d0000 ilevel 4
 db{0}> bt
 memcpy(c4c59284,c4a77268,0,c081c07a,c0bed420,0,0,0,a,c0c30780) at 
 netbsd:memcpy+
 0x14
 usb_transfer_complete(c4c59284,1234,4,a,c44fff80,c4c592d8,c4c592d8,c4c59284,c4c5
 296c,c4d57044) at netbsd:usb_transfer_complete+0x18a
 uhci_idone.clone.8(c4c592d8,1264,4,a,8,c052c9e1,0,c4b6a000,c0b8dac0,dc376278) 
 at
  netbsd:uhci_idone.clone.8+0xb4
 uhci_softintr(c4b6a004,0,0,c0100307,0,10,0,c45e3a80,10,c45e3a80) at 
 netbsd:uhci_
 softintr+0x1a3
 softint_dispatch(c45e3d20,4,0,0,ffffffff,ffffffff,dbe3dd90,dbe3dc24,dbe3dc40,0) 
 a
 t netbsd:softint_dispatch+0x72
 fatal page fault in supervisor mode
 trap type 6 code 0 eip c027eef0 cs 8 eflags 210246 cr2 38 ilevel 8
 kernel: supervisor trap page fault, code=0
 Faulted in DDB; continuing...

 Note the high value of the cx register.

 The memcpy (found after extensive testing) is from a callback to 
 ubt_recv_sco_complete.

 In ubt_recv_sco_complete:  It either memcopys 'size' or MHLEN - 'got' (if size 
 + got is > MHLEN.)  So what causes the crash is when got is greater than 
 MHLEN resulting in an extremely large memcpy length.  Which memcpy does until 
 hitting an unmapped page.  So I was actually good that it crashed.

 The solution is not to memcpy if got is greater than or equal to MHLEN which 
 is probably a more succinct test than I used in my patch.  I decided it was 
 also a good idea to test whether size was zero or greater than MHLEN so to 
 avoid any potential crashes, but I'm not certain that this is necessary.

 So here's my patch:

 --- ubt.c.orig  2012-04-07 21:28:18.000000000 +1000
 +++ ubt.c       2012-04-22 14:51:14.000000000 +1000
 @@ -1672,10 +1672,14 @@
                         if (got + size > want)
                                 size = want - got;

 -                       if (got + size > MHLEN)
 +                       if (got + size > MHLEN) {
 +                               if (MHLEN - got > 0 && MHLEN - got <= MHLEN)
                                 memcpy(ptr, frame, MHLEN - got);
 -                       else
 -                               memcpy(ptr, frame, size);
 +                       }
 +                       else {
 +                               if (size > 0 && size <= MHLEN)
 +                                       memcpy(ptr, frame, size);
 +                       }

                         ptr += size;
                         got += size;


 This problem affects NetBSD 5 and current,  so if considered could NetBSD 5 
 and 6 be pulled up.


 Regards,

 Nat.

Responsible-Changed-From-To: port-i386-maintainer->kern-bug-people
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Tue, 24 Apr 2012 07:30:04 +0000
Responsible-Changed-Why:
Problem is in the ubt driver, which is machine-independent.

regarding the patch: wouldn't it be better to check for bogus values of
size and/or got earlier instead of limiting the memcpy?


From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46338 CVS commit: src/sys/dev/usb
Date: Tue, 24 Apr 2012 16:04:50 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Tue Apr 24 20:04:50 UTC 2012

 Modified Files:
 	src/sys/dev/usb: ubt.c

 Log Message:
 PR/46338: Nat Sloss: Prevent ubt synchronization loss from overwriting memory.


 To generate a diff of this commit:
 cvs rdiff -u -r1.46 -r1.47 src/sys/dev/usb/ubt.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Nat Sloss <nathanialsloss@yahoo.com.au>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/46338 CVS commit: src/sys/dev/usb
Date: Mon, 30 Apr 2012 02:27:30 +1000

 Hi.

 Many thanks Christos, your patch works perfectly.  I tested it vigorously and 
 no more crashes.

 And your patch was by far better than anything I could have conceived.

 So thanks again, I just hope it makes NetBSD 6 before its pending release.

 Regards,

 Nat.

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	nathanialsloss@yahoo.com.au
Cc: 
Subject: Re: PR/46338 CVS commit: src/sys/dev/usb
Date: Sun, 29 Apr 2012 13:50:06 -0400

 On Apr 29,  4:30pm, nathanialsloss@yahoo.com.au (Nat Sloss) wrote:
 -- Subject: Re: PR/46338 CVS commit: src/sys/dev/usb

 |  Hi.
 |  
 |  Many thanks Christos, your patch works perfectly.  I tested it vigorously and 
 |  no more crashes.

 You are welcome!

 |  And your patch was by far better than anything I could have conceived.

 Well, keep programming and you'll be better than me soon :-)

 |  So thanks again, I just hope it makes NetBSD 6 before its pending release.

 I will submit a pullup.

 christos

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46338 CVS commit: [netbsd-6] src/sys/dev/usb
Date: Mon, 7 May 2012 16:25:42 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Mon May  7 16:25:42 UTC 2012

 Modified Files:
 	src/sys/dev/usb [netbsd-6]: ubt.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #216):
 	sys/dev/usb/ubt.c: revision 1.47
 PR/46338: Nat Sloss: Prevent ubt synchronization loss from overwriting memory.


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.44.2.1 src/sys/dev/usb/ubt.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->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 09 Sep 2012 01:08:03 +0000
State-Changed-Why:
fixed and pulled up in april.


>Unformatted:

Home	PR Database Search