Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Ondrej Zary <linux@zary.sk>
To:     Bart Van Assche <bvanassche@acm.org>
Subject: Re: NULL pointer dereference in qla24xx_abort_command, kernel 4.19.98 (Debian)
Date:   Thu, 27 Feb 2020 18:09:07 +0100
User-Agent: KMail/1.9.10
Cc:     qla2xxx-upstream@qlogic.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <202002231929.01662.linux@zary.sk> <202002240920.17691.linux@zary.sk> <1fbad673-1b8c-0813-c60e-a4f56330a972@acm.org>
In-Reply-To: <1fbad673-1b8c-0813-c60e-a4f56330a972@acm.org>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <202002271809.07717.linux@zary.sk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On Tuesday 25 February 2020 04:41:48 Bart Van Assche wrote:
> On 2020-02-24 00:20, Ondrej Zary wrote:
> > Looks like it's in some inlined function.
> > 
> > /usr/src/linux-source-4.19# gdb /lib/modules/4.19.0-8-amd64/kernel/drivers/scsi/qla2xxx/qla2xxx.ko
> > GNU gdb (Debian 8.2.1-2+b3) 8.2.1
> > ...
> > Reading symbols from /lib/modules/4.19.0-8-amd64/kernel/drivers/scsi/qla2xxx/qla2xxx.ko...Reading symbols 
> > from /usr/lib/debug//lib/modules/4.19.0-8-amd64/kernel/drivers/scsi/qla2xxx/qla2xxx.ko...done.
> > done.
> > 
> > (gdb) list *(qla24xx_async_abort_cmd+0x1b)
> > 0xf88b is in qla24xx_async_abort_cmd (./arch/x86/include/asm/atomic.h:97).
> > 92       *
> > 93       * Atomically increments @v by 1.
> > 94       */
> > 95      static __always_inline void arch_atomic_inc(atomic_t *v)
> > 96      {
> > 97              asm volatile(LOCK_PREFIX "incl %0"
> > 98                           : "+m" (v->counter) :: "memory");
> > 99      }
> > 100     #define arch_atomic_inc arch_atomic_inc
> >
> > [ ... ]
> > 
> > (gdb) disassemble qla24xx_async_abort_cmd
> > Dump of assembler code for function qla24xx_async_abort_cmd:
> >    0x000000000000f870 <+0>:     callq  0xf875 <qla24xx_async_abort_cmd+5>
> >    0x000000000000f875 <+5>:     push   %r15
> >    0x000000000000f877 <+7>:     push   %r14
> >    0x000000000000f879 <+9>:     push   %r13
> >    0x000000000000f87b <+11>:    push   %r12
> >    0x000000000000f87d <+13>:    push   %rbp
> >    0x000000000000f87e <+14>:    push   %rbx
> >    0x000000000000f87f <+15>:    mov    0x28(%rdi),%r13
> >    0x000000000000f883 <+19>:    mov    0x20(%rdi),%r15
> >    0x000000000000f887 <+23>:    mov    0x48(%rdi),%r14
> >    0x000000000000f88b <+27>:    lock incl 0x4(%r14)
> >    0x000000000000f890 <+32>:    mfence
> 
> Thanks, this is very helpful. I think the above means that the crash is
> triggered by the following code:
> 
> 	sp = qla2xxx_get_qpair_sp(cmd_sp->qpair, cmd_sp->fcport,
> 		GFP_KERNEL);
> 
> From the start of qla2xxx_get_qpair_sp():
> 
> 	QLA_QPAIR_MARK_BUSY(qpair, bail);
> 
> From qla_def.h:
> 
> #define QLA_QPAIR_MARK_BUSY(__qpair, __bail) do {	\
> 	atomic_inc(&__qpair->ref_count);		\
> 	mb();						\
> 	if (__qpair->delete_in_progress) {		\
> 		atomic_dec(&__qpair->ref_count);	\
> 		__bail = 1;				\
> 	} else {					\
> 	       __bail = 0;				\
> 	}						\
> } while (0)
> 
> One of the changes between kernel version v4.9.210 and v4.19.98 is the
> following: "qla2xxx: Add multiple queue pair functionality". I think the
>  above information means that the cmd_sp->qpair pointer is NULL. I will
> let QLogic recommend a solution.

Thank you very much for the analysis.
Unfortunately, QLogic does not seem to care...

-- 
Ondrej Zary