Re: NULL pointer dereference in qla24xx_abort_command, kernel 4.19.98 (Debian)

Ondrej Zary
2020-03-02T22:26:14Z

On Thursday 27 February 2020 18:09:07 Ondrej Zary wrote:
>
> 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...

Let's try to CC the people at Cavium that signed-off the commit.

--
Ondrej Zary