Messages in this thread | | | Date | Tue, 2 Mar 2021 15:33:33 +0800 (GMT+08:00) | From | dinghao.liu@zju ... | 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@zju.edu.cn> wrote: > > > > > On Mon, Mar 1, 2021 at 9:44 AM Dinghao Liu <dinghao.liu@zju.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
| |