Re: [PATCH v2 06/10] qla2xxx: Fix crash due to null pointer access.

[Madhani, Himanshu]

On 12/23/16, 12:47 AM, "Bart Van Assche"  wrote:

>This patch needs a more detailed description. There are many changes in this
>patch. What changes are the changes that prevent the NULL pointer dereference?
>What changes (if any) were made as the result of code inspection?

We saw this stack trace on one test setup which never was reproduced.
This patch is result of code inspection only and we have not seen this being 
Reproduced or reported by customer. 

Re: [PATCH v2 06/10] qla2xxx: Fix crash due to null pointer access.

[Bart Van Assche]

On Wed, 2016-12-21 at 13:57 -0800, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> This patch fixes crash due to NULL pointer access.
> 
> Following stack trace will be seen.
> 
> [1469877.797315] Call Trace:
> [1469877.799940]  [] qla2x00_mem_alloc+0xb09/0x10c0 
> [qla2xxx]
> [1469877.806980]  [] qla2x00_probe_one+0x86a/0x1b50 
> [qla2xxx]
> [1469877.814013]  [] ? __pm_runtime_resume+0x51/0xa0
> [1469877.820265]  [] ? _raw_spin_lock_irqsave+0x25/0x90
> [1469877.826776]  [] ? _raw_spin_unlock_irqrestore+0x6d/0x80
> [1469877.833720]  [] ? preempt_count_sub+0xb1/0x100
> [1469877.839885]  [] ? _raw_spin_unlock_irqrestore+0x4c/0x80
> [1469877.846830]  [] local_pci_probe+0x4c/0xb0
> [1469877.852562]  [] ? preempt_count_sub+0xb1/0x100
> [1469877.858727]  [] pci_call_probe+0x89/0xb0

This patch needs a more detailed description. There are many changes in this
patch. What changes are the changes that prevent the NULL pointer dereference?
What changes (if any) were made as the result of code inspection?

Bart.

[PATCH v2 06/10] qla2xxx: Fix crash due to null pointer access.

[Himanshu Madhani]

From: Quinn Tran 

This patch fixes crash due to NULL pointer access.

Following stack trace will be seen.

[1469877.797315] Call Trace:
[1469877.799940]  [] qla2x00_mem_alloc+0xb09/0x10c0 [qla2xxx]
[1469877.806980]  [] qla2x00_probe_one+0x86a/0x1b50 [qla2xxx]
[1469877.814013]  [] ? __pm_runtime_resume+0x51/0xa0
[1469877.820265]  [] ? _raw_spin_lock_irqsave+0x25/0x90
[1469877.826776]  [] ? _raw_spin_unlock_irqrestore+0x6d/0x80
[1469877.833720]  [] ? preempt_count_sub+0xb1/0x100
[1469877.839885]  [] ? _raw_spin_unlock_irqrestore+0x4c/0x80
[1469877.846830]  [] local_pci_probe+0x4c/0xb0
[1469877.852562]  [] ? preempt_count_sub+0xb1/0x100
[1469877.858727]  [] pci_call_probe+0x89/0xb0

Cc: 
Signed-off-by: Quinn Tran 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_os.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 8521cfe..df57fb3 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3662,7 +3662,7 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, 
fc_port_t *fcport,
sizeof(struct ct6_dsd), 0,
SLAB_HWCACHE_ALIGN, NULL);
if (!ctx_cachep)
-   goto fail_free_gid_list;
+   goto fail_free_srb_mempool;
}
ha->ctx_mempool = mempool_create_slab_pool(SRB_MIN_REQ,
ctx_cachep);
@@ -3815,7 +3815,7 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, 
fc_port_t *fcport,
ha->loop_id_map = kzalloc(BITS_TO_LONGS(LOOPID_MAP_SIZE) * sizeof(long),
GFP_KERNEL);
if (!ha->loop_id_map)
-   goto fail_async_pd;
+   goto fail_loop_id_map;
else {
qla2x00_set_reserved_loop_ids(ha);
ql_dbg_pci(ql_dbg_init, ha->pdev, 0x0123,
@@ -3824,10 +3824,15 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, 
fc_port_t *fcport,
 
return 0;
 
+fail_loop_id_map:
+   dma_pool_free(ha->s_dma_pool, ha->async_pd, ha->async_pd_dma);
+   ha->async_pd = NULL;
 fail_async_pd:
dma_pool_free(ha->s_dma_pool, ha->ex_init_cb, ha->ex_init_cb_dma);
+   ha->ex_init_cb = NULL;
 fail_ex_init_cb:
kfree(ha->npiv_info);
+   ha->npiv_info = NULL;
 fail_npiv_info:
dma_free_coherent(>pdev->dev, ((*rsp)->length + 1) *
sizeof(response_t), (*rsp)->ring, (*rsp)->dma);
@@ -3851,6 +3856,14 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, 
fc_port_t *fcport,
dma_pool_free(ha->s_dma_pool, ha->ms_iocb, ha->ms_iocb_dma);
ha->ms_iocb = NULL;
ha->ms_iocb_dma = 0;
+
+   if (ha->sns_cmd) {
+   dma_free_coherent(>pdev->dev, sizeof(struct sns_cmd_pkt),
+   ha->sns_cmd, ha->sns_cmd_dma);
+   ha->sns_cmd_dma = 0;
+   ha->sns_cmd = NULL;
+   }
+
 fail_dma_pool:
if (IS_QLA82XX(ha) || ql2xenabledif) {
dma_pool_destroy(ha->fcp_cmnd_dma_pool);
@@ -3868,10 +3881,12 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, 
fc_port_t *fcport,
kfree(ha->nvram);
ha->nvram = NULL;
 fail_free_ctx_mempool:
-   mempool_destroy(ha->ctx_mempool);
+   if (ha->ctx_mempool)
+   mempool_destroy(ha->ctx_mempool);
ha->ctx_mempool = NULL;
 fail_free_srb_mempool:
-   mempool_destroy(ha->srb_mempool);
+   if (ha->srb_mempool)
+   mempool_destroy(ha->srb_mempool);
ha->srb_mempool = NULL;
 fail_free_gid_list:
dma_free_coherent(>pdev->dev, qla2x00_gid_list_size(ha),
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html