Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, Jan 31 2008 at 1:08 +0200, Alan Stern [EMAIL PROTECTED] wrote:
Boaz:
This looks like it might have something to do with your changes.
Alan Stern
On Wed, 30 Jan 2008, Mark Glines wrote:
:
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Hi,
[ 95.406864] usb 4-1: new full speed USB device using uhci_hcd and address
2
[ 95.550372] usb 4-1: configuration #1 chosen from 1 choice
[ 95.622808] usbcore: registered new interface driver libusual
[ 95.681971] Initializing USB Mass Storage driver...
[ 95.775056] scsi3 : SCSI emulation for USB Mass Storage devices
[ 95.777071] usbcore: registered new interface driver usb-storage
[ 95.777084] USB Mass Storage support registered.
[ 95.777928] usb-storage: device found at 2
[ 95.777935] usb-storage: waiting for device to settle before scanning
[ 97.291557] Unable to handle kernel NULL pointer dereference at
RIP:
[ 97.291567] [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291587] PGD 6a25c067 PUD 6a3f1067 PMD 0
[ 97.291596] Oops: [1] PREEMPT SMP
[ 97.291603] CPU 1
[ 97.291607] Modules linked in: usb_storage libusual i915 drm ipv6 nfsd
lockd nfs_acl auth_rpcgss sunrpc exportfs rfcomm l2cap dm_mod fuse dock tun
kvm_intel kvm acpi_cpufreq rfkill_input rfkill coretemp bonding hci_usb
bluetooth arc4 ecb iwl4965 snd_hda_intel mac80211 snd_pcm uhci_hcd ehci_hcd
usbcore pcmcia snd_timer snd_page_alloc ide_cd firmware_class psmouse
snd_hwdep parport_pc yenta_socket snd 8250_pnp parport 8250 rsrc_nonstatic
cdrom thermal video cfg80211 pcmcia_core i2c_i801 ac serial_core battery
intel_agp button output sg evdev
[ 97.291706] Pid: 7107, comm: usb-storage Not tainted 2.6.24 #3
[ 97.291711] RIP: 0010:[8835fa4e] [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291727] RSP: 0018:810067e71db0 EFLAGS: 00010283
[ 97.291732] RAX: 810074586600 RBX: 87654321 RCX:
[ 97.291737] RDX: 0004 RSI: 810067cfb424 RDI:
810074586624
[ 97.291742] RBP: 810067e71e10 R08: 810002973550 R09:
[ 97.291747] R10: 81006a236000 R11: R12:
[ 97.291751] R13: 0024 R14: 0024 R15:
0600
[ 97.291757] FS: () GS:81007d00d500()
knlGS:
[ 97.291763] CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b
[ 97.291768] CR2: CR3: 6a2ea000 CR4:
26e0
[ 97.291772] DR0: DR1: DR2:
[ 97.291777] DR3: DR6: 0ff0 DR7:
0400
[ 97.291783] Process usb-storage (pid: 7107, threadinfo 810067e7,
task 810067d10f20)
[ 97.291788] Stack: 0003 810067e71e2c
810067e71e20
[ 97.291800] 006067d10f20 810067cfb400 810002973550
0060
[ 97.291810] 81006a236000 810067d14c78 810067d14df0
810067e71eb0
[ 97.291820] Call Trace:
[ 97.291840] [8835fc14]
:usb_storage:usb_stor_set_xfer_buf+0x34/0x60
[ 97.291857] [88366d58]
:usb_storage:isd200_ata_command+0x188/0x530
[ 97.291877] [8836142e]
:usb_storage:usb_stor_control_thread+0x1be/0x250
[ 97.291892] [812b5106] _spin_unlock_irqrestore+0x16/0x40
[ 97.291906] [88361270]
:usb_storage:usb_stor_control_thread+0x0/0x250
[ 97.291918] [8105290d] kthread+0x4d/0x80
[ 97.291928] [8100d2e8] child_rip+0xa/0x12
[ 97.291943] [810528c0] kthread+0x0/0x80
[ 97.291950] [8100d2de] child_rip+0x0/0x12
[ 97.291956]
[ 97.291958]
[ 97.291959] Code: 49 39 1c 24 0f 85 78 01 00 00 4d 8b 44 24 08 41 f6 c0
01 0f
[ 97.291985] RIP [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291999] RSP 810067e71db0
[ 97.292002] CR2:
[ 97.292009] ---[ end trace 77e35c70f05f8a11 ]---
:usb_storage:usb_stor_access_xfer_buf+0xde is the first pointer
dereference in an inlined call to sg_page().
static inline struct page *sg_page(struct scatterlist *sg)
{
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg-sg_magic != SG_MAGIC);
BUG_ON(sg_is_chain(sg));
#endif
return (struct page *)((sg)-page_link ~0x3);
}
Turning on CONFIG_DEBUG_SG didn't make much difference, because the sg
pointer itself is NULL. Thus, it segfaulted before BUG_ON was actually
called.
This hardware worked fine with 2.6.23.12... though my kernel config is
rather different from that one. Before I start digging and trying to
work out all the details and hopefully fix it, I was wondering if you've
already seen this?
Thanks,
Mark
Please check the below patch.
one thing that I can see is
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, Jan 31 2008 at 17:08 +0200, Mark Glines [EMAIL PROTECTED] wrote: On Thu, 31 Jan 2008 11:27:39 +0200 Boaz Harrosh [EMAIL PROTECTED] wrote: Please check the below patch. one thing that I can see is that the isd200 does an INQUARY transfer of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c sends an INQUARY with 36 bytes buffer. So we have an underflow in usb_stor_access_xfer_buf(). The below patch will only check my theory. I will send a proper fix later, please confirm that this fixes it. What kills me is that this condition has existed before my patch, I'll try to see why it is triggered now I applied this patch to 2.6.24, and it now works for me. It was crashing consistently whenever I'd plug this device in, now it goes through successfully: Yes Thanks this is grate :) I will send a proper patch to scsi maintainer. Alan is it OK to send this patch threw James's scsi-misc? [24775.788039] usb 3-2: new full speed USB device using uhci_hcd and address 3 [24775.939275] usb 3-2: configuration #1 chosen from 1 choice [24776.084409] usbcore: registered new interface driver libusual [24776.103604] Initializing USB Mass Storage driver... [24776.213916] scsi3 : SCSI emulation for USB Mass Storage devices [24776.214366] usbcore: registered new interface driver usb-storage [24776.214377] USB Mass Storage support registered. [24776.215604] usb-storage: device found at 3 [24776.215724] usb-storage: waiting for device to settle before scanning [24778.78] scsi 3:0:0:0: Direct-Access SAMSUNG HM120JC YL10 PQ: 0 ANSI: 0 [24778.333715] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 MB) [24778.333841] sd 3:0:0:0: [sdb] Write Protect is off [24778.333848] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 [24778.333853] sd 3:0:0:0: [sdb] Assuming drive cache: write through [24778.334196] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 MB) [24778.334396] sd 3:0:0:0: [sdb] Write Protect is off [24778.334403] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 [24778.334408] sd 3:0:0:0: [sdb] Assuming drive cache: write through [24778.334414] sdb: sdb1 [24778.824103] sd 3:0:0:0: [sdb] Attached SCSI disk [24778.824210] sd 3:0:0:0: Attached scsi generic sg1 type 0 [24778.825119] usb-storage: device scan complete I'm happy to test further patches. Let me know if you need more testing. Do you still want me to try out the scsi-misc branch? No, That was my mistake, scsi-misc is now identical to mainline. This here is a new fix that will need to go in. I will send a patch soonish. If you can test it and send a Tested-by: it could be grate Mark --- drivers/usb/storage/protocol.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c index a41ce21..d0ff1f6 100644 --- a/drivers/usb/storage/protocol.c +++ b/drivers/usb/storage/protocol.c @@ -229,6 +229,12 @@ void usb_stor_set_xfer_buf(unsigned char *buffer, unsigned int offset = 0; struct scatterlist *sg = NULL; +BUG_ON(!scsi_sglist(srb)); + +if(buflen scsi_bufflen(srb)) +buflen = scsi_bufflen(srb); +/*FIXME: should we set an underflow condition here*/ + usb_stor_access_xfer_buf(buffer, buflen, srb, sg, offset, TO_XFER_BUF); if (buflen scsi_bufflen(srb)) Thanks Mark (CCing linux-scsi ml) Boaz - To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, 31 Jan 2008 11:27:39 +0200 Boaz Harrosh [EMAIL PROTECTED] wrote: Please check the below patch. one thing that I can see is that the isd200 does an INQUARY transfer of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c sends an INQUARY with 36 bytes buffer. So we have an underflow in usb_stor_access_xfer_buf(). The below patch will only check my theory. I will send a proper fix later, please confirm that this fixes it. What kills me is that this condition has existed before my patch, I'll try to see why it is triggered now I applied this patch to 2.6.24, and it now works for me. It was crashing consistently whenever I'd plug this device in, now it goes through successfully: [24775.788039] usb 3-2: new full speed USB device using uhci_hcd and address 3 [24775.939275] usb 3-2: configuration #1 chosen from 1 choice [24776.084409] usbcore: registered new interface driver libusual [24776.103604] Initializing USB Mass Storage driver... [24776.213916] scsi3 : SCSI emulation for USB Mass Storage devices [24776.214366] usbcore: registered new interface driver usb-storage [24776.214377] USB Mass Storage support registered. [24776.215604] usb-storage: device found at 3 [24776.215724] usb-storage: waiting for device to settle before scanning [24778.78] scsi 3:0:0:0: Direct-Access SAMSUNG HM120JC YL10 PQ: 0 ANSI: 0 [24778.333715] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 MB) [24778.333841] sd 3:0:0:0: [sdb] Write Protect is off [24778.333848] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 [24778.333853] sd 3:0:0:0: [sdb] Assuming drive cache: write through [24778.334196] sd 3:0:0:0: [sdb] 234441648 512-byte hardware sectors (120034 MB) [24778.334396] sd 3:0:0:0: [sdb] Write Protect is off [24778.334403] sd 3:0:0:0: [sdb] Mode Sense: 00 00 00 00 [24778.334408] sd 3:0:0:0: [sdb] Assuming drive cache: write through [24778.334414] sdb: sdb1 [24778.824103] sd 3:0:0:0: [sdb] Attached SCSI disk [24778.824210] sd 3:0:0:0: Attached scsi generic sg1 type 0 [24778.825119] usb-storage: device scan complete I'm happy to test further patches. Let me know if you need more testing. Do you still want me to try out the scsi-misc branch? Mark --- drivers/usb/storage/protocol.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c index a41ce21..d0ff1f6 100644 --- a/drivers/usb/storage/protocol.c +++ b/drivers/usb/storage/protocol.c @@ -229,6 +229,12 @@ void usb_stor_set_xfer_buf(unsigned char *buffer, unsigned int offset = 0; struct scatterlist *sg = NULL; + BUG_ON(!scsi_sglist(srb)); + + if(buflen scsi_bufflen(srb)) + buflen = scsi_bufflen(srb); + /*FIXME: should we set an underflow condition here*/ + usb_stor_access_xfer_buf(buffer, buflen, srb, sg, offset, TO_XFER_BUF); if (buflen scsi_bufflen(srb)) - To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, 31 Jan 2008, Boaz Harrosh wrote: Please check the below patch. one thing that I can see is that the isd200 does an INQUARY transfer of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c sends an INQUARY with 36 bytes buffer. So we have an underflow in usb_stor_access_xfer_buf(). Maybe the isd200 routine should be changed also, so that it doesn't try to store too much data in the buffer. I will send a proper patch to scsi maintainer. Alan is it OK to send this patch threw James's scsi-misc? You should send patches to Matt Dharm, since he is the usb-storage maintainer. Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
On Thu, Jan 31 2008 at 18:45 +0200, Alan Stern [EMAIL PROTECTED] wrote: On Thu, 31 Jan 2008, Boaz Harrosh wrote: Please check the below patch. one thing that I can see is that the isd200 does an INQUARY transfer of sizeof(struct inquiry_data) which is 96 bytes, when scsi_scan.c sends an INQUARY with 36 bytes buffer. So we have an underflow in usb_stor_access_xfer_buf(). Maybe the isd200 routine should be changed also, so that it doesn't try to store too much data in the buffer. I will send a proper patch to scsi maintainer. Alan is it OK to send this patch threw James's scsi-misc? You should send patches to Matt Dharm, since he is the usb-storage maintainer. Alan Stern - Right, Please see patch posted as reply to the original email. I have also fixed isd200 to return what was asked. The fix to protocol.c is also different and more general now. Will send to Matthew Dharm. Matthew - is it OK to send this threw James, please ACK. Mark - this fix is different we do need testing. Thanks to all Boaz - To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] 2.6.24: NULL scatter-gather pointer in usb_storage:usb_stor_access_xfer_buf?
Boaz:
This looks like it might have something to do with your changes.
Alan Stern
On Wed, 30 Jan 2008, Mark Glines wrote:
:
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Hi,
[ 95.406864] usb 4-1: new full speed USB device using uhci_hcd and address 2
[ 95.550372] usb 4-1: configuration #1 chosen from 1 choice
[ 95.622808] usbcore: registered new interface driver libusual
[ 95.681971] Initializing USB Mass Storage driver...
[ 95.775056] scsi3 : SCSI emulation for USB Mass Storage devices
[ 95.777071] usbcore: registered new interface driver usb-storage
[ 95.777084] USB Mass Storage support registered.
[ 95.777928] usb-storage: device found at 2
[ 95.777935] usb-storage: waiting for device to settle before scanning
[ 97.291557] Unable to handle kernel NULL pointer dereference at
RIP:
[ 97.291567] [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291587] PGD 6a25c067 PUD 6a3f1067 PMD 0
[ 97.291596] Oops: [1] PREEMPT SMP
[ 97.291603] CPU 1
[ 97.291607] Modules linked in: usb_storage libusual i915 drm ipv6 nfsd
lockd nfs_acl auth_rpcgss sunrpc exportfs rfcomm l2cap dm_mod fuse dock tun
kvm_intel kvm acpi_cpufreq rfkill_input rfkill coretemp bonding hci_usb
bluetooth arc4 ecb iwl4965 snd_hda_intel mac80211 snd_pcm uhci_hcd ehci_hcd
usbcore pcmcia snd_timer snd_page_alloc ide_cd firmware_class psmouse
snd_hwdep parport_pc yenta_socket snd 8250_pnp parport 8250 rsrc_nonstatic
cdrom thermal video cfg80211 pcmcia_core i2c_i801 ac serial_core battery
intel_agp button output sg evdev
[ 97.291706] Pid: 7107, comm: usb-storage Not tainted 2.6.24 #3
[ 97.291711] RIP: 0010:[8835fa4e] [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291727] RSP: 0018:810067e71db0 EFLAGS: 00010283
[ 97.291732] RAX: 810074586600 RBX: 87654321 RCX:
[ 97.291737] RDX: 0004 RSI: 810067cfb424 RDI:
810074586624
[ 97.291742] RBP: 810067e71e10 R08: 810002973550 R09:
[ 97.291747] R10: 81006a236000 R11: R12:
[ 97.291751] R13: 0024 R14: 0024 R15:
0600
[ 97.291757] FS: () GS:81007d00d500()
knlGS:
[ 97.291763] CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b
[ 97.291768] CR2: CR3: 6a2ea000 CR4:
26e0
[ 97.291772] DR0: DR1: DR2:
[ 97.291777] DR3: DR6: 0ff0 DR7:
0400
[ 97.291783] Process usb-storage (pid: 7107, threadinfo 810067e7,
task 810067d10f20)
[ 97.291788] Stack: 0003 810067e71e2c
810067e71e20
[ 97.291800] 006067d10f20 810067cfb400 810002973550
0060
[ 97.291810] 81006a236000 810067d14c78 810067d14df0
810067e71eb0
[ 97.291820] Call Trace:
[ 97.291840] [8835fc14]
:usb_storage:usb_stor_set_xfer_buf+0x34/0x60
[ 97.291857] [88366d58]
:usb_storage:isd200_ata_command+0x188/0x530
[ 97.291877] [8836142e]
:usb_storage:usb_stor_control_thread+0x1be/0x250
[ 97.291892] [812b5106] _spin_unlock_irqrestore+0x16/0x40
[ 97.291906] [88361270]
:usb_storage:usb_stor_control_thread+0x0/0x250
[ 97.291918] [8105290d] kthread+0x4d/0x80
[ 97.291928] [8100d2e8] child_rip+0xa/0x12
[ 97.291943] [810528c0] kthread+0x0/0x80
[ 97.291950] [8100d2de] child_rip+0x0/0x12
[ 97.291956]
[ 97.291958]
[ 97.291959] Code: 49 39 1c 24 0f 85 78 01 00 00 4d 8b 44 24 08 41 f6 c0 01
0f
[ 97.291985] RIP [8835fa4e]
:usb_storage:usb_stor_access_xfer_buf+0xde/0x270
[ 97.291999] RSP 810067e71db0
[ 97.292002] CR2:
[ 97.292009] ---[ end trace 77e35c70f05f8a11 ]---
:usb_storage:usb_stor_access_xfer_buf+0xde is the first pointer
dereference in an inlined call to sg_page().
static inline struct page *sg_page(struct scatterlist *sg)
{
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg-sg_magic != SG_MAGIC);
BUG_ON(sg_is_chain(sg));
#endif
return (struct page *)((sg)-page_link ~0x3);
}
Turning on CONFIG_DEBUG_SG didn't make much difference, because the sg
pointer itself is NULL. Thus, it segfaulted before BUG_ON was actually
called.
This hardware worked fine with 2.6.23.12... though my kernel config is
rather different from that one. Before I start digging and trying to
work out all the details and hopefully fix it, I was wondering if you've
already seen this?
Thanks,
Mark
-
This SF.net email is sponsored by: Microsoft
Defy
