| Message ID | 20221028031709.3120927-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: fix handle error case in pci_alloc_child_bus() | expand |
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 }
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);
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(-)