From: Dan Carpenter <dan.carpenter@oracle.com>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: kbuild@lists.01.org, lkp@intel.com, kbuild-all@lists.01.org,
linux-kernel@vger.kernel.org,
Heiko Carstens <heiko.carstens@de.ibm.com>
Subject: Re: arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
Date: Thu, 3 Dec 2020 14:19:54 +0300 [thread overview]
Message-ID: <20201203111954.GK2789@kadam> (raw)
In-Reply-To: <52b96642-f78c-478e-913d-b294c385c5ab@linux.ibm.com>
On Thu, Dec 03, 2020 at 11:52:48AM +0100, Niklas Schnelle wrote:
>
>
> On 12/3/20 11:27 AM, Dan Carpenter wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 3bb61aa61828499a7d0f5e560051625fd02ae7e4
> > commit: 3047766bc6ec9c6bc9ece85b45a41ff401e8d988 s390/pci: fix enabling a reserved PCI function
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
> >
> > vim +101 arch/s390/pci/pci_event.c
> >
> > aa3b7c296732b43 Sebastian Ott 2013-12-12 76 static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
> > cbc0dd1f856b52b Jan Glauber 2012-11-29 77 {
> > cbc0dd1f856b52b Jan Glauber 2012-11-29 78 struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
> > 9a99649f2a89fdf Sebastian Ott 2016-01-29 79 struct pci_dev *pdev = NULL;
> > 623bd44d3f277b7 Sebastian Ott 2017-05-09 80 enum zpci_state state;
> > d795ddad36cbc82 Sebastian Ott 2013-11-15 81 int ret;
> > cbc0dd1f856b52b Jan Glauber 2012-11-29 82
> > 05bc1be6db4b268 Pierre Morel 2020-03-23 @83 if (zdev && zdev->zbus && zdev->zbus->bus)
> > ^^^^^^^^^
> > Check for NULL
> >
> > 44510d6fa0c00aa Pierre Morel 2020-04-22 84 pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
> > 9a99649f2a89fdf Sebastian Ott 2016-01-29 85
> > 1f1dcbd4f23bd1f Sebastian Ott 2013-10-22 86 zpci_err("avail CCDF:\n");
> > 1f1dcbd4f23bd1f Sebastian Ott 2013-10-22 87 zpci_err_hex(ccdf, sizeof(*ccdf));
> > cbc0dd1f856b52b Jan Glauber 2012-11-29 88
> > cbc0dd1f856b52b Jan Glauber 2012-11-29 89 switch (ccdf->pec) {
> > 7fc611ff3ff1a0b Sebastian Ott 2015-06-16 90 case 0x0301: /* Reserved|Standby -> Configured */
> > 7fc611ff3ff1a0b Sebastian Ott 2015-06-16 91 if (!zdev) {
> > f606b3ef47c9f87 Pierre Morel 2020-03-25 92 ret = clp_add_pci_device(ccdf->fid, ccdf->fh, 1);
> > 7fc611ff3ff1a0b Sebastian Ott 2015-06-16 93 break;
> > 7fc611ff3ff1a0b Sebastian Ott 2015-06-16 94 }
> > fcf2f402937a669 Sebastian Ott 2013-12-18 95 zdev->fh = ccdf->fh;
> > f606b3ef47c9f87 Pierre Morel 2020-03-25 96 zdev->state = ZPCI_FN_STATE_CONFIGURED;
> > 3047766bc6ec9c6 Niklas Schnelle 2020-06-18 97 ret = zpci_enable_device(zdev);
> > 3047766bc6ec9c6 Niklas Schnelle 2020-06-18 98 if (ret)
> > 3047766bc6ec9c6 Niklas Schnelle 2020-06-18 99 break;
> > 3047766bc6ec9c6 Niklas Schnelle 2020-06-18 100
> > 3047766bc6ec9c6 Niklas Schnelle 2020-06-18 @101 pdev = pci_scan_single_device(zdev->zbus->bus, zdev->devfn);
> > ^^^^^^^^^^^^^^^^
> > Unchecked dereference
>
> First, thanks for reporting this is definitely appreciated!
> We have also seen the same smatch report internally
> and I determined that this is a false positive.
>
> This is because the existing zdev->zbus NULL check could already never
> trigger.
I don't consider it a "false positive" exactly because the NULL checking
is inconsisent. I would instead say that it is "correct but harmless".
That said, if Smatch can determined that "zdev->zbus" is not NULL then
it doesn't print a warning in these situations. As it stands now Smatch
doesn't understand lists very well, but I plan to fix this in upcoming
months. Once that gets fixed, Smatch will still assume that
zpci_create_device() is racy... :/ And then finally it will only
silence the warning when the cross function database has been built and
I don't think the the zero bot builds the DB for s390.
Anyway, these don't affect runtime so it's not time sensitive. Thanks
for taking the time to look at these!
regards,
dan carpenter
next prev parent reply other threads:[~2020-12-03 11:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-03 10:27 arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83) Dan Carpenter
2020-12-03 10:52 ` Niklas Schnelle
2020-12-03 11:19 ` Dan Carpenter [this message]
2020-12-03 11:48 ` Niklas Schnelle
2021-01-19 6:14 Dan Carpenter
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=20201203111954.GK2789@kadam \
--to=dan.carpenter@oracle.com \
--cc=heiko.carstens@de.ibm.com \
--cc=kbuild-all@lists.01.org \
--cc=kbuild@lists.01.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=schnelle@linux.ibm.com \
/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).