linux-kernel - Re: Reported regressions for 4.7 as of Sunday, 2016-06-19

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1957A3C6-C4C2-4E7A-84E1-31A48F1469CD@qlogic.com>
Date:	Thu, 23 Jun 2016 16:13:45 +0000
From:	Quinn Tran <quinn.tran@...gic.com>
To:	Johannes Thumshirn <jthumshirn@...e.de>
CC:	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Josh Boyer <jwboyer@...oraproject.org>,
	Thorsten Leemhuis <regressions@...mhuis.info>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: Reported regressions for 4.7 as of Sunday, 2016-06-19


-----Original Message-----
From: Johannes Thumshirn <jthumshirn@...e.de>
Date: Thursday, June 23, 2016 at 12:22 AM
To: Quinn Tran <quinn.tran@...gic.com>
Cc: "Martin K. Petersen" <martin.petersen@...cle.com>, Linus Torvalds <torvalds@...ux-foundation.org>, Josh Boyer <jwboyer@...oraproject.org>, Thorsten Leemhuis <regressions@...mhuis.info>, linux-kernel <linux-kernel@...r.kernel.org>, linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: Reported regressions for 4.7 as of Sunday, 2016-06-19

>[+ Cc linux-scsi@...r.kernel.org ]
>
>On Wed, Jun 22, 2016 at 03:57:35PM +0000, Quinn Tran wrote:
>> Johannes,  Martin,
>> 
>> Based on the screen shot/call trace,  it looks like this adapter is not using MSIX.  It defaulted back to MSI or INTx interrupt.  The code made an assumption  of MSIX is available.  There is no point in go through that code segment.
>> 
>> Can you try this work around?  It’s untested.  Thanks.
>> 
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
>> index 5649c20..e033ecb 100644
>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>> @@ -2548,7 +2548,7 @@ void qla24xx_process_response_queue(struct scsi_qla_host *vha,
>>         if (!vha->flags.online)
>>                 return;
>>  
>> -       if (rsp->msix->cpuid != smp_processor_id()) {
>> +       if (rsp->msix && (rsp->msix->cpuid != smp_processor_id())) {
>>                 /* if kernel does not notify qla of IRQ's CPU change,
>>                  * then set it here.
>>                  */
>> 
>
>But this still does not fix the race which would be possible if the HBA is
>using MSI-X but triggering IRQs early enough.
>
>Have a look at this (I admit theoretical) path:
>qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
>{
>	[...]
>	/* Enable MSI-X vectors for the base queue */
>	for (i = 0; i < 2; i++) {
>			qentry = &ha->msix_entries[i];
>			if (IS_P3P_TYPE(ha))
>				ret = request_irq(qentry->vector,
>					qla82xx_msix_entries[i].handler,
>					0, qla82xx_msix_entries[i].name, rsp);
>			else
>				ret = request_irq(qentry->vector,
>					msix_entries[i].handler,
>					0, msix_entries[i].name, rsp);
>			if (ret)
>				goto msix_register_fail;
>							<--- IRQ arrives here

QT: setting up the interrupt vector does not mean the interrupt starts firing immediately.  Interrupt starting firing when the driver is ready to accept the interrupt by enabling the interrupt (ha->isp_ops->enable_intrs(ha)) later on in time.  In addition, that particular code path/qla24xx_process_response_queue  is not executed until driver feeds commands to the hardware work queue.  

IF there is a left over interrupt that happens to trigger the call immediately, there is another check that prevent the code from getting to the point of the “theoretical" race.


>			qentry->have_irq = 1;
>			qentry->rsp = rsp;
>			rsp->msix = qentry;
>
>			[...]
>
>
>void qla24xx_process_response_queue(struct scsi_qla_host *vha,
>        struct rsp_que *rsp)
>{
--->8------
	if (!vha->flags.online)
		return;

---8<------
>	if (rsp->msix->cpuid != smp_processor_id()) {
>                  ^
>                  \--- rsp->msix == NULL
>
>			/* if kernel does not notify qla of IRQ's CPU change,
>			 * then set it here.
>			 */
>			rsp->msix->cpuid = smp_processor_id();
>			ha->tgt.rspq_vector_cpuid = rsp->msix->cpuid;
>
>-- 
>Johannes Thumshirn                                          Storage
>jthumshirn@...e.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