linux-kernel - Re: 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: <56ec2ffd.a3db1.177f1db3d26.Coremail.dinghao.liu@zju.edu.cn>
Date:   Tue, 2 Mar 2021 15:33:33 +0800 (GMT+08:00)
From:   dinghao.liu@....edu.cn
To:     "Andy Shevchenko" <andy.shevchenko@...il.com>
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: 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.
> 

I cannot answer this question immediately. I think it's possible to build 
a designed input to trigger this case for some professional attackers. 

> > 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.
> 

Thanks for your advice. I also checked all use of ata_qc_from_tag() in the 
whole kernel and found all of them had return value checks except for the 
calls in sata_dwc_isr(), which is odd. There is no issue currently does not
mean it will never happen in the future. So I suggest the maintainer of function
sata_dwc_isr() to fix this issue.

Regards,
Dinghao