* [BusLogic] DMA-API: device driver failed to check map error
@ 2013-09-10 6:03 Tetsuo Handa
2013-09-10 12:36 ` James Bottomley
0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2013-09-10 6:03 UTC (permalink / raw)
To: khalid; +Cc: JBottomley, linux-kernel
Hello.
I got below warning on current linux.git .
----------
[ 2.612237] scsi: ***** BusLogic SCSI Driver Version 2.1.16 of 18 July 2002 *****
[ 2.613067] scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>
[ 2.630942] scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
[ 2.633063] scsi0: Firmware Version: 5.07B, I/O Address: 0x10C0, IRQ Channel: 17/Level
[ 2.633125] scsi0: PCI Bus: 0, Device: 16, Address: 0xD8800000, Host Adapter SCSI ID: 7
[ 2.633188] scsi0: Parity Checking: Enabled, Extended Translation: Enabled
[ 2.633250] scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
[ 2.635721] scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
[ 2.637054] scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
[ 2.637116] scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
[ 2.637179] scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
[ 2.639812] scsi0: *** BusLogic BT-958 Initialized Successfully ***
[ 4.635828] scsi0 : BusLogic BT-958
[ 4.641883] [sched_delayed] sched: RT throttling activated
[ 4.647510] ------------[ cut here ]------------
[ 4.647573] WARNING: CPU: 1 PID: 1 at lib/dma-debug.c:937 check_unmap+0x777/0x7f0()
[ 4.648851] pci 0000:00:10.0: DMA-API: device driver failed to check map error[device address=0x00000000349cddc0] [size=96 bytes] [mapped as single]
[ 4.649873] Modules linked in:
[ 4.649873] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.11.0-08716-g26b0332-dirty #8
[ 4.649873] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 08/15/2008
[ 4.649873] 000003a9 00000000 f64b7de4 c11e8096 f64b7df0 c11e80dd c1210437 f64b7e1c
[ 4.649873] c10421b9 c1519440 f64b7e4c 00000001 c1518220 000003a9 c1210437 f64b7e2c
[ 4.649873] 00000001 f64b7ea4 f64b7e38 c1042211 00000009 f64b7e2c c1519440 f64b7e4c
[ 4.649873] Call Trace:
[ 4.649873] [<c11e8096>] __dump_stack+0x16/0x20
[ 4.649873] [<c11e80dd>] dump_stack+0x3d/0x60
[ 4.649873] [<c1210437>] ? check_unmap+0x777/0x7f0
[ 4.649873] [<c10421b9>] warn_slowpath_common+0x79/0xa0
[ 4.649873] [<c1210437>] ? check_unmap+0x777/0x7f0
[ 4.649873] [<c1042211>] warn_slowpath_fmt+0x31/0x40
[ 4.649873] [<c1210437>] check_unmap+0x777/0x7f0
[ 4.649873] [<c1210e18>] debug_dma_unmap_page+0x78/0x90
[ 4.649873] [<c1289703>] blogic_dealloc_ccb+0x83/0xb0
[ 4.649873] [<c128a847>] blogic_process_ccbs+0x3c7/0x3f0
[ 4.649873] [<c128a423>] ? blogic_scan_inbox+0x43/0xa0
[ 4.649873] [<c128a8f1>] blogic_inthandler+0x81/0x150
[ 4.649873] [<c10a9b4e>] ? handle_irq_event+0x2e/0x60
[ 4.649873] [<c10a9a38>] handle_irq_event_percpu+0x38/0x120
[ 4.649873] [<c10a9b4e>] ? handle_irq_event+0x2e/0x60
[ 4.649873] [<c10a9b57>] handle_irq_event+0x37/0x60
[ 4.649873] [<c10ac4d0>] ? handle_level_irq+0xb0/0xb0
[ 4.649873] [<c10ac557>] handle_fasteoi_irq+0x87/0xc0
[ 4.649873] <IRQ> [<c1003fdc>] ? do_IRQ+0x3c/0xb0
[ 4.649873] [<c10948ca>] ? mark_held_locks+0xca/0x100
[ 4.649873] [<c13c9111>] ? common_interrupt+0x31/0x36
[ 4.649873] [<c13c77e7>] ? _raw_spin_unlock_irqrestore+0x47/0x60
[ 4.649873] [<c128b05e>] ? blogic_qcmd+0x3e/0x50
[ 4.649873] [<c127c2f4>] ? scsi_dispatch_cmd+0x174/0x1f0
[ 4.649873] [<c1094b2b>] ? trace_hardirqs_on+0xb/0x10
[ 4.649873] [<c1282e54>] ? scsi_request_fn+0x314/0x3a0
[ 4.649873] [<c11d364b>] ? __blk_run_queue+0x2b/0x40
[ 4.649873] [<c11d9c42>] ? blk_execute_rq_nowait+0xa2/0xd0
[ 4.649873] [<c11d9b70>] ? blk_rq_map_kern+0x130/0x130
[ 4.649873] [<c11d9cfa>] ? blk_execute_rq+0x8a/0xf0
[ 4.649873] [<c11d9b70>] ? blk_rq_map_kern+0x130/0x130
[ 4.649873] [<c11d9f4e>] ? blk_recount_segments+0x1e/0x40
[ 4.649873] [<c11d0000>] ? aes_encrypt+0xe40/0x1450
[ 4.649873] [<c11d9b4f>] ? blk_rq_map_kern+0x10f/0x130
[ 4.649873] [<c1281834>] ? scsi_execute+0xe4/0x150
[ 4.649873] [<c128190e>] ? scsi_execute_req_flags+0x6e/0xa0
[ 4.649873] [<c1284972>] ? scsi_probe_lun+0x112/0x300
[ 4.649873] [<c128512d>] ? scsi_probe_and_add_lun+0x10d/0x2f0
[ 4.649873] [<c1284358>] ? scsi_alloc_sdev+0x248/0x2b0
[ 4.649873] [<c128514d>] ? scsi_probe_and_add_lun+0x12d/0x2f0
[ 4.649873] [<c1259340>] ? anon_transport_class_unregister+0x30/0x30
[ 4.649873] [<c12846c8>] ? scsi_alloc_target+0x1c8/0x220
[ 4.649873] [<c1285b00>] ? __scsi_scan_target+0xa0/0xf0
[ 4.649873] [<c1285c4a>] ? scsi_scan_channel+0x5a/0x90
[ 4.649873] [<c1285d79>] ? scsi_scan_host_selected+0xf9/0x140
[ 4.649873] [<c12860c9>] ? do_scsi_scan_host+0x69/0x80
[ 4.649873] [<c1286130>] ? scsi_scan_host+0x30/0x50
[ 4.649873] [<c15f6d1c>] ? blogic_init+0x32c/0x3b0
[ 4.649873] [<c15f69f0>] ? blogic_inithoststruct+0x80/0x80
[ 4.649873] [<c1000472>] ? do_one_initcall+0x32/0xd0
[ 4.649873] [<c105fa60>] ? parse_one+0xc0/0xe0
[ 4.649873] [<c105fc0a>] ? parse_args+0x7a/0x170
[ 4.649873] [<c15cb410>] ? loglevel+0x30/0x30
[ 4.649873] [<c15cbb6a>] ? do_initcall_level+0x7a/0x90
[ 4.649873] [<c15cb410>] ? loglevel+0x30/0x30
[ 4.649873] [<c15cbb98>] ? do_initcalls+0x18/0x20
[ 4.649873] [<c15cbbc8>] ? do_basic_setup+0x28/0x30
[ 4.649873] [<c15cbc7f>] ? kernel_init_freeable+0x5f/0xf0
[ 4.649873] [<c13c173b>] ? kernel_init+0xb/0xe0
[ 4.649873] [<c13c8c18>] ? ret_from_kernel_thread+0x1c/0x2c
[ 4.649873] [<c13c1730>] ? rest_init+0x140/0x140
[ 4.649873] ---[ end trace d5c0cda419f9730c ]---
[ 4.649873] Mapped at:
[ 4.649873] [<c1210c44>] debug_dma_map_page+0x64/0x140
[ 4.649873] [<c128af55>] blogic_qcmd_lck+0x485/0x550
[ 4.649873] [<c128b052>] blogic_qcmd+0x32/0x50
[ 4.649873] [<c127c2f4>] scsi_dispatch_cmd+0x174/0x1f0
[ 4.649873] [<c1282e54>] scsi_request_fn+0x314/0x3a0
[ 4.664545] scsi 0:0:0:0: Direct-Access VMware, VMware Virtual S 1.0 PQ: 0 ANSI: 2
[ 4.692725] sd 0:0:0:0: [sda] 83886080 512-byte logical blocks: (42.9 GB/40.0 GiB)
[ 4.695383] sd 0:0:0:0: [sda] Write Protect is off
[ 4.697271] sd 0:0:0:0: Attached scsi generic sg0 type 0
----------
$ addr2line -e vmlinux c128b052
drivers/scsi/BusLogic.c:3231 => blogic_qcmd_lck
$ addr2line -e vmlinux c128af55
include/asm-generic/pci-dma-compat.h:31 => pci_map_single
The location seems to be pci_map_single() in blogic_qcmd_lck().
memcpy(ccb->cdb, cdb, cdblen);
ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
ccb->sensedata = pci_map_single(adapter->pci_device,
command->sense_buffer, ccb->sense_datalen,
PCI_DMA_FROMDEVICE);
ccb->command = command;
command->scsi_done = comp_cb;
Regards.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BusLogic] DMA-API: device driver failed to check map error
2013-09-10 6:03 [BusLogic] DMA-API: device driver failed to check map error Tetsuo Handa
@ 2013-09-10 12:36 ` James Bottomley
2013-09-12 16:53 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re: [BusLogic] DMA-API: device driver failed to check map error) Khalid Aziz
0 siblings, 1 reply; 7+ messages in thread
From: James Bottomley @ 2013-09-10 12:36 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: khalid, linux-kernel, linux-scsi
Add missing cc of linux-scsi
On Tue, 2013-09-10 at 15:03 +0900, Tetsuo Handa wrote:
> Hello.
>
> I got below warning on current linux.git .
>
> ----------
> [ 2.612237] scsi: ***** BusLogic SCSI Driver Version 2.1.16 of 18 July 2002 *****
> [ 2.613067] scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>
> [ 2.630942] scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> [ 2.633063] scsi0: Firmware Version: 5.07B, I/O Address: 0x10C0, IRQ Channel: 17/Level
> [ 2.633125] scsi0: PCI Bus: 0, Device: 16, Address: 0xD8800000, Host Adapter SCSI ID: 7
> [ 2.633188] scsi0: Parity Checking: Enabled, Extended Translation: Enabled
> [ 2.633250] scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> [ 2.635721] scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> [ 2.637054] scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> [ 2.637116] scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> [ 2.637179] scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> [ 2.639812] scsi0: *** BusLogic BT-958 Initialized Successfully ***
> [ 4.635828] scsi0 : BusLogic BT-958
> [ 4.641883] [sched_delayed] sched: RT throttling activated
> [ 4.647510] ------------[ cut here ]------------
> [ 4.647573] WARNING: CPU: 1 PID: 1 at lib/dma-debug.c:937 check_unmap+0x777/0x7f0()
> [ 4.648851] pci 0000:00:10.0: DMA-API: device driver failed to check map error[device address=0x00000000349cddc0] [size=96 bytes] [mapped as single]
> [ 4.649873] Modules linked in:
> [ 4.649873] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.11.0-08716-g26b0332-dirty #8
> [ 4.649873] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 08/15/2008
> [ 4.649873] 000003a9 00000000 f64b7de4 c11e8096 f64b7df0 c11e80dd c1210437 f64b7e1c
> [ 4.649873] c10421b9 c1519440 f64b7e4c 00000001 c1518220 000003a9 c1210437 f64b7e2c
> [ 4.649873] 00000001 f64b7ea4 f64b7e38 c1042211 00000009 f64b7e2c c1519440 f64b7e4c
> [ 4.649873] Call Trace:
> [ 4.649873] [<c11e8096>] __dump_stack+0x16/0x20
> [ 4.649873] [<c11e80dd>] dump_stack+0x3d/0x60
> [ 4.649873] [<c1210437>] ? check_unmap+0x777/0x7f0
> [ 4.649873] [<c10421b9>] warn_slowpath_common+0x79/0xa0
> [ 4.649873] [<c1210437>] ? check_unmap+0x777/0x7f0
> [ 4.649873] [<c1042211>] warn_slowpath_fmt+0x31/0x40
> [ 4.649873] [<c1210437>] check_unmap+0x777/0x7f0
> [ 4.649873] [<c1210e18>] debug_dma_unmap_page+0x78/0x90
> [ 4.649873] [<c1289703>] blogic_dealloc_ccb+0x83/0xb0
> [ 4.649873] [<c128a847>] blogic_process_ccbs+0x3c7/0x3f0
> [ 4.649873] [<c128a423>] ? blogic_scan_inbox+0x43/0xa0
> [ 4.649873] [<c128a8f1>] blogic_inthandler+0x81/0x150
> [ 4.649873] [<c10a9b4e>] ? handle_irq_event+0x2e/0x60
> [ 4.649873] [<c10a9a38>] handle_irq_event_percpu+0x38/0x120
> [ 4.649873] [<c10a9b4e>] ? handle_irq_event+0x2e/0x60
> [ 4.649873] [<c10a9b57>] handle_irq_event+0x37/0x60
> [ 4.649873] [<c10ac4d0>] ? handle_level_irq+0xb0/0xb0
> [ 4.649873] [<c10ac557>] handle_fasteoi_irq+0x87/0xc0
> [ 4.649873] <IRQ> [<c1003fdc>] ? do_IRQ+0x3c/0xb0
> [ 4.649873] [<c10948ca>] ? mark_held_locks+0xca/0x100
> [ 4.649873] [<c13c9111>] ? common_interrupt+0x31/0x36
> [ 4.649873] [<c13c77e7>] ? _raw_spin_unlock_irqrestore+0x47/0x60
> [ 4.649873] [<c128b05e>] ? blogic_qcmd+0x3e/0x50
> [ 4.649873] [<c127c2f4>] ? scsi_dispatch_cmd+0x174/0x1f0
> [ 4.649873] [<c1094b2b>] ? trace_hardirqs_on+0xb/0x10
> [ 4.649873] [<c1282e54>] ? scsi_request_fn+0x314/0x3a0
> [ 4.649873] [<c11d364b>] ? __blk_run_queue+0x2b/0x40
> [ 4.649873] [<c11d9c42>] ? blk_execute_rq_nowait+0xa2/0xd0
> [ 4.649873] [<c11d9b70>] ? blk_rq_map_kern+0x130/0x130
> [ 4.649873] [<c11d9cfa>] ? blk_execute_rq+0x8a/0xf0
> [ 4.649873] [<c11d9b70>] ? blk_rq_map_kern+0x130/0x130
> [ 4.649873] [<c11d9f4e>] ? blk_recount_segments+0x1e/0x40
> [ 4.649873] [<c11d0000>] ? aes_encrypt+0xe40/0x1450
> [ 4.649873] [<c11d9b4f>] ? blk_rq_map_kern+0x10f/0x130
> [ 4.649873] [<c1281834>] ? scsi_execute+0xe4/0x150
> [ 4.649873] [<c128190e>] ? scsi_execute_req_flags+0x6e/0xa0
> [ 4.649873] [<c1284972>] ? scsi_probe_lun+0x112/0x300
> [ 4.649873] [<c128512d>] ? scsi_probe_and_add_lun+0x10d/0x2f0
> [ 4.649873] [<c1284358>] ? scsi_alloc_sdev+0x248/0x2b0
> [ 4.649873] [<c128514d>] ? scsi_probe_and_add_lun+0x12d/0x2f0
> [ 4.649873] [<c1259340>] ? anon_transport_class_unregister+0x30/0x30
> [ 4.649873] [<c12846c8>] ? scsi_alloc_target+0x1c8/0x220
> [ 4.649873] [<c1285b00>] ? __scsi_scan_target+0xa0/0xf0
> [ 4.649873] [<c1285c4a>] ? scsi_scan_channel+0x5a/0x90
> [ 4.649873] [<c1285d79>] ? scsi_scan_host_selected+0xf9/0x140
> [ 4.649873] [<c12860c9>] ? do_scsi_scan_host+0x69/0x80
> [ 4.649873] [<c1286130>] ? scsi_scan_host+0x30/0x50
> [ 4.649873] [<c15f6d1c>] ? blogic_init+0x32c/0x3b0
> [ 4.649873] [<c15f69f0>] ? blogic_inithoststruct+0x80/0x80
> [ 4.649873] [<c1000472>] ? do_one_initcall+0x32/0xd0
> [ 4.649873] [<c105fa60>] ? parse_one+0xc0/0xe0
> [ 4.649873] [<c105fc0a>] ? parse_args+0x7a/0x170
> [ 4.649873] [<c15cb410>] ? loglevel+0x30/0x30
> [ 4.649873] [<c15cbb6a>] ? do_initcall_level+0x7a/0x90
> [ 4.649873] [<c15cb410>] ? loglevel+0x30/0x30
> [ 4.649873] [<c15cbb98>] ? do_initcalls+0x18/0x20
> [ 4.649873] [<c15cbbc8>] ? do_basic_setup+0x28/0x30
> [ 4.649873] [<c15cbc7f>] ? kernel_init_freeable+0x5f/0xf0
> [ 4.649873] [<c13c173b>] ? kernel_init+0xb/0xe0
> [ 4.649873] [<c13c8c18>] ? ret_from_kernel_thread+0x1c/0x2c
> [ 4.649873] [<c13c1730>] ? rest_init+0x140/0x140
> [ 4.649873] ---[ end trace d5c0cda419f9730c ]---
> [ 4.649873] Mapped at:
> [ 4.649873] [<c1210c44>] debug_dma_map_page+0x64/0x140
> [ 4.649873] [<c128af55>] blogic_qcmd_lck+0x485/0x550
> [ 4.649873] [<c128b052>] blogic_qcmd+0x32/0x50
> [ 4.649873] [<c127c2f4>] scsi_dispatch_cmd+0x174/0x1f0
> [ 4.649873] [<c1282e54>] scsi_request_fn+0x314/0x3a0
> [ 4.664545] scsi 0:0:0:0: Direct-Access VMware, VMware Virtual S 1.0 PQ: 0 ANSI: 2
> [ 4.692725] sd 0:0:0:0: [sda] 83886080 512-byte logical blocks: (42.9 GB/40.0 GiB)
> [ 4.695383] sd 0:0:0:0: [sda] Write Protect is off
> [ 4.697271] sd 0:0:0:0: Attached scsi generic sg0 type 0
> ----------
>
> $ addr2line -e vmlinux c128b052
> drivers/scsi/BusLogic.c:3231 => blogic_qcmd_lck
>
> $ addr2line -e vmlinux c128af55
> include/asm-generic/pci-dma-compat.h:31 => pci_map_single
>
> The location seems to be pci_map_single() in blogic_qcmd_lck().
>
> memcpy(ccb->cdb, cdb, cdblen);
> ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
> ccb->sensedata = pci_map_single(adapter->pci_device,
> command->sense_buffer, ccb->sense_datalen,
> PCI_DMA_FROMDEVICE);
> ccb->command = command;
> command->scsi_done = comp_cb;
>
> Regards.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re: [BusLogic] DMA-API: device driver failed to check map error)
2013-09-10 12:36 ` James Bottomley
@ 2013-09-12 16:53 ` Khalid Aziz
2013-09-13 15:42 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was " Tetsuo Handa
0 siblings, 1 reply; 7+ messages in thread
From: Khalid Aziz @ 2013-09-12 16:53 UTC (permalink / raw)
To: James Bottomley; +Cc: Tetsuo Handa, linux-kernel, linux-scsi
Added check for DMA mapping errors for request sense data
buffer. Checking for mapping error can avoid potential wild
writes. This patch was prompted by the warning from
dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.
Signed-off-by: Khalid Aziz <khalid.aziz@oracle.com>
---
drivers/scsi/BusLogic.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index feab3a5..90cf1aa 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -26,8 +26,8 @@
*/
-#define blogic_drvr_version "2.1.16"
-#define blogic_drvr_date "18 July 2002"
+#define blogic_drvr_version "2.1.17"
+#define blogic_drvr_date "12 September 2013"
#include <linux/module.h>
#include <linux/init.h>
@@ -311,12 +311,13 @@ static struct blogic_ccb *blogic_alloc_ccb(struct blogic_adapter *adapter)
caller.
*/
-static void blogic_dealloc_ccb(struct blogic_ccb *ccb)
+static void blogic_dealloc_ccb(struct blogic_ccb *ccb, int dma_unmap)
{
struct blogic_adapter *adapter = ccb->adapter;
scsi_dma_unmap(ccb->command);
- pci_unmap_single(adapter->pci_device, ccb->sensedata,
+ if (dma_unmap)
+ pci_unmap_single(adapter->pci_device, ccb->sensedata,
ccb->sense_datalen, PCI_DMA_FROMDEVICE);
ccb->command = NULL;
@@ -2762,8 +2763,8 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
/*
Place CCB back on the Host Adapter's free list.
*/
- blogic_dealloc_ccb(ccb);
-#if 0 /* this needs to be redone different for new EH */
+ blogic_dealloc_ccb(ccb, 1);
+#if 0 /* this needs to be redone different for new EH */
/*
Bus Device Reset CCBs have the command field
non-NULL only when a Bus Device Reset was requested
@@ -2791,7 +2792,7 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
if (ccb->status == BLOGIC_CCB_RESET &&
ccb->tgt_id == tgt_id) {
command = ccb->command;
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
adapter->active_cmds[tgt_id]--;
command->result = DID_RESET << 16;
command->scsi_done(command);
@@ -2862,7 +2863,7 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
/*
Place CCB back on the Host Adapter's free list.
*/
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
/*
Call the SCSI Command Completion Routine.
*/
@@ -3034,6 +3035,7 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
int buflen = scsi_bufflen(command);
int count;
struct blogic_ccb *ccb;
+ dma_addr_t sense_buf;
/*
SCSI REQUEST_SENSE commands will be executed automatically by the
@@ -3179,9 +3181,17 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
}
memcpy(ccb->cdb, cdb, cdblen);
ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
- ccb->sensedata = pci_map_single(adapter->pci_device,
+ sense_buf = pci_map_single(adapter->pci_device,
command->sense_buffer, ccb->sense_datalen,
PCI_DMA_FROMDEVICE);
+ if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
+ blogic_err("DMA mapping for sense data buffer failed\n",
+ adapter);
+ scsi_dma_map(command);
+ blogic_dealloc_ccb(ccb, 0);
+ return SCSI_MLQUEUE_HOST_BUSY;
+ }
+ ccb->sensedata = sense_buf;
ccb->command = command;
command->scsi_done = comp_cb;
if (blogic_multimaster_type(adapter)) {
@@ -3203,7 +3213,7 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
ccb)) {
blogic_warn("Still unable to write Outgoing Mailbox - " "Host Adapter Dead?\n", adapter);
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
command->result = DID_ERROR << 16;
command->scsi_done(command);
}
@@ -3337,7 +3347,7 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
for (ccb = adapter->all_ccbs; ccb != NULL; ccb = ccb->next_all)
if (ccb->status == BLOGIC_CCB_ACTIVE)
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
/*
* Wait a few seconds between the Host Adapter Hard Reset which
* initiates a SCSI Bus Reset and issuing any SCSI Commands. Some
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)
2013-09-12 16:53 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re: [BusLogic] DMA-API: device driver failed to check map error) Khalid Aziz
@ 2013-09-13 15:42 ` Tetsuo Handa
2013-09-13 15:55 ` Khalid Aziz
0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2013-09-13 15:42 UTC (permalink / raw)
To: khalid.aziz; +Cc: jbottomley, linux-kernel, linux-scsi
Khalid Aziz wrote:
> Added check for DMA mapping errors for request sense data
> buffer. Checking for mapping error can avoid potential wild
> writes. This patch was prompted by the warning from
> dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.
This patch fixes CONFIG_DMA_API_DEBUG warning.
But excuse me, is this error path correct?
> @@ -309,16 +309,17 @@ static struct blogic_ccb *blogic_alloc_ccb(struct blogic_adapter *adapter)
> blogic_dealloc_ccb deallocates a CCB, returning it to the Host Adapter's
> free list. The Host Adapter's Lock should already have been acquired by the
> caller.
> */
>
> -static void blogic_dealloc_ccb(struct blogic_ccb *ccb)
> +static void blogic_dealloc_ccb(struct blogic_ccb *ccb, int dma_unmap)
> {
> struct blogic_adapter *adapter = ccb->adapter;
>
> scsi_dma_unmap(ccb->command);
blogic_dealloc_ccb() uses "ccb->command". But
> - pci_unmap_single(adapter->pci_device, ccb->sensedata,
> + if (dma_unmap)
> + pci_unmap_single(adapter->pci_device, ccb->sensedata,
> ccb->sense_datalen, PCI_DMA_FROMDEVICE);
>
> ccb->command = NULL;
> ccb->status = BLOGIC_CCB_FREE;
> ccb->next = adapter->free_ccbs;
> @@ -3177,13 +3179,21 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
> ccb->legacy_tag = queuetag;
> }
> }
> memcpy(ccb->cdb, cdb, cdblen);
> ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
> - ccb->sensedata = pci_map_single(adapter->pci_device,
> + sense_buf = pci_map_single(adapter->pci_device,
> command->sense_buffer, ccb->sense_datalen,
> PCI_DMA_FROMDEVICE);
> + if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
> + blogic_err("DMA mapping for sense data buffer failed\n",
> + adapter);
> + scsi_dma_map(command);
> + blogic_dealloc_ccb(ccb, 0);
when was "ccb->command = command;" called before calling blogic_dealloc_ccb()?
> + return SCSI_MLQUEUE_HOST_BUSY;
> + }
> + ccb->sensedata = sense_buf;
> ccb->command = command;
> command->scsi_done = comp_cb;
> if (blogic_multimaster_type(adapter)) {
> /*
> Place the CCB in an Outgoing Mailbox. The higher levels
Also, what happens if "scsi_dma_map(command);" returned -ENOMEM ?
If you are calling scsi_dma_map() because blogic_dealloc_ccb() calls
scsi_dma_unmap(), why can't we do like
{
struct blogic_adapter *adapter = ccb->adapter;
ccb->command = NULL;
ccb->status = BLOGIC_CCB_FREE;
ccb->next = adapter->free_ccbs;
adapter->free_ccbs = ccb;
}
instead of
scsi_dma_map(command);
blogic_dealloc_ccb(ccb);
?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)
2013-09-13 15:42 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was " Tetsuo Handa
@ 2013-09-13 15:55 ` Khalid Aziz
2013-09-13 19:44 ` [PATCH v2] " Khalid Aziz
0 siblings, 1 reply; 7+ messages in thread
From: Khalid Aziz @ 2013-09-13 15:55 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: jbottomley, linux-kernel, linux-scsi
On 09/13/2013 09:42 AM, Tetsuo Handa wrote:
> Khalid Aziz wrote:
>> Added check for DMA mapping errors for request sense data
>> buffer. Checking for mapping error can avoid potential wild
>> writes. This patch was prompted by the warning from
>> dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.
>
> This patch fixes CONFIG_DMA_API_DEBUG warning.
> But excuse me, is this error path correct?
>
>> @@ -309,16 +309,17 @@ static struct blogic_ccb *blogic_alloc_ccb(struct blogic_adapter *adapter)
>> blogic_dealloc_ccb deallocates a CCB, returning it to the Host Adapter's
>> free list. The Host Adapter's Lock should already have been acquired by the
>> caller.
>> */
>>
>> -static void blogic_dealloc_ccb(struct blogic_ccb *ccb)
>> +static void blogic_dealloc_ccb(struct blogic_ccb *ccb, int dma_unmap)
>> {
>> struct blogic_adapter *adapter = ccb->adapter;
>>
>> scsi_dma_unmap(ccb->command);
>
> blogic_dealloc_ccb() uses "ccb->command". But
>
>> - pci_unmap_single(adapter->pci_device, ccb->sensedata,
>> + if (dma_unmap)
>> + pci_unmap_single(adapter->pci_device, ccb->sensedata,
>> ccb->sense_datalen, PCI_DMA_FROMDEVICE);
>>
>> ccb->command = NULL;
>> ccb->status = BLOGIC_CCB_FREE;
>> ccb->next = adapter->free_ccbs;
>> @@ -3177,13 +3179,21 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
>> ccb->legacy_tag = queuetag;
>> }
>> }
>> memcpy(ccb->cdb, cdb, cdblen);
>> ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
>> - ccb->sensedata = pci_map_single(adapter->pci_device,
>> + sense_buf = pci_map_single(adapter->pci_device,
>> command->sense_buffer, ccb->sense_datalen,
>> PCI_DMA_FROMDEVICE);
>> + if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
>> + blogic_err("DMA mapping for sense data buffer failed\n",
>> + adapter);
>> + scsi_dma_map(command);
>> + blogic_dealloc_ccb(ccb, 0);
>
> when was "ccb->command = command;" called before calling blogic_dealloc_ccb()?
>
>> + return SCSI_MLQUEUE_HOST_BUSY;
>> + }
>> + ccb->sensedata = sense_buf;
>> ccb->command = command;
>> command->scsi_done = comp_cb;
>> if (blogic_multimaster_type(adapter)) {
>> /*
>> Place the CCB in an Outgoing Mailbox. The higher levels
>
> Also, what happens if "scsi_dma_map(command);" returned -ENOMEM ?
> If you are calling scsi_dma_map() because blogic_dealloc_ccb() calls
> scsi_dma_unmap(), why can't we do like
>
> {
> struct blogic_adapter *adapter = ccb->adapter;
> ccb->command = NULL;
> ccb->status = BLOGIC_CCB_FREE;
> ccb->next = adapter->free_ccbs;
> adapter->free_ccbs = ccb;
> }
>
> instead of
>
> scsi_dma_map(command);
> blogic_dealloc_ccb(ccb);
>
> ?
>
That is a typo. I was going to call just scsi_dma_unmap(), had copied
down the previous down the previous scsi_dma_map() line, read the code
further and decided I needed to call blogic_dealloc_ccb() instead so we
don't leak CCBs, added the call to blogic_dealloc_ccb() and forgot to
delete the previously added and unfinished line. scsi_dma_map() had
already been called earlier which is why scsi_dma_unmap() needs to be
called which is taken care of by blogic_dealloc_ccb().
I need to delete the call to scsi_dma_map() which was not supposed to be
there and move "ccb->command = command;" up before the call to
dma_mapping_error(). I will send out an updated patch.
Good catch. Thanks!
--
Khalid
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)
2013-09-13 15:55 ` Khalid Aziz
@ 2013-09-13 19:44 ` Khalid Aziz
2013-09-14 3:34 ` [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (wasRe:[BusLogic] " Tetsuo Handa
0 siblings, 1 reply; 7+ messages in thread
From: Khalid Aziz @ 2013-09-13 19:44 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: jbottomley, linux-kernel, linux-scsi
Added check for DMA mapping errors for request sense data
buffer. Checking for mapping error can avoid potential wild
writes. This patch was prompted by the warning from
dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.
Signed-off-by: Khalid Aziz <khalid.aziz@oracle.com>
---
Changelog:
v1 - Fixed a typo and addressed a potential NULL pointer
dereference issue
drivers/scsi/BusLogic.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index feab3a5..e63b788 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -26,8 +26,8 @@
*/
-#define blogic_drvr_version "2.1.16"
-#define blogic_drvr_date "18 July 2002"
+#define blogic_drvr_version "2.1.17"
+#define blogic_drvr_date "12 September 2013"
#include <linux/module.h>
#include <linux/init.h>
@@ -311,12 +311,14 @@ static struct blogic_ccb *blogic_alloc_ccb(struct blogic_adapter *adapter)
caller.
*/
-static void blogic_dealloc_ccb(struct blogic_ccb *ccb)
+static void blogic_dealloc_ccb(struct blogic_ccb *ccb, int dma_unmap)
{
struct blogic_adapter *adapter = ccb->adapter;
- scsi_dma_unmap(ccb->command);
- pci_unmap_single(adapter->pci_device, ccb->sensedata,
+ if (ccb->command != NULL)
+ scsi_dma_unmap(ccb->command);
+ if (dma_unmap)
+ pci_unmap_single(adapter->pci_device, ccb->sensedata,
ccb->sense_datalen, PCI_DMA_FROMDEVICE);
ccb->command = NULL;
@@ -2762,8 +2764,8 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
/*
Place CCB back on the Host Adapter's free list.
*/
- blogic_dealloc_ccb(ccb);
-#if 0 /* this needs to be redone different for new EH */
+ blogic_dealloc_ccb(ccb, 1);
+#if 0 /* this needs to be redone different for new EH */
/*
Bus Device Reset CCBs have the command field
non-NULL only when a Bus Device Reset was requested
@@ -2791,7 +2793,7 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
if (ccb->status == BLOGIC_CCB_RESET &&
ccb->tgt_id == tgt_id) {
command = ccb->command;
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
adapter->active_cmds[tgt_id]--;
command->result = DID_RESET << 16;
command->scsi_done(command);
@@ -2862,7 +2864,7 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
/*
Place CCB back on the Host Adapter's free list.
*/
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
/*
Call the SCSI Command Completion Routine.
*/
@@ -3034,6 +3036,7 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
int buflen = scsi_bufflen(command);
int count;
struct blogic_ccb *ccb;
+ dma_addr_t sense_buf;
/*
SCSI REQUEST_SENSE commands will be executed automatically by the
@@ -3179,10 +3182,17 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
}
memcpy(ccb->cdb, cdb, cdblen);
ccb->sense_datalen = SCSI_SENSE_BUFFERSIZE;
- ccb->sensedata = pci_map_single(adapter->pci_device,
+ ccb->command = command;
+ sense_buf = pci_map_single(adapter->pci_device,
command->sense_buffer, ccb->sense_datalen,
PCI_DMA_FROMDEVICE);
- ccb->command = command;
+ if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
+ blogic_err("DMA mapping for sense data buffer failed\n",
+ adapter);
+ blogic_dealloc_ccb(ccb, 0);
+ return SCSI_MLQUEUE_HOST_BUSY;
+ }
+ ccb->sensedata = sense_buf;
command->scsi_done = comp_cb;
if (blogic_multimaster_type(adapter)) {
/*
@@ -3203,7 +3213,7 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
ccb)) {
blogic_warn("Still unable to write Outgoing Mailbox - " "Host Adapter Dead?\n", adapter);
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
command->result = DID_ERROR << 16;
command->scsi_done(command);
}
@@ -3337,7 +3347,7 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
for (ccb = adapter->all_ccbs; ccb != NULL; ccb = ccb->next_all)
if (ccb->status == BLOGIC_CCB_ACTIVE)
- blogic_dealloc_ccb(ccb);
+ blogic_dealloc_ccb(ccb, 1);
/*
* Wait a few seconds between the Host Adapter Hard Reset which
* initiates a SCSI Bus Reset and issuing any SCSI Commands. Some
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (wasRe:[BusLogic] DMA-API: device driver failed to check map error)
2013-09-13 19:44 ` [PATCH v2] " Khalid Aziz
@ 2013-09-14 3:34 ` Tetsuo Handa
0 siblings, 0 replies; 7+ messages in thread
From: Tetsuo Handa @ 2013-09-14 3:34 UTC (permalink / raw)
To: khalid.aziz; +Cc: jbottomley, linux-kernel, linux-scsi
Khalid Aziz wrote:
> Added check for DMA mapping errors for request sense data
> buffer. Checking for mapping error can avoid potential wild
> writes. This patch was prompted by the warning from
> dma_unmap when kernel is compiled with CONFIG_DMA_API_DEBUG.
This patch looks OK and works OK to me.
Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-14 3:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-10 6:03 [BusLogic] DMA-API: device driver failed to check map error Tetsuo Handa
2013-09-10 12:36 ` James Bottomley
2013-09-12 16:53 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re: [BusLogic] DMA-API: device driver failed to check map error) Khalid Aziz
2013-09-13 15:42 ` [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was " Tetsuo Handa
2013-09-13 15:55 ` Khalid Aziz
2013-09-13 19:44 ` [PATCH v2] " Khalid Aziz
2013-09-14 3:34 ` [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (wasRe:[BusLogic] " Tetsuo Handa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.