Re: [PATCH] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()
Sebastian Herbszt writes: > Johannes Thumshirn wrote: >> If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution >> continues normally and mp gets kfree()d. >> >> If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the >> error label where lpfc_mbuf_free() is called with mp->virt and mp->phys as >> function arguments. This is the use after free. Following the use after free >> mp >> gets kfree()d again which is a double free. > > A similar patch was posted by Colin Ian King on 2015-07-31 [1]. > > [1] http://marc.info/?l=linux-scsi&m=143835937206204&w=2 OK, Is it already in James' tree (haven't checked)? The problematic code was merged for 4.2-rc1 so if the fix (Collin's or mine I don't care) could go in while we're still in the rc phase, we could avoid all that stable circus. Thanks for digging this out. Byte, Johannes -- 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
Re: [patch] qla4xxx: remove some bogus casts
On 08/17/2015 07:36 AM, Dan Carpenter wrote: These casts are wrong and unnecessary. They annoy static checkers because they imply we are planning to write sizeof(long) bytes to a sizeof(u32) buffer which would corrupt memory. Hello Dan, Can you verify whether that patch is still needed after having applied the patch series I posted in July (and that was acknowledged by QLogic): http://thread.gmane.org/gmane.linux.scsi/103135 ? Thanks, Bart. -- 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
Re: [PATCH v6 1/3] cxlflash: Base error recovery support
Hi Brian,
Thanks for reviewing.
All good suggestions that I am fine with implementing. As these are fairly
minor,
would you be okay with these being made in a separate ‘fix' patch series?
-matt
> On Aug 17, 2015, at 9:38 AM, Brian King wrote:
>
> On 08/13/2015 09:47 PM, Matthew R. Ochs wrote:
>> --- a/drivers/scsi/cxlflash/common.h
>> +++ b/drivers/scsi/cxlflash/common.h
>> @@ -76,6 +76,12 @@ enum cxlflash_init_state {
>> INIT_STATE_SCSI
>> };
>>
>> +enum cxlflash_state {
>> +STATE_NORMAL, /* Normal running state, everything good */
>> +STATE_LIMBO,/* Limbo running state, trying to reset/recover */
>
> Might be more clear to call this STATE_RESETTING or STATE_RECOVERY
>
>> +STATE_FAILTERM /* Failed/terminating state, error out users/threads */
>> +};
>> +
>> /*
>> * Each context has its own set of resource handles that is visible
>> * only from that context.
>
>> @@ -105,7 +109,8 @@ struct cxlflash_cfg {
>>
>> wait_queue_head_t tmf_waitq;
>> bool tmf_active;
>> -u8 err_recovery_active:1;
>> +wait_queue_head_t limbo_waitq;
>
> How about reset_waitq instead?
>
>> +enum cxlflash_state state;
>> };
>>
>> struct afu_cmd {
>
>> @@ -455,9 +471,21 @@ static int cxlflash_eh_device_reset_handler(struct
>> scsi_cmnd *scp)
>> get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>> get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>>
>> -rcr = send_tmf(afu, scp, TMF_LUN_RESET);
>> -if (unlikely(rcr))
>> +switch (cfg->state) {
>> +case STATE_NORMAL:
>> +rcr = send_tmf(afu, scp, TMF_LUN_RESET);
>> +if (unlikely(rcr))
>> +rc = FAILED;
>> +break;
>> +case STATE_LIMBO:
>> +wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
>> +if (cfg->state == STATE_NORMAL)
>> +break;
>
> In this case you've been asked to do a LUN reset but didn't actually reset
> the LUN. I'd suggest
> restructuring this switch statement to send the LUN reset TMF in the case
> where you had
> to wait for an AFU reset to complete.
>
>> +/* fall through */
>> +default:
>> rc = FAILED;
>> +break;
>> +}
>>
>> pr_debug("%s: returning rc=%d\n", __func__, rc);
>> return rc;
>> @@ -487,11 +515,29 @@ static int cxlflash_eh_host_reset_handler(struct
>> scsi_cmnd *scp)
>> get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>> get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>>
>> -rcr = cxlflash_afu_reset(cfg);
>> -if (rcr == 0)
>> -rc = SUCCESS;
>> -else
>> +switch (cfg->state) {
>> +case STATE_NORMAL:
>> +cfg->state = STATE_LIMBO;
>> +scsi_block_requests(cfg->host);
>> +
>> +rcr = cxlflash_afu_reset(cfg);
>> +if (rcr) {
>> +rc = FAILED;
>> +cfg->state = STATE_FAILTERM;
>> +} else
>> +cfg->state = STATE_NORMAL;
>> +wake_up_all(&cfg->limbo_waitq);
>> +scsi_unblock_requests(cfg->host);
>
> The scsi_block_requests / scsi_unblock_requests is not necessary in this
> path, since SCSI EH
> will already be preventing any new commands being issued via queuecommand.
>
>> +break;
>> +case STATE_LIMBO:
>> +wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
>> +if (cfg->state == STATE_NORMAL)
>> +break;
>> +/* fall through */
>> +default:
>> rc = FAILED;
>> +break;
>> +}
>>
>> pr_debug("%s: returning rc=%d\n", __func__, rc);
>> return rc;
>> @@ -642,7 +688,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
>> cxlflash_cfg *cfg)
>> struct pci_dev *pdev = cfg->dev;
>>
>> if (pci_channel_offline(pdev))
>> -wait_event_timeout(cfg->eeh_waitq,
>> +wait_event_timeout(cfg->limbo_waitq,
>> !pci_channel_offline(pdev),
>> CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);
>> }
>> @@ -825,6 +871,8 @@ static void cxlflash_remove(struct pci_dev *pdev)
>> !cfg->tmf_active);
>> spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
>>
>> +cfg->state = STATE_FAILTERM;
>
> I don't see any locking around the setting or reading of this flag. What are
> the
> implications if the processor reorders the store to change this state either
> here
> or elsewhere. Same goes for the load associated with checking the state.
>
>> +
>> switch (cfg->init_state) {
>> case INIT_STATE_SCSI:
>> scsi_remove_host(cfg->host);
>> @@ -1879,6 +1927,8 @@ static int init_afu(struct cxlflash_cfg *cfg)
>> struct afu *afu = cfg->afu;
>> struct device *dev = &cfg->dev->dev;
>>
>> +cxl_perst_reloads_same_image(cfg->cxl_afu, true);
>> +
>> rc = init_mc(c
Re: [PATCH] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()
Johannes Thumshirn wrote: > If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution > continues normally and mp gets kfree()d. > > If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the > error label where lpfc_mbuf_free() is called with mp->virt and mp->phys as > function arguments. This is the use after free. Following the use after free > mp > gets kfree()d again which is a double free. A similar patch was posted by Colin Ian King on 2015-07-31 [1]. [1] http://marc.info/?l=linux-scsi&m=143835937206204&w=2 Sebastian -- 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
Re: Persistent Reservation API V2
Does this look fine to you Jens? I'd love to get this API into 4.3 so I can submit the NFS SCSI layout patches that depend on it for 4.4. -- 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
Re: [PATCH 1/1] Using the local variable instead of I/O flag to acquire io_req_lock in fnic_queuecommand() to avoid deadloack
Broadcasting…
Regards,
Hiral
On 7/20/15, 5:48 PM, "Hiral Shah (hishah)" wrote:
>Hi Martin,
>
>Thanks for the suggestion. Actually boot make more sense as the value will be
>either 0 or 1 only. We have already release following patch to other non-linux
>customers. We will make sure next time.
>
>
>Regards,
>Hiral
>
>
>
>
>
>
>
>
>On 7/16/15, 9:56 PM, "Martin K. Petersen" wrote:
>
>>> "Hiral" == Hiral Shah writes:
>>
>>Hiral> We added changes in fnic driver patch 1.6.0.16 to acquire
>>Hiral> io_req_lock in fnic_queuecommand() before issuing I/O so that io
>>Hiral> completion is serialized. But when releasing the lock we check
>>Hiral> for the I/O flag and this could be modified if IO abort occurs
>>Hiral> before I/O completion. In this case we wont release the lock and
>>Hiral> causes deadlock in some scenerios. Using the local variable to
>>Hiral> check the IO lock status will resolve the problem.
>>
>>Maybe bool instead of int?
>>
>>Otherwise OK.
>>
>>Reviewed-by: Martin K. Petersen
>>
>>--
>>Martin K. PetersenOracle Linux Engineering
N�r��yb�X��ǧv�^�){.n�+{���"�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
[PATCH] SCSI: Fix NULL pointer dereference in runtime PM
The routines in scsi_rpm.c assume that if a runtime-PM callback is
invoked for a SCSI device, it can only mean that the device's driver
has asked the block layer to handle the runtime power management (by
calling blk_pm_runtime_init(), which among other things sets q->dev).
However, this assumption turns out to be wrong for things like the ses
driver. Normally ses devices are not allowed to do runtime PM, but
userspace can override this setting. If this happens, the kernel gets
a NULL pointer dereference when blk_post_runtime_resume() tries to use
the uninitialized q->dev pointer.
This patch fixes the problem by calling the block layer's runtime-PM
routines only if the device's driver really does have a runtime-PM
callback routine. Since ses doesn't define any such callbacks, the
crash won't occur.
This fixes Bugzilla #101371.
Signed-off-by: Alan Stern
Reported-by: Stanisław Pitucha
Reported-by: Ilan Cohen
Tested-by: Ilan Cohen
---
[as1784]
drivers/scsi/scsi_pm.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
Index: usb-4.0/drivers/scsi/scsi_pm.c
===
--- usb-4.0.orig/drivers/scsi/scsi_pm.c
+++ usb-4.0/drivers/scsi/scsi_pm.c
@@ -217,15 +217,15 @@ static int sdev_runtime_suspend(struct d
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
struct scsi_device *sdev = to_scsi_device(dev);
- int err;
+ int err = 0;
- err = blk_pre_runtime_suspend(sdev->request_queue);
- if (err)
- return err;
- if (pm && pm->runtime_suspend)
+ if (pm && pm->runtime_suspend) {
+ err = blk_pre_runtime_suspend(sdev->request_queue);
+ if (err)
+ return err;
err = pm->runtime_suspend(dev);
- blk_post_runtime_suspend(sdev->request_queue, err);
-
+ blk_post_runtime_suspend(sdev->request_queue, err);
+ }
return err;
}
@@ -248,11 +248,11 @@ static int sdev_runtime_resume(struct de
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int err = 0;
- blk_pre_runtime_resume(sdev->request_queue);
- if (pm && pm->runtime_resume)
+ if (pm && pm->runtime_resume) {
+ blk_pre_runtime_resume(sdev->request_queue);
err = pm->runtime_resume(dev);
- blk_post_runtime_resume(sdev->request_queue, err);
-
+ blk_post_runtime_resume(sdev->request_queue, err);
+ }
return err;
}
--
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
[patch] qla4xxx: remove some bogus casts
These casts are wrong and unnecessary. They annoy static checkers
because they imply we are planning to write sizeof(long) bytes to a
sizeof(u32) buffer which would corrupt memory.
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index 4180d6d..d97ea5d 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -101,19 +101,13 @@ int qla4xxx_init_rings(struct scsi_qla_host *ha)
ha->response_ptr = &ha->response_ring[ha->response_out];
if (is_qla8022(ha)) {
- writel(0,
- (unsigned long __iomem *)&ha->qla4_82xx_reg->req_q_out);
- writel(0,
- (unsigned long __iomem *)&ha->qla4_82xx_reg->rsp_q_in);
- writel(0,
- (unsigned long __iomem *)&ha->qla4_82xx_reg->rsp_q_out);
+ writel(0, &ha->qla4_82xx_reg->req_q_out);
+ writel(0, &ha->qla4_82xx_reg->rsp_q_in);
+ writel(0, &ha->qla4_82xx_reg->rsp_q_out);
} else if (is_qla8032(ha) || is_qla8042(ha)) {
- writel(0,
- (unsigned long __iomem *)&ha->qla4_83xx_reg->req_q_in);
- writel(0,
- (unsigned long __iomem *)&ha->qla4_83xx_reg->rsp_q_in);
- writel(0,
- (unsigned long __iomem *)&ha->qla4_83xx_reg->rsp_q_out);
+ writel(0, &ha->qla4_83xx_reg->req_q_in);
+ writel(0, &ha->qla4_83xx_reg->rsp_q_in);
+ writel(0, &ha->qla4_83xx_reg->rsp_q_out);
} else {
/*
* Initialize DMA Shadow registers. The firmware is really
--
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
Re: [PATCH v6 1/3] cxlflash: Base error recovery support
On 08/13/2015 09:47 PM, Matthew R. Ochs wrote:
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -76,6 +76,12 @@ enum cxlflash_init_state {
> INIT_STATE_SCSI
> };
>
> +enum cxlflash_state {
> + STATE_NORMAL, /* Normal running state, everything good */
> + STATE_LIMBO,/* Limbo running state, trying to reset/recover */
Might be more clear to call this STATE_RESETTING or STATE_RECOVERY
> + STATE_FAILTERM /* Failed/terminating state, error out users/threads */
> +};
> +
> /*
> * Each context has its own set of resource handles that is visible
> * only from that context.
> @@ -105,7 +109,8 @@ struct cxlflash_cfg {
>
> wait_queue_head_t tmf_waitq;
> bool tmf_active;
> - u8 err_recovery_active:1;
> + wait_queue_head_t limbo_waitq;
How about reset_waitq instead?
> + enum cxlflash_state state;
> };
>
> struct afu_cmd {
> @@ -455,9 +471,21 @@ static int cxlflash_eh_device_reset_handler(struct
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>
> - rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> - if (unlikely(rcr))
> + switch (cfg->state) {
> + case STATE_NORMAL:
> + rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> + if (unlikely(rcr))
> + rc = FAILED;
> + break;
> + case STATE_LIMBO:
> + wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
> + if (cfg->state == STATE_NORMAL)
> + break;
In this case you've been asked to do a LUN reset but didn't actually reset the
LUN. I'd suggest
restructuring this switch statement to send the LUN reset TMF in the case where
you had
to wait for an AFU reset to complete.
> + /* fall through */
> + default:
> rc = FAILED;
> + break;
> + }
>
> pr_debug("%s: returning rc=%d\n", __func__, rc);
> return rc;
> @@ -487,11 +515,29 @@ static int cxlflash_eh_host_reset_handler(struct
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>
> - rcr = cxlflash_afu_reset(cfg);
> - if (rcr == 0)
> - rc = SUCCESS;
> - else
> + switch (cfg->state) {
> + case STATE_NORMAL:
> + cfg->state = STATE_LIMBO;
> + scsi_block_requests(cfg->host);
> +
> + rcr = cxlflash_afu_reset(cfg);
> + if (rcr) {
> + rc = FAILED;
> + cfg->state = STATE_FAILTERM;
> + } else
> + cfg->state = STATE_NORMAL;
> + wake_up_all(&cfg->limbo_waitq);
> + scsi_unblock_requests(cfg->host);
The scsi_block_requests / scsi_unblock_requests is not necessary in this path,
since SCSI EH
will already be preventing any new commands being issued via queuecommand.
> + break;
> + case STATE_LIMBO:
> + wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
> + if (cfg->state == STATE_NORMAL)
> + break;
> + /* fall through */
> + default:
> rc = FAILED;
> + break;
> + }
>
> pr_debug("%s: returning rc=%d\n", __func__, rc);
> return rc;
> @@ -642,7 +688,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
> cxlflash_cfg *cfg)
> struct pci_dev *pdev = cfg->dev;
>
> if (pci_channel_offline(pdev))
> - wait_event_timeout(cfg->eeh_waitq,
> + wait_event_timeout(cfg->limbo_waitq,
> !pci_channel_offline(pdev),
> CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);
> }
> @@ -825,6 +871,8 @@ static void cxlflash_remove(struct pci_dev *pdev)
> !cfg->tmf_active);
> spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
>
> + cfg->state = STATE_FAILTERM;
I don't see any locking around the setting or reading of this flag. What are the
implications if the processor reorders the store to change this state either
here
or elsewhere. Same goes for the load associated with checking the state.
> +
> switch (cfg->init_state) {
> case INIT_STATE_SCSI:
> scsi_remove_host(cfg->host);
> @@ -1879,6 +1927,8 @@ static int init_afu(struct cxlflash_cfg *cfg)
> struct afu *afu = cfg->afu;
> struct device *dev = &cfg->dev->dev;
>
> + cxl_perst_reloads_same_image(cfg->cxl_afu, true);
> +
> rc = init_mc(cfg);
> if (rc) {
> dev_err(dev, "%s: call to init_mc failed, rc=%d!\n",
Reviewed-by: Brian King
--
Brian King
Power Linux I/O
IBM Linux Technology Center
--
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:/
Re: [PATCH V5 11/11] [SCSI] aacraid: Requests at least 2 MSIx in pci_enable_msix_range()
On Mon, 2015-08-17 at 14:12 +, Mahesh Rajashekhara wrote: > Hi James, > > Please let me know the status of V6 patchset. Well, you altered it fairly significantly with the combination, so I need the original reviewers at least to glance over it and say their reviews stand. If you kept the review tags going V5->V6, this would be a lot easier: as long as the reviewers were cc'd, I could apply the series if they didn't object. Now I have to wait for them to say it's OK. James -- 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
RE: [PATCH V5 11/11] [SCSI] aacraid: Requests at least 2 MSIx in pci_enable_msix_range()
Hi James, Please let me know the status of V6 patchset. Thanks, Mahesh -Original Message- From: Mahesh Rajashekhara [mailto:mahesh.rajashekh...@pmcs.com] Sent: Thursday, August 13, 2015 5:52 AM To: James Bottomley; Tomas Henzl Cc: jbottom...@parallels.com; linux-scsi@vger.kernel.org; aacr...@pmc-sierra.com; Harry Yang; Rich Bono Subject: RE: [PATCH V5 11/11] [SCSI] aacraid: Requests at least 2 MSIx in pci_enable_msix_range() As Tomas Henzl suggested, I have merged 10 and 11 of V5 into single patch (i.e 9 of V6) Changes from V5: Applied driver update change at the end. Merged 10 and 11 of V5 into 9 of V6. AAC_MAX_MSIX definition change. aac_msi option description and subject change. Thanks, Mahesh -Original Message- From: James Bottomley [mailto:james.bottom...@hansenpartnership.com] Sent: Wednesday, August 12, 2015 11:59 PM To: Tomas Henzl Cc: Rajinikanth Pandurangan; jbottom...@parallels.com; linux-scsi@vger.kernel.org; aacr...@pmc-sierra.com; Harry Yang; Rich Bono; Mahesh Rajashekhara; Achim Leubner; Murthy Bhat Subject: Re: [PATCH V5 11/11] [SCSI] aacraid: Requests at least 2 MSIx in pci_enable_msix_range() > On 22.7.2015 18:49, rajinikanth.panduran...@pmcs.com wrote: > > From: Rajinikanth Pandurangan > > > > Description: > > In MSIx mode, we need at least 2 vectors. > > > > Changes from V4: > > Newly created for V5 based on review comment. > > > > Signed-off-by: Rajinikanth Pandurangan > > Reviewed-by: Tomas Henzl I'm confused by your [PATCH v6] posting: where did this patch go ... do you want it dropped or did you just forget to include it? James
[PATCH v2] pm80xx: Don't override ts->stat on IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
In case psataPayload->status has a status of IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
ts->stat gets set to SAS_OPEN_REJECT but a missing 'break' statement causes a
fallthrough to the default handler of the switch statement overriding ts->stat
to SAS_DEV_NO_RESPONSE.
Signed-off-by: Johannes Thumshirn
Acked-by: Jack Wang
---
drivers/scsi/pm8001/pm8001_hwi.c | 1 +
drivers/scsi/pm8001/pm80xx_hwi.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 96dcc09..d0feec5 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -2642,6 +2642,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
void *piomb)
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_OPEN_REJECT;
ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+ break;
default:
PM8001_IO_DBG(pm8001_ha,
pm8001_printk("Unknown status 0x%x\n", status));
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 05cce46..18d4ac4 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2314,6 +2314,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
void *piomb)
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_OPEN_REJECT;
ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+ break;
default:
PM8001_IO_DBG(pm8001_ha,
pm8001_printk("Unknown status 0x%x\n", status));
--
2.5.0
--
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
Re: [PATCH] pm80xx: Don't override ts->stat on IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
Jack Wang writes:
> 2015-08-17 15:04 GMT+02:00 Johannes Thumshirn :
>> In case XXX returns with a status of IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
Apparently I've hit enter to fast. This shouldn't be XXX but
psataPayload->status. I'll be sending a v2 :-(.
>> ts->stat gets set to SAS_OPEN_REJECT but a missing 'break' statement causes a
>> fallthrough to the default handler of the switch statement overriding
>> ts->stat
>> to SAS_DEV_NO_RESPONSE.
>>
>> Signed-off-by: Johannes Thumshirn
>
> Thanks, please feel free to add:
> Acked-by: Jack Wang
>
>> ---
>> drivers/scsi/pm8001/pm8001_hwi.c | 1 +
>> drivers/scsi/pm8001/pm80xx_hwi.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
>> b/drivers/scsi/pm8001/pm8001_hwi.c
>> index 96dcc09..d0feec5 100644
>> --- a/drivers/scsi/pm8001/pm8001_hwi.c
>> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
>> @@ -2642,6 +2642,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
>> void *piomb)
>> ts->resp = SAS_TASK_COMPLETE;
>> ts->stat = SAS_OPEN_REJECT;
>> ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
>> + break;
>> default:
>> PM8001_IO_DBG(pm8001_ha,
>> pm8001_printk("Unknown status 0x%x\n", status));
>> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c
>> b/drivers/scsi/pm8001/pm80xx_hwi.c
>> index 05cce46..18d4ac4 100644
>> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
>> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
>> @@ -2314,6 +2314,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
>> void *piomb)
>> ts->resp = SAS_TASK_COMPLETE;
>> ts->stat = SAS_OPEN_REJECT;
>> ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
>> + break;
>> default:
>> PM8001_IO_DBG(pm8001_ha,
>> pm8001_printk("Unknown status 0x%x\n", status));
>> --
>> 2.5.0
>>
> --
> 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
--
Johannes Thumshirn Storage
jthumsh...@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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
Re: [PATCH] pm80xx: Don't override ts->stat on IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
2015-08-17 15:04 GMT+02:00 Johannes Thumshirn :
> In case XXX returns with a status of IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
> ts->stat gets set to SAS_OPEN_REJECT but a missing 'break' statement causes a
> fallthrough to the default handler of the switch statement overriding ts->stat
> to SAS_DEV_NO_RESPONSE.
>
> Signed-off-by: Johannes Thumshirn
Thanks, please feel free to add:
Acked-by: Jack Wang
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 1 +
> drivers/scsi/pm8001/pm80xx_hwi.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
> b/drivers/scsi/pm8001/pm8001_hwi.c
> index 96dcc09..d0feec5 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -2642,6 +2642,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
> void *piomb)
> ts->resp = SAS_TASK_COMPLETE;
> ts->stat = SAS_OPEN_REJECT;
> ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
> + break;
> default:
> PM8001_IO_DBG(pm8001_ha,
> pm8001_printk("Unknown status 0x%x\n", status));
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c
> b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 05cce46..18d4ac4 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -2314,6 +2314,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
> void *piomb)
> ts->resp = SAS_TASK_COMPLETE;
> ts->stat = SAS_OPEN_REJECT;
> ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
> + break;
> default:
> PM8001_IO_DBG(pm8001_ha,
> pm8001_printk("Unknown status 0x%x\n", status));
> --
> 2.5.0
>
--
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
[PATCH] pm80xx: Don't override ts->stat on IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
In case XXX returns with a status of IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY
ts->stat gets set to SAS_OPEN_REJECT but a missing 'break' statement causes a
fallthrough to the default handler of the switch statement overriding ts->stat
to SAS_DEV_NO_RESPONSE.
Signed-off-by: Johannes Thumshirn
---
drivers/scsi/pm8001/pm8001_hwi.c | 1 +
drivers/scsi/pm8001/pm80xx_hwi.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 96dcc09..d0feec5 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -2642,6 +2642,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
void *piomb)
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_OPEN_REJECT;
ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+ break;
default:
PM8001_IO_DBG(pm8001_ha,
pm8001_printk("Unknown status 0x%x\n", status));
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 05cce46..18d4ac4 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2314,6 +2314,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
void *piomb)
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_OPEN_REJECT;
ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+ break;
default:
PM8001_IO_DBG(pm8001_ha,
pm8001_printk("Unknown status 0x%x\n", status));
--
2.5.0
--
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
[PATCH] [SCSI] bfa: check if port is non NULL before dereferencing
In bfa_fcs_lport_get_rport_max_speed() check if port is non NULL before dereferencing it's child port->fcs->bfa to trl_enabled. NB: I'm not entirely sure if port can even be NULL, so the check for NULL might be useless as well. Signed-off-by: Johannes Thumshirn --- drivers/scsi/bfa/bfa_fcs_lport.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c index ff75ef8..096c2a2 100644 --- a/drivers/scsi/bfa/bfa_fcs_lport.c +++ b/drivers/scsi/bfa/bfa_fcs_lport.c @@ -5826,12 +5826,14 @@ bfa_fcs_lport_get_rport_max_speed(bfa_fcs_lport_t *port) bfa_port_speed_t max_speed = 0; struct bfa_port_attr_s port_attr; bfa_port_speed_t port_speed, rport_speed; - bfa_boolean_t trl_enabled = bfa_fcport_is_ratelim(port->fcs->bfa); + bfa_boolean_t trl_enabled; if (port == NULL) return 0; + trl_enabled = bfa_fcport_is_ratelim(port->fcs->bfa); + fcs = port->fcs; /* Get Physical port's current speed */ -- 2.5.0 -- 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
[PATCH] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()
If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution continues normally and mp gets kfree()d. If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the error label where lpfc_mbuf_free() is called with mp->virt and mp->phys as function arguments. This is the use after free. Following the use after free mp gets kfree()d again which is a double free. Signed-off-by: Johannes Thumshirn --- drivers/scsi/lpfc/lpfc_mbox.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c index eb62772..4abb93a 100644 --- a/drivers/scsi/lpfc/lpfc_mbox.c +++ b/drivers/scsi/lpfc/lpfc_mbox.c @@ -2284,7 +2284,7 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, LPFC_MBOXQ_t *mbox) (struct lpfc_rdp_context *)(mbox->context2); if (bf_get(lpfc_mqe_status, &mbox->u.mqe)) - goto error; + goto error_mbuf_free; lpfc_sli_bemem_bcopy(mp->virt, &rdp_context->page_a2, DMP_SFF_PAGE_A2_SIZE); @@ -2299,13 +2299,14 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, LPFC_MBOXQ_t *mbox) mbox->mbox_cmpl = lpfc_mbx_cmpl_rdp_link_stat; mbox->context2 = (struct lpfc_rdp_context *) rdp_context; if (lpfc_sli_issue_mbox(phba, mbox, MBX_NOWAIT) == MBX_NOT_FINISHED) - goto error; + goto error_cmd_free; return; -error: +error_mbuf_free: lpfc_mbuf_free(phba, mp->virt, mp->phys); kfree(mp); +error_cmd_free: lpfc_sli4_mbox_cmd_free(phba, mbox); rdp_context->cmpl(phba, rdp_context, FAILURE); } -- 2.5.0 -- 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
Re: [PATCH V2 2/8] pm80xx: Corrected device state changes in I_T_Nexus_Reset.
On Fri, Aug 14, 2015 at 9:15 AM, Viswas G wrote: > On Thu, Aug 13, 2015 at 6:13 PM, Jinpu Wang > wrote: >> Hi >> >> On Tue, Aug 11, 2015 at 11:36 AM, wrote: >>> From: Viswas G >>> >>> In Nexus reset the device state request are not needed. >>> >>> Changes from V1: >>> Device state change request has been removed as the firmware >>> will handle it during internal cleanup. Also updated the >>> proper return value in case of failures. >> >> If above is true, should we remove the device state change command >> support at all? >> > > We can't remove the state change command as it is required for > some task management command such as ABORT_TASK. > Ok, thanks -- Mit freundlichen Grüßen, Best Regards, Jack Wang Linux Kernel Developer Storage ProfitBricks GmbH The IaaS-Company. ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Tel: +49 30 5770083-42 Fax: +49 30 5770085-98 Email: jinpu.w...@profitbricks.com URL: http://www.profitbricks.de Sitz der Gesellschaft: Berlin. Registergericht: Amtsgericht Charlottenburg, HRB 125506 B. Geschäftsführer: Andreas Gauger, Achim Weiss. -- 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
