* [PATCH 1/3] ipr: fix addition of abort command to HRRQ free queue
2013-03-14 18:52 [PATCH 0/3] Fixed several ipr bugs when bring up an ipr adapter wenxiong
@ 2013-03-14 18:52 ` wenxiong
2013-03-21 13:52 ` Brian King
2013-03-14 18:52 ` [PATCH 2/3] ipr: dlpar failed when adding an adapter back wenxiong
2013-03-14 18:52 ` [PATCH 3/3] ipr: Avoid target_destroy accessing memory after it was freed wenxiong
2 siblings, 1 reply; 6+ messages in thread
From: wenxiong @ 2013-03-14 18:52 UTC (permalink / raw)
To: James.Bottomley; +Cc: linux-scsi, brking, klebers, Wen Xiong
[-- Attachment #1: ipr_fix_hrrq_free_queue --]
[-- Type: text/plain, Size: 941 bytes --]
From: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
The abort command issued by ipr_cancel_op() is being added to the wrong
HRRQ free queue after the command returns. Fix it by using the HRRQ
pointer in the ipr command struct itself.
Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
Signed-off-by: Wen Xiong <wenxiong@linux.vnet.ibm.com>
---
drivers/scsi/ipr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c 2013-03-14 11:49:29.819030052 -0500
+++ b/drivers/scsi/ipr.c 2013-03-14 13:15:46.289276262 -0500
@@ -5148,7 +5148,7 @@ static int ipr_cancel_op(struct scsi_cmn
ipr_trace;
}
- list_add_tail(&ipr_cmd->queue, &hrrq->hrrq_free_q);
+ list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_free_q);
if (!ipr_is_naca_model(res))
res->needs_sync_complete = 1;
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] ipr: dlpar failed when adding an adapter back
2013-03-14 18:52 [PATCH 0/3] Fixed several ipr bugs when bring up an ipr adapter wenxiong
2013-03-14 18:52 ` [PATCH 1/3] ipr: fix addition of abort command to HRRQ free queue wenxiong
@ 2013-03-14 18:52 ` wenxiong
2013-03-14 18:52 ` [PATCH 3/3] ipr: Avoid target_destroy accessing memory after it was freed wenxiong
2 siblings, 0 replies; 6+ messages in thread
From: wenxiong @ 2013-03-14 18:52 UTC (permalink / raw)
To: James.Bottomley; +Cc: linux-scsi, brking, klebers, Wen Xiong
[-- Attachment #1: reinit_used_res_q --]
[-- Type: text/plain, Size: 883 bytes --]
Reinitialize resource queue prior to freeing resource entries to ensure they
are not referenced. This fixes an issue with target_destoy accessing memory
after it was freed.
Signed-off-by: Wen Xiong <wenxiong@linux.vnet.ibm.com>
---
drivers/scsi/ipr.c | 1 +
1 file changed, 1 insertion(+)
Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c 2013-03-14 13:15:46.289276262 -0500
+++ b/drivers/scsi/ipr.c 2013-03-14 13:16:03.398966326 -0500
@@ -9728,6 +9728,7 @@ static void __ipr_remove(struct pci_dev
spin_unlock_irqrestore(ioa_cfg->host->host_lock, host_lock_flags);
wait_event(ioa_cfg->reset_wait_q, !ioa_cfg->in_reset_reload);
flush_work(&ioa_cfg->work_q);
+ INIT_LIST_HEAD(&ioa_cfg->used_res_q);
spin_lock_irqsave(ioa_cfg->host->host_lock, host_lock_flags);
spin_lock(&ipr_driver_lock);
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ipr: Avoid target_destroy accessing memory after it was freed
2013-03-14 18:52 [PATCH 0/3] Fixed several ipr bugs when bring up an ipr adapter wenxiong
2013-03-14 18:52 ` [PATCH 1/3] ipr: fix addition of abort command to HRRQ free queue wenxiong
2013-03-14 18:52 ` [PATCH 2/3] ipr: dlpar failed when adding an adapter back wenxiong
@ 2013-03-14 18:52 ` wenxiong
2013-05-09 16:32 ` wenxiong
2 siblings, 1 reply; 6+ messages in thread
From: wenxiong @ 2013-03-14 18:52 UTC (permalink / raw)
To: James.Bottomley; +Cc: linux-scsi, brking, klebers, Wen Xiong
[-- Attachment #1: define_ids_as_long --]
[-- Type: text/plain, Size: 2204 bytes --]
Defined target_ids,array_ids and vsets_ids as unsigned long to avoid
target_destroy accessing memory after it was freed.
Signed-off-by: Wen Xiong <wenxiong@linux.vnet.ibm.com>
---
drivers/scsi/ipr.c | 16 ----------------
drivers/scsi/ipr.h | 6 +++---
2 files changed, 3 insertions(+), 19 deletions(-)
Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c 2013-03-14 13:16:03.398966326 -0500
+++ b/drivers/scsi/ipr.c 2013-03-14 13:17:04.828022126 -0500
@@ -8972,19 +8972,6 @@ static int ipr_alloc_mem(struct ipr_ioa_
if (!ioa_cfg->res_entries)
goto out;
- if (ioa_cfg->sis64) {
- ioa_cfg->target_ids = kzalloc(sizeof(unsigned long) *
- BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
- ioa_cfg->array_ids = kzalloc(sizeof(unsigned long) *
- BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
- ioa_cfg->vset_ids = kzalloc(sizeof(unsigned long) *
- BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
-
- if (!ioa_cfg->target_ids || !ioa_cfg->array_ids
- || !ioa_cfg->vset_ids)
- goto out_free_res_entries;
- }
-
for (i = 0; i < ioa_cfg->max_devs_supported; i++) {
list_add_tail(&ioa_cfg->res_entries[i].queue, &ioa_cfg->free_res_q);
ioa_cfg->res_entries[i].ioa_cfg = ioa_cfg;
@@ -9081,9 +9068,6 @@ out_free_vpd_cbs:
ioa_cfg->vpd_cbs, ioa_cfg->vpd_cbs_dma);
out_free_res_entries:
kfree(ioa_cfg->res_entries);
- kfree(ioa_cfg->target_ids);
- kfree(ioa_cfg->array_ids);
- kfree(ioa_cfg->vset_ids);
goto out;
}
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h 2013-03-14 11:49:21.408965542 -0500
+++ b/drivers/scsi/ipr.h 2013-03-14 13:16:20.131452448 -0500
@@ -1440,9 +1440,9 @@ struct ipr_ioa_cfg {
/*
* Bitmaps for SIS64 generated target values
*/
- unsigned long *target_ids;
- unsigned long *array_ids;
- unsigned long *vset_ids;
+ unsigned long target_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
+ unsigned long array_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
+ unsigned long vset_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
u16 type; /* CCIN of the card */
--
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] ipr: Avoid target_destroy accessing memory after it was freed
2013-03-14 18:52 ` [PATCH 3/3] ipr: Avoid target_destroy accessing memory after it was freed wenxiong
@ 2013-05-09 16:32 ` wenxiong
0 siblings, 0 replies; 6+ messages in thread
From: wenxiong @ 2013-05-09 16:32 UTC (permalink / raw)
To: James.Bottomley, linux-scsi, brking, klebers
Hi James,
Patch 1 and Patch 2 merged into next-scsi and 3.9.0 on 4/6/13 but
Patch3 is missed in both of next-scsi
and 3.9.0 tree. Can you merge Patch 3 or I need to re-send the Patch3?
Thanks for your help!
Wendy
Quoting wenxiong@linux.vnet.ibm.com:
> Defined target_ids,array_ids and vsets_ids as unsigned long to avoid
> target_destroy accessing memory after it was freed.
>
>
> Signed-off-by: Wen Xiong <wenxiong@linux.vnet.ibm.com>
> ---
> drivers/scsi/ipr.c | 16 ----------------
> drivers/scsi/ipr.h | 6 +++---
> 2 files changed, 3 insertions(+), 19 deletions(-)
>
> Index: b/drivers/scsi/ipr.c
> ===================================================================
> --- a/drivers/scsi/ipr.c 2013-03-14 13:16:03.398966326 -0500
> +++ b/drivers/scsi/ipr.c 2013-03-14 13:17:04.828022126 -0500
> @@ -8972,19 +8972,6 @@ static int ipr_alloc_mem(struct ipr_ioa_
> if (!ioa_cfg->res_entries)
> goto out;
>
> - if (ioa_cfg->sis64) {
> - ioa_cfg->target_ids = kzalloc(sizeof(unsigned long) *
> - BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
> - ioa_cfg->array_ids = kzalloc(sizeof(unsigned long) *
> - BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
> - ioa_cfg->vset_ids = kzalloc(sizeof(unsigned long) *
> - BITS_TO_LONGS(ioa_cfg->max_devs_supported), GFP_KERNEL);
> -
> - if (!ioa_cfg->target_ids || !ioa_cfg->array_ids
> - || !ioa_cfg->vset_ids)
> - goto out_free_res_entries;
> - }
> -
> for (i = 0; i < ioa_cfg->max_devs_supported; i++) {
> list_add_tail(&ioa_cfg->res_entries[i].queue, &ioa_cfg->free_res_q);
> ioa_cfg->res_entries[i].ioa_cfg = ioa_cfg;
> @@ -9081,9 +9068,6 @@ out_free_vpd_cbs:
> ioa_cfg->vpd_cbs, ioa_cfg->vpd_cbs_dma);
> out_free_res_entries:
> kfree(ioa_cfg->res_entries);
> - kfree(ioa_cfg->target_ids);
> - kfree(ioa_cfg->array_ids);
> - kfree(ioa_cfg->vset_ids);
> goto out;
> }
>
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h 2013-03-14 11:49:21.408965542 -0500
> +++ b/drivers/scsi/ipr.h 2013-03-14 13:16:20.131452448 -0500
> @@ -1440,9 +1440,9 @@ struct ipr_ioa_cfg {
> /*
> * Bitmaps for SIS64 generated target values
> */
> - unsigned long *target_ids;
> - unsigned long *array_ids;
> - unsigned long *vset_ids;
> + unsigned long target_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
> + unsigned long array_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
> + unsigned long vset_ids[BITS_TO_LONGS(IPR_MAX_SIS64_DEVS)];
>
> u16 type; /* CCIN of the card */
>
>
> --
^ permalink raw reply [flat|nested] 6+ messages in thread