Re: [PATCH 2/2] qla2xxx: Fix inadequate lock protection for ABTS.

["Madhani, Himanshu" <Himanshu.Madhani@cavium.com>]

Hi Greg, 

> On Mar 31, 2017, at 12:46 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> On Thu, Mar 30, 2017 at 09:18:53AM -0700, Himanshu Madhani wrote:
>> 
>> 
>> On Thu, 30 Mar 2017, 1:38am, Greg KH wrote:
>> 
>>> On Tue, Mar 28, 2017 at 12:50:20PM -0700, Himanshu Madhani wrote:
>>>> From: Quinn Tran <quinn.tran@cavium.com>
>>>> 
>>>> commit 8f6fc8d4e7ae2347d6261d11a7eb2b247d2954d8 upstream.
>>>> 
>>>> Normally, ABTS is sent to Target Core as Task MGMT command.
>>>> In the case of error, qla2xxx needs to send response, hardware_lock
>>>> is required to prevent request queue corruption.
>>>> 
>>>> Cc: <stable@vger.kernel.org>
>>>> Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
>>>> Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
>>>> ---
>>>> drivers/scsi/qla2xxx/qla_target.c | 11 ++++++++++-
>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
>>>> index 26fe9cb3a963..e2e71e0de857 100644
>>>> --- a/drivers/scsi/qla2xxx/qla_target.c
>>>> +++ b/drivers/scsi/qla2xxx/qla_target.c
>>>> @@ -120,6 +120,9 @@ static void qlt_send_notify_ack(struct scsi_qla_host *vha,
>>>> 	uint16_t srr_flags, uint16_t srr_reject_code, uint8_t srr_explan);
>>>> static void qlt_send_term_imm_notif(struct scsi_qla_host *vha,
>>>> 	struct imm_ntfy_from_isp *imm, int ha_locked);
>>>> +static void qlt_24xx_handle_abts(struct scsi_qla_host *,
>>>> +	struct abts_recv_from_24xx *);
>>>> +
>>>> /*
>>>>  * Global Variables
>>>>  */
>>>> @@ -357,6 +360,8 @@ void qlt_response_pkt_all_vps(struct scsi_qla_host *vha, response_t *pkt)
>>>> 		    (struct abts_recv_from_24xx *)pkt;
>>>> 		struct scsi_qla_host *host = qlt_find_host_by_vp_idx(vha,
>>>> 		    entry->vp_index);
>>>> +		unsigned long flags;
>>>> +
>>>> 		if (unlikely(!host)) {
>>>> 			ql_dbg(ql_dbg_tgt, vha, 0xe044,
>>>> 			    "qla_target(%d): Response pkt "
>>>> @@ -364,7 +369,11 @@ void qlt_response_pkt_all_vps(struct scsi_qla_host *vha, response_t *pkt)
>>>> 			    "vp_index %d\n", vha->vp_idx, entry->vp_index);
>>>> 			break;
>>>> 		}
>>>> -		qlt_response_pkt(host, pkt);
>>>> +		if (!ha_locked)
>>>> +			spin_lock_irqsave(&host->hw->hardware_lock, flags);
>>>> +		qlt_24xx_handle_abts(host, (struct abts_recv_from_24xx *)atio);
>>>> +		if (!ha_locked)
>>>> +			spin_unlock_irqrestore(&host->hw->hardware_lock, flags);
>>>> 		break;
>>>> 	}
>>>> 
>>> 
>>> You didn't test build this patch did you :(
>>> 
>>> Please do so, it breaks the build here on my end.  I'll drop it from my
>>> queue, please fix this up _properly_ and resend after you have tested it
>>> out.
>>> 
>> 
>> Apologies for the wrong backport. Looks like original patch has not yet 
>> made it into 4.10 stable. So this backport is not needed at this time.
> 
> What "original patch”?

This patch (2/2) fixes issue which was introduced by following patch.
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=41dc529a4602ac737020f423f84686a81de38e6d

> 
> Does that mean that patch 1/2 of this series should also be dropped?
> 

No 1/2 patch is independent of the this patch. 

> totally confused,
> 
> greg k-h

Thanks,
- Himanshu