linux-kernel - Re: Re: [PATCH] sata_dwc_460ex: Fix missing check in sata_dwc

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: <CAHp75VdT04AVzW=C=SubHjUE5_MHBeC0ptHTFSLgKzrSP3HmRQ@mail.gmail.com>
Date:   Mon, 1 Mar 2021 15:47:03 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Dinghao Liu <dinghao.liu@....edu.cn>
Cc:     Kangjie Lu <kjlu@....edu>, Jens Axboe <axboe@...nel.dk>,
        linux-ide@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Re: [PATCH] sata_dwc_460ex: Fix missing check in sata_dwc_isr

On Mon, Mar 1, 2021 at 1:20 PM <dinghao.liu@....edu.cn> wrote:
>
> > On Mon, Mar 1, 2021 at 9:44 AM Dinghao Liu <dinghao.liu@....edu.cn> wrote:
> > >
> > > ata_qc_from_tag() may return a null pointer and further lead to
> > > null-pointer-dereference. Add a return value check to avoid such case.
> >
> > Can you elaborate more on this? Is it a real case?
> > I have a hardware, how can I reproduce this?
> >
>
> In the branch 'if (intpr & SATA_DWC_INTPR_NEWFP)', we call ata_qc_from_tag()
> and access qc->ap->link.active_tag immediately. If ata_qc_from_tag() returns
> a null pointer, accessing qc->ap->link.active_tag may crash the system.

Yes, I can see that. My question is how to get into the case when this
will be true.

> This issue is reported by my static analysis tool, so I don't have the
> vulnerable input currently.

Should we blindly follow everything that some (non-ideal) tool
reports? I don't think so.
For all my experiments with that hardware, I haven't heard about the
issue with NULL pointers. Useless checks make code harder to read and
CPU to waste cycles. It might be maintainers of this driver consider
otherwise, so not my call.

-- 
With Best Regards,
Andy Shevchenko