* [PATCH] PCI: fix handle error case in pci_alloc_child_bus()
@ 2022-10-28 3:17 Yang Yingliang
2022-11-02 11:59 ` Dan Carpenter
0 siblings, 1 reply; 2+ messages in thread
From: Yang Yingliang @ 2022-10-28 3:17 UTC (permalink / raw)
To: linux-pci; +Cc: yinghai, bhelgaas, rafael.j.wysocki, yangyingliang
If device_register() returns error in pci_alloc_child_bus(), but it's not
handled, the 'bridge->subordinate' points a bus that is not registered,
it will lead kernel crash because of trying to delete unregistered device
in pci_remove_bus_device().
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000108
CPU: 48 PID: 38377 Comm: bash Kdump: loaded Tainted: G W 6.1.0-rc1+ #172
Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.58 10/24/2018
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : device_del+0x54/0x3d0
lr : device_del+0x37c/0x3d0
Call trace:
device_del+0x54/0x3d0
device_unregister+0x24/0x78
pci_remove_bus+0x90/0xa0
pci_remove_bus_device+0x128/0x138
pci_stop_and_remove_bus_device_locked+0x2c/0x40
remove_store+0xa4/0xb
Beside, the 'child' allocated by pci_alloc_bus() and the name allocated by
dev_set_name() need be freed, and also the refcount of bridge and of_node
which is get in pci_alloc_child_bus() need be put.
Fix these by setting 'bridge->subordinate' to NULL and calling put_device(),
if device_register() returns error, and return NULL in pci_alloc_child_bus().
Fixes: 4f535093cf8f ("PCI: Put pci_dev in device tree as early as possible")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/pci/probe.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 792dfee9cfd7..afd564f49f06 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1139,7 +1139,11 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
add_dev:
pci_set_bus_msi_domain(child);
ret = device_register(&child->dev);
- WARN_ON(ret < 0);
+ if (WARN_ON(ret < 0)) {
+ bridge->subordinate = NULL;
+ put_device(&child->dev);
+ return NULL;
+ }
pcibios_add_bus(child);
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] PCI: fix handle error case in pci_alloc_child_bus()
2022-10-28 3:17 [PATCH] PCI: fix handle error case in pci_alloc_child_bus() Yang Yingliang
@ 2022-11-02 11:59 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2022-11-02 11:59 UTC (permalink / raw)
To: oe-kbuild, Yang Yingliang, linux-pci
Cc: lkp, oe-kbuild-all, yinghai, bhelgaas, rafael.j.wysocki,
yangyingliang
Hi Yang,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yang-Yingliang/PCI-fix-handle-error-case-in-pci_alloc_child_bus/20221028-112049
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/20221028031709.3120927-1-yangyingliang%40huawei.com
patch subject: [PATCH] PCI: fix handle error case in pci_alloc_child_bus()
config: x86_64-randconfig-m001
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
smatch warnings:
drivers/pci/probe.c:1143 pci_alloc_child_bus() error: we previously assumed 'bridge' could be null (see line 1111)
vim +/bridge +1143 drivers/pci/probe.c
cbd4e055fc8f09 Adrian Bunk 2008-04-18 1076 static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
cbd4e055fc8f09 Adrian Bunk 2008-04-18 1077 struct pci_dev *bridge, int busnr)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1078 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1079 struct pci_bus *child;
07e292950b9368 Rob Herring 2020-08-20 1080 struct pci_host_bridge *host;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1081 int i;
4f535093cf8f6d Yinghai Lu 2013-01-21 1082 int ret;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1083
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1084 /* Allocate a new bus and inherit stuff from the parent */
670ba0c8883b57 Catalin Marinas 2014-09-29 1085 child = pci_alloc_bus(parent);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1086 if (!child)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1087 return NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1088
^1da177e4c3f41 Linus Torvalds 2005-04-16 1089 child->parent = parent;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1090 child->sysdata = parent->sysdata;
6e325a62a0a228 Michael S. Tsirkin 2006-02-14 1091 child->bus_flags = parent->bus_flags;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1092
07e292950b9368 Rob Herring 2020-08-20 1093 host = pci_find_host_bridge(parent);
07e292950b9368 Rob Herring 2020-08-20 1094 if (host->child_ops)
07e292950b9368 Rob Herring 2020-08-20 1095 child->ops = host->child_ops;
07e292950b9368 Rob Herring 2020-08-20 1096 else
07e292950b9368 Rob Herring 2020-08-20 1097 child->ops = parent->ops;
07e292950b9368 Rob Herring 2020-08-20 1098
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1099 /*
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1100 * Initialize some portions of the bus device, but don't register
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1101 * it now as the parent is not properly set up yet.
fd7d1ced29e5be Greg Kroah-Hartman 2007-05-22 1102 */
fd7d1ced29e5be Greg Kroah-Hartman 2007-05-22 1103 child->dev.class = &pcibus_class;
1a9271331ab663 Kay Sievers 2008-10-30 1104 dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1105
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1106 /* Set up the primary, secondary and subordinate bus numbers */
b918c62e086b21 Yinghai Lu 2012-05-17 1107 child->number = child->busn_res.start = busnr;
b918c62e086b21 Yinghai Lu 2012-05-17 1108 child->primary = parent->busn_res.start;
b918c62e086b21 Yinghai Lu 2012-05-17 1109 child->busn_res.end = 0xff;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1110
4f535093cf8f6d Yinghai Lu 2013-01-21 @1111 if (!bridge) {
^^^^^^
This code assumes bridge can be NULL.
4f535093cf8f6d Yinghai Lu 2013-01-21 1112 child->dev.parent = parent->bridge;
4f535093cf8f6d Yinghai Lu 2013-01-21 1113 goto add_dev;
4f535093cf8f6d Yinghai Lu 2013-01-21 1114 }
3789fa8a2e5345 Yu Zhao 2008-11-22 1115
3789fa8a2e5345 Yu Zhao 2008-11-22 1116 child->self = bridge;
3789fa8a2e5345 Yu Zhao 2008-11-22 1117 child->bridge = get_device(&bridge->dev);
4f535093cf8f6d Yinghai Lu 2013-01-21 1118 child->dev.parent = child->bridge;
98d9f30c820d50 Benjamin Herrenschmidt 2011-04-11 1119 pci_set_bus_of_node(child);
9be60ca0497a25 Matthew Wilcox 2009-12-13 1120 pci_set_bus_speed(child);
9be60ca0497a25 Matthew Wilcox 2009-12-13 1121
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1122 /*
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1123 * Check whether extended config space is accessible on the child
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1124 * bus. Note that we currently assume it is always accessible on
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1125 * the root bus.
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1126 */
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1127 if (!pci_bridge_child_ext_cfg_accessible(bridge)) {
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1128 child->bus_flags |= PCI_BUS_FLAGS_NO_EXTCFG;
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1129 pci_info(child, "extended config space not accessible\n");
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1130 }
17e8f0d4cee2bf Gilles Buloz 2018-05-03 1131
3e466e2d3a04c7 Bjorn Helgaas 2017-11-30 1132 /* Set up default resource pointers and names */
fde09c6d8f92de Yu Zhao 2008-11-22 1133 for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1134 child->resource[i] = &bridge->resource[PCI_BRIDGE_RESOURCES+i];
^1da177e4c3f41 Linus Torvalds 2005-04-16 1135 child->resource[i]->name = child->name;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1136 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1137 bridge->subordinate = child;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1138
4f535093cf8f6d Yinghai Lu 2013-01-21 1139 add_dev:
44aa0c657e3e45 Marc Zyngier 2015-07-28 1140 pci_set_bus_msi_domain(child);
4f535093cf8f6d Yinghai Lu 2013-01-21 1141 ret = device_register(&child->dev);
b1aa2ac592efcc Yang Yingliang 2022-10-28 1142 if (WARN_ON(ret < 0)) {
b1aa2ac592efcc Yang Yingliang 2022-10-28 @1143 bridge->subordinate = NULL;
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Unchecked dereference.
b1aa2ac592efcc Yang Yingliang 2022-10-28 1144 put_device(&child->dev);
b1aa2ac592efcc Yang Yingliang 2022-10-28 1145 return NULL;
b1aa2ac592efcc Yang Yingliang 2022-10-28 1146 }
4f535093cf8f6d Yinghai Lu 2013-01-21 1147
10a9574756201f Jiang Liu 2013-04-12 1148 pcibios_add_bus(child);
10a9574756201f Jiang Liu 2013-04-12 1149
057bd2e0528ec6 Thierry Reding 2016-02-09 1150 if (child->ops->add_bus) {
057bd2e0528ec6 Thierry Reding 2016-02-09 1151 ret = child->ops->add_bus(child);
057bd2e0528ec6 Thierry Reding 2016-02-09 1152 if (WARN_ON(ret < 0))
057bd2e0528ec6 Thierry Reding 2016-02-09 1153 dev_err(&child->dev, "failed to add bus: %d\n", ret);
057bd2e0528ec6 Thierry Reding 2016-02-09 1154 }
057bd2e0528ec6 Thierry Reding 2016-02-09 1155
4f535093cf8f6d Yinghai Lu 2013-01-21 1156 /* Create legacy_io and legacy_mem files for this bus */
4f535093cf8f6d Yinghai Lu 2013-01-21 1157 pci_create_legacy_files(child);
4f535093cf8f6d Yinghai Lu 2013-01-21 1158
^1da177e4c3f41 Linus Torvalds 2005-04-16 1159 return child;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1160 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-02 12:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-28 3:17 [PATCH] PCI: fix handle error case in pci_alloc_child_bus() Yang Yingliang
2022-11-02 11:59 ` 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).