linux-kernel - Re: NULL pointer dereference in qla24xx_abort

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]

Date:   Thu, 27 Feb 2020 18:09:07 +0100
From:   Ondrej Zary <linux@...y.sk>
To:     Bart Van Assche <bvanassche@....org>
Cc:     qla2xxx-upstream@...gic.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: NULL pointer dereference in qla24xx_abort_command, kernel 4.19.98 (Debian)



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