* arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
@ 2020-12-03 10:27 Dan Carpenter
2020-12-03 10:52 ` Niklas Schnelle
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2020-12-03 10:27 UTC (permalink / raw)
To: kbuild, Niklas Schnelle; +Cc: lkp, kbuild-all, linux-kernel, Heiko Carstens
[-- Attachment #1: Type: text/plain, Size: 7068 bytes --]
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
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 102 if (!pdev)
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 103 break;
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 104
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 105 pci_bus_add_device(pdev);
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 106 pci_lock_rescan_remove();
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 107 pci_bus_add_devices(zdev->zbus->bus);
3047766bc6ec9c6 Niklas Schnelle 2020-06-18 108 pci_unlock_rescan_remove();
cbc0dd1f856b52b Jan Glauber 2012-11-29 109 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 110 case 0x0302: /* Reserved -> Standby */
f606b3ef47c9f87 Pierre Morel 2020-03-25 111 if (!zdev) {
d795ddad36cbc82 Sebastian Ott 2013-11-15 112 clp_add_pci_device(ccdf->fid, ccdf->fh, 0);
cbc0dd1f856b52b Jan Glauber 2012-11-29 113 break;
f606b3ef47c9f87 Pierre Morel 2020-03-25 114 }
f606b3ef47c9f87 Pierre Morel 2020-03-25 115 zdev->fh = ccdf->fh;
f606b3ef47c9f87 Pierre Morel 2020-03-25 116 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 117 case 0x0303: /* Deconfiguration requested */
623bd44d3f277b7 Sebastian Ott 2017-05-09 118 if (!zdev)
623bd44d3f277b7 Sebastian Ott 2017-05-09 119 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 120 if (pdev)
2a01bd1bd3d28d1 Sebastian Ott 2015-07-28 121 pci_stop_and_remove_bus_device_locked(pdev);
cbc0dd1f856b52b Jan Glauber 2012-11-29 122
d795ddad36cbc82 Sebastian Ott 2013-11-15 123 ret = zpci_disable_device(zdev);
d795ddad36cbc82 Sebastian Ott 2013-11-15 124 if (ret)
d795ddad36cbc82 Sebastian Ott 2013-11-15 125 break;
1f1dcbd4f23bd1f Sebastian Ott 2013-10-22 126
d795ddad36cbc82 Sebastian Ott 2013-11-15 127 ret = sclp_pci_deconfigure(zdev->fid);
d795ddad36cbc82 Sebastian Ott 2013-11-15 128 zpci_dbg(3, "deconf fid:%x, rc:%d\n", zdev->fid, ret);
d795ddad36cbc82 Sebastian Ott 2013-11-15 129 if (!ret)
d795ddad36cbc82 Sebastian Ott 2013-11-15 130 zdev->state = ZPCI_FN_STATE_STANDBY;
cbc0dd1f856b52b Jan Glauber 2012-11-29 131
d795ddad36cbc82 Sebastian Ott 2013-11-15 132 break;
623bd44d3f277b7 Sebastian Ott 2017-05-09 133 case 0x0304: /* Configured -> Standby|Reserved */
623bd44d3f277b7 Sebastian Ott 2017-05-09 134 if (!zdev)
623bd44d3f277b7 Sebastian Ott 2017-05-09 135 break;
0c0c2776926018e Sebastian Ott 2013-12-12 136 if (pdev) {
0c0c2776926018e Sebastian Ott 2013-12-12 137 /* Give the driver a hint that the function is
0c0c2776926018e Sebastian Ott 2013-12-12 138 * already unusable. */
0c0c2776926018e Sebastian Ott 2013-12-12 139 pdev->error_state = pci_channel_io_perm_failure;
2a01bd1bd3d28d1 Sebastian Ott 2015-07-28 140 pci_stop_and_remove_bus_device_locked(pdev);
0c0c2776926018e Sebastian Ott 2013-12-12 141 }
1f1dcbd4f23bd1f Sebastian Ott 2013-10-22 142
d795ddad36cbc82 Sebastian Ott 2013-11-15 143 zdev->state = ZPCI_FN_STATE_STANDBY;
623bd44d3f277b7 Sebastian Ott 2017-05-09 144 if (!clp_get_state(ccdf->fid, &state) &&
623bd44d3f277b7 Sebastian Ott 2017-05-09 145 state == ZPCI_FN_STATE_RESERVED) {
05bc1be6db4b268 Pierre Morel 2020-03-23 146 zpci_zdev_put(zdev);
623bd44d3f277b7 Sebastian Ott 2017-05-09 147 }
d795ddad36cbc82 Sebastian Ott 2013-11-15 148 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 149 case 0x0306: /* 0x308 or 0x302 for multiple devices */
d795ddad36cbc82 Sebastian Ott 2013-11-15 150 clp_rescan_pci_devices();
d795ddad36cbc82 Sebastian Ott 2013-11-15 151 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 152 case 0x0308: /* Standby -> Reserved */
704268925d32a04 Sebastian Ott 2013-12-12 153 if (!zdev)
704268925d32a04 Sebastian Ott 2013-12-12 154 break;
05bc1be6db4b268 Pierre Morel 2020-03-23 155 zpci_zdev_put(zdev);
d795ddad36cbc82 Sebastian Ott 2013-11-15 156 break;
d795ddad36cbc82 Sebastian Ott 2013-11-15 157 default:
d795ddad36cbc82 Sebastian Ott 2013-11-15 158 break;
cbc0dd1f856b52b Jan Glauber 2012-11-29 159 }
cbc0dd1f856b52b Jan Glauber 2012-11-29 160 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33031 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
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
0 siblings, 1 reply; 5+ messages in thread
From: Niklas Schnelle @ 2020-12-03 10:52 UTC (permalink / raw)
To: Dan Carpenter, kbuild; +Cc: lkp, kbuild-all, linux-kernel, Heiko Carstens
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. Sadly this is all a bit hard to see but let me try to share
my reasoning here. That said, we do plan a couple of cleanups/rework here
that will also see the removal of the zdev->zbus NULL check at the start
of the function.
The existing check is here:
76 static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
77 {
78 struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
79 struct pci_dev *pdev = NULL;
80 enum zpci_state state;
81 int ret;
82
83 if (zdev && zdev->zbus && zdev->zbus->bus)
84 pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
Now for both the new zdev->zbus->bus dereference and the check in line 83 we
check zdev != NULL beforehand. So that's a given.
If we have the zdev it came out of get_zdev_by_fid() (line 78) which found it in
the zpci_list.
It only ever gets into that list in zpci_create_device() and
gets removed if anything in there fails, including zpci_bus_register_device()
which when successful has definitely set zdev->zbus != NULL.
Furthermore zpci_create_device() is only called in clp_add_pci_device()
which is either called during init of zPCI (in the initial CLP List PCI)
or from the kthread that handles events via zpci_event_availability()
which checks the the initialization is complete.
Therefore there is no race for the window in which
zpci_create_device() has the zdev in the list but no zbus set.
Either way, this is definitely too hard to reason about e.g.
we should only add devices to the zpci_list once they are
fully initialized and we're working on a redesign of this.
If you prefer a short term fix so this doesn't trigger
again we could simply remove the zdev->zbus NULL check.
>
... snip ...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
2020-12-03 10:52 ` Niklas Schnelle
@ 2020-12-03 11:19 ` Dan Carpenter
2020-12-03 11:48 ` Niklas Schnelle
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2020-12-03 11:19 UTC (permalink / raw)
To: Niklas Schnelle; +Cc: kbuild, lkp, kbuild-all, linux-kernel, Heiko Carstens
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
2020-12-03 11:19 ` Dan Carpenter
@ 2020-12-03 11:48 ` Niklas Schnelle
0 siblings, 0 replies; 5+ messages in thread
From: Niklas Schnelle @ 2020-12-03 11:48 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kbuild, lkp, kbuild-all, linux-kernel, Heiko Carstens
On 12/3/20 12:19 PM, Dan Carpenter wrote:
> 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".
Good point, I think your wording captures the situation better and I agree
the checks as is definitely inconsistent.
>
> 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.
Sounds like some exciting developments are ahead that will
make smatch even more powerful. That said, it's already pretty amazing
to me what it does.
>
> Anyway, these don't affect runtime so it's not time sensitive. Thanks
> for taking the time to look at these!
Thank you for working on smatch. The report it gave for this definitely
made it easy to understand why it was complaining and when I saw it on our
internal smatch I too wasn't immediately sure it was harmless so the report was
definitely useful.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* arch/s390/pci/pci_event.c:101 __zpci_event_availability() error: we previously assumed 'zdev->zbus' could be null (see line 83)
@ 2021-01-19 6:14 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-01-19 6:14 UTC (permalink / raw)
To: kbuild, Niklas Schnelle; +Cc: lkp, kbuild-all, linux-kernel, Heiko Carstens
[-- Attachment #1: Type: text/plain, Size: 7082 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 65f0d2414b7079556fbbcc070b3d1c9f9587606d
commit: 3047766bc6ec9c6bc9ece85b45a41ff401e8d988 s390/pci: fix enabling a reserved PCI function
config: s390-randconfig-m031-20210114 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
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
aa3b7c296732b4 Sebastian Ott 2013-12-12 76 static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
cbc0dd1f856b52 Jan Glauber 2012-11-29 77 {
cbc0dd1f856b52 Jan Glauber 2012-11-29 78 struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
9a99649f2a89fd Sebastian Ott 2016-01-29 79 struct pci_dev *pdev = NULL;
623bd44d3f277b Sebastian Ott 2017-05-09 80 enum zpci_state state;
d795ddad36cbc8 Sebastian Ott 2013-11-15 81 int ret;
cbc0dd1f856b52 Jan Glauber 2012-11-29 82
05bc1be6db4b26 Pierre Morel 2020-03-23 @83 if (zdev && zdev->zbus && zdev->zbus->bus)
^^^^^^^^^^
Can this be NULL?
44510d6fa0c00a Pierre Morel 2020-04-22 84 pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
9a99649f2a89fd Sebastian Ott 2016-01-29 85
1f1dcbd4f23bd1 Sebastian Ott 2013-10-22 86 zpci_err("avail CCDF:\n");
1f1dcbd4f23bd1 Sebastian Ott 2013-10-22 87 zpci_err_hex(ccdf, sizeof(*ccdf));
cbc0dd1f856b52 Jan Glauber 2012-11-29 88
cbc0dd1f856b52 Jan Glauber 2012-11-29 89 switch (ccdf->pec) {
7fc611ff3ff1a0 Sebastian Ott 2015-06-16 90 case 0x0301: /* Reserved|Standby -> Configured */
7fc611ff3ff1a0 Sebastian Ott 2015-06-16 91 if (!zdev) {
f606b3ef47c9f8 Pierre Morel 2020-03-25 92 ret = clp_add_pci_device(ccdf->fid, ccdf->fh, 1);
7fc611ff3ff1a0 Sebastian Ott 2015-06-16 93 break;
7fc611ff3ff1a0 Sebastian Ott 2015-06-16 94 }
fcf2f402937a66 Sebastian Ott 2013-12-18 95 zdev->fh = ccdf->fh;
f606b3ef47c9f8 Pierre Morel 2020-03-25 96 zdev->state = ZPCI_FN_STATE_CONFIGURED;
3047766bc6ec9c Niklas Schnelle 2020-06-18 97 ret = zpci_enable_device(zdev);
3047766bc6ec9c Niklas Schnelle 2020-06-18 98 if (ret)
3047766bc6ec9c Niklas Schnelle 2020-06-18 99 break;
3047766bc6ec9c Niklas Schnelle 2020-06-18 100
3047766bc6ec9c Niklas Schnelle 2020-06-18 @101 pdev = pci_scan_single_device(zdev->zbus->bus, zdev->devfn);
^^^^^^^^^^^^^^^
Unchecked dereference
3047766bc6ec9c Niklas Schnelle 2020-06-18 102 if (!pdev)
3047766bc6ec9c Niklas Schnelle 2020-06-18 103 break;
3047766bc6ec9c Niklas Schnelle 2020-06-18 104
3047766bc6ec9c Niklas Schnelle 2020-06-18 105 pci_bus_add_device(pdev);
3047766bc6ec9c Niklas Schnelle 2020-06-18 106 pci_lock_rescan_remove();
3047766bc6ec9c Niklas Schnelle 2020-06-18 107 pci_bus_add_devices(zdev->zbus->bus);
3047766bc6ec9c Niklas Schnelle 2020-06-18 108 pci_unlock_rescan_remove();
cbc0dd1f856b52 Jan Glauber 2012-11-29 109 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 110 case 0x0302: /* Reserved -> Standby */
f606b3ef47c9f8 Pierre Morel 2020-03-25 111 if (!zdev) {
d795ddad36cbc8 Sebastian Ott 2013-11-15 112 clp_add_pci_device(ccdf->fid, ccdf->fh, 0);
cbc0dd1f856b52 Jan Glauber 2012-11-29 113 break;
f606b3ef47c9f8 Pierre Morel 2020-03-25 114 }
f606b3ef47c9f8 Pierre Morel 2020-03-25 115 zdev->fh = ccdf->fh;
f606b3ef47c9f8 Pierre Morel 2020-03-25 116 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 117 case 0x0303: /* Deconfiguration requested */
623bd44d3f277b Sebastian Ott 2017-05-09 118 if (!zdev)
623bd44d3f277b Sebastian Ott 2017-05-09 119 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 120 if (pdev)
2a01bd1bd3d28d Sebastian Ott 2015-07-28 121 pci_stop_and_remove_bus_device_locked(pdev);
cbc0dd1f856b52 Jan Glauber 2012-11-29 122
d795ddad36cbc8 Sebastian Ott 2013-11-15 123 ret = zpci_disable_device(zdev);
d795ddad36cbc8 Sebastian Ott 2013-11-15 124 if (ret)
d795ddad36cbc8 Sebastian Ott 2013-11-15 125 break;
1f1dcbd4f23bd1 Sebastian Ott 2013-10-22 126
d795ddad36cbc8 Sebastian Ott 2013-11-15 127 ret = sclp_pci_deconfigure(zdev->fid);
d795ddad36cbc8 Sebastian Ott 2013-11-15 128 zpci_dbg(3, "deconf fid:%x, rc:%d\n", zdev->fid, ret);
d795ddad36cbc8 Sebastian Ott 2013-11-15 129 if (!ret)
d795ddad36cbc8 Sebastian Ott 2013-11-15 130 zdev->state = ZPCI_FN_STATE_STANDBY;
cbc0dd1f856b52 Jan Glauber 2012-11-29 131
d795ddad36cbc8 Sebastian Ott 2013-11-15 132 break;
623bd44d3f277b Sebastian Ott 2017-05-09 133 case 0x0304: /* Configured -> Standby|Reserved */
623bd44d3f277b Sebastian Ott 2017-05-09 134 if (!zdev)
623bd44d3f277b Sebastian Ott 2017-05-09 135 break;
0c0c2776926018 Sebastian Ott 2013-12-12 136 if (pdev) {
0c0c2776926018 Sebastian Ott 2013-12-12 137 /* Give the driver a hint that the function is
0c0c2776926018 Sebastian Ott 2013-12-12 138 * already unusable. */
0c0c2776926018 Sebastian Ott 2013-12-12 139 pdev->error_state = pci_channel_io_perm_failure;
2a01bd1bd3d28d Sebastian Ott 2015-07-28 140 pci_stop_and_remove_bus_device_locked(pdev);
0c0c2776926018 Sebastian Ott 2013-12-12 141 }
1f1dcbd4f23bd1 Sebastian Ott 2013-10-22 142
d795ddad36cbc8 Sebastian Ott 2013-11-15 143 zdev->state = ZPCI_FN_STATE_STANDBY;
623bd44d3f277b Sebastian Ott 2017-05-09 144 if (!clp_get_state(ccdf->fid, &state) &&
623bd44d3f277b Sebastian Ott 2017-05-09 145 state == ZPCI_FN_STATE_RESERVED) {
05bc1be6db4b26 Pierre Morel 2020-03-23 146 zpci_zdev_put(zdev);
623bd44d3f277b Sebastian Ott 2017-05-09 147 }
d795ddad36cbc8 Sebastian Ott 2013-11-15 148 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 149 case 0x0306: /* 0x308 or 0x302 for multiple devices */
d795ddad36cbc8 Sebastian Ott 2013-11-15 150 clp_rescan_pci_devices();
d795ddad36cbc8 Sebastian Ott 2013-11-15 151 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 152 case 0x0308: /* Standby -> Reserved */
704268925d32a0 Sebastian Ott 2013-12-12 153 if (!zdev)
704268925d32a0 Sebastian Ott 2013-12-12 154 break;
05bc1be6db4b26 Pierre Morel 2020-03-23 155 zpci_zdev_put(zdev);
d795ddad36cbc8 Sebastian Ott 2013-11-15 156 break;
d795ddad36cbc8 Sebastian Ott 2013-11-15 157 default:
d795ddad36cbc8 Sebastian Ott 2013-11-15 158 break;
cbc0dd1f856b52 Jan Glauber 2012-11-29 159 }
cbc0dd1f856b52 Jan Glauber 2012-11-29 160 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22803 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-19 6:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2020-12-03 11:48 ` Niklas Schnelle
2021-01-19 6:14 Dan Carpenter
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).