2013-09-10 06:04:14

by Tetsuo Handa

[permalink] [raw]
Subject: [BusLogic] DMA-API: device driver failed to check map error

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 <[email protected]>
[ 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.


2013-09-10 12:36:52

by James Bottomley

[permalink] [raw]
Subject: Re: [BusLogic] DMA-API: device driver failed to check map error

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 <[email protected]>
> [ 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.

2013-09-12 16:53:37

by Khalid Aziz

[permalink] [raw]
Subject: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re: [BusLogic] DMA-API: device driver failed to check map error)

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 <[email protected]>
---
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


2013-09-13 15:42:44

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)

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);

?

2013-09-13 15:55:27

by Khalid Aziz

[permalink] [raw]
Subject: Re: [PATCH] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)

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

2013-09-13 19:51:17

by Khalid Aziz

[permalink] [raw]
Subject: [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (was Re:[BusLogic] DMA-API: device driver failed to check map error)

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 <[email protected]>
---
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


2013-09-14 03:34:41

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH v2] SCSI: buslogic: Added check for DMA mapping errors (wasRe:[BusLogic] DMA-API: device driver failed to check map error)

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.