Re: Re: [PATCH] sata_dwc_460ex: Fix missing check in sata_dwc_isr

From: Andy Shevchenko
Date: Mon Mar 01 2021 - 08:48:14 EST


On Mon, Mar 1, 2021 at 1:20 PM <dinghao.liu@xxxxxxxxxx> wrote:
>
> > On Mon, Mar 1, 2021 at 9:44 AM Dinghao Liu <dinghao.liu@xxxxxxxxxx> 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