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