From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Dinghao Liu <dinghao.liu@zju.edu.cn>
Cc: Kangjie Lu <kjlu@umn.edu>, Jens Axboe <axboe@kernel.dk>,
linux-ide@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Re: [PATCH] sata_dwc_460ex: Fix missing check in sata_dwc_isr
Date: Mon, 1 Mar 2021 15:47:03 +0200 [thread overview]
Message-ID: <CAHp75VdT04AVzW=C=SubHjUE5_MHBeC0ptHTFSLgKzrSP3HmRQ@mail.gmail.com> (raw)
In-Reply-To: <44c09a14.a0cbd.177ed8446ea.Coremail.dinghao.liu@zju.edu.cn>
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.
> 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
next prev parent reply other threads:[~2021-03-01 13:48 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-01 7:28 [PATCH] sata_dwc_460ex: Fix missing check in sata_dwc_isr Dinghao Liu
2021-03-01 10:31 ` Andy Shevchenko
2021-03-01 11:20 ` dinghao.liu
2021-03-01 13:47 ` Andy Shevchenko [this message]
2021-03-02 7:33 ` dinghao.liu
2021-03-02 10:20 ` Andy Shevchenko
2021-03-03 5:21 ` dinghao.liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHp75VdT04AVzW=C=SubHjUE5_MHBeC0ptHTFSLgKzrSP3HmRQ@mail.gmail.com' \
--to=andy.shevchenko@gmail.com \
--cc=axboe@kernel.dk \
--cc=dinghao.liu@zju.edu.cn \
--cc=kjlu@umn.edu \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).