* [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
@ 2018-05-11 5:08 Julia Lawall
2018-05-11 5:45 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2018-05-11 5:08 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Bjorn Helgaas, linux-pci, kbuild-all
The devm_kzalloc on line 324 makes the kfrees on lines 332 and 355
incorrect. They can just be removed.
julia
---------- Forwarded message ----------
Date: Fri, 11 May 2018 10:49:38 +0800
From: kbuild test robot <lkp@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free
of devm_ allocated data
CC: kbuild-all@01.org
CC: linux-pci@vger.kernel.org
TO: Jan Kiszka <jan.kiszka@siemens.com>
CC: Bjorn Helgaas <helgaas@kernel.org>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/resource
head: f2837b7a1764f7b58d2febec2fe37fb0cd99de24
commit: 67ed62eb921e31fd51c1d83ca6313b9aa3cead46 [8/10] PCI: Remove unused of_pci_get_host_bridge_resources()
:::::: branch date: 4 hours ago
:::::: commit date: 4 hours ago
>> drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
drivers/pci/of.c:355:1-6: WARNING: invalid free of devm_ allocated data
# https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=67ed62eb921e31fd51c1d83ca6313b9aa3cead46
git remote add pci https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
git remote update pci
git checkout 67ed62eb921e31fd51c1d83ca6313b9aa3cead46
vim +332 drivers/pci/of.c
4670d610d Rob Herring 2018-01-17 244
4670d610d Rob Herring 2018-01-17 245 #if defined(CONFIG_OF_ADDRESS)
67ed62eb9 Jan Kiszka 2018-05-09 246 /**
67ed62eb9 Jan Kiszka 2018-05-09 247 * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
67ed62eb9 Jan Kiszka 2018-05-09 248 * host bridge resources from DT
67ed62eb9 Jan Kiszka 2018-05-09 249 * @dev: host bridge device
67ed62eb9 Jan Kiszka 2018-05-09 250 * @busno: bus number associated with the bridge root bus
67ed62eb9 Jan Kiszka 2018-05-09 251 * @bus_max: maximum number of buses for this bridge
67ed62eb9 Jan Kiszka 2018-05-09 252 * @resources: list where the range of resources will be added after DT parsing
67ed62eb9 Jan Kiszka 2018-05-09 253 * @io_base: pointer to a variable that will contain on return the physical
67ed62eb9 Jan Kiszka 2018-05-09 254 * address for the start of the I/O range. Can be NULL if the caller doesn't
67ed62eb9 Jan Kiszka 2018-05-09 255 * expect I/O ranges to be present in the device tree.
67ed62eb9 Jan Kiszka 2018-05-09 256 *
67ed62eb9 Jan Kiszka 2018-05-09 257 * This function will parse the "ranges" property of a PCI host bridge device
67ed62eb9 Jan Kiszka 2018-05-09 258 * node and setup the resource mapping based on its content. It is expected
67ed62eb9 Jan Kiszka 2018-05-09 259 * that the property conforms with the Power ePAPR document.
67ed62eb9 Jan Kiszka 2018-05-09 260 *
67ed62eb9 Jan Kiszka 2018-05-09 261 * It returns zero if the range parsing has been successful or a standard error
67ed62eb9 Jan Kiszka 2018-05-09 262 * value if it failed.
67ed62eb9 Jan Kiszka 2018-05-09 263 */
67ed62eb9 Jan Kiszka 2018-05-09 264 int devm_of_pci_get_host_bridge_resources(struct device *dev,
4670d610d Rob Herring 2018-01-17 265 unsigned char busno, unsigned char bus_max,
4670d610d Rob Herring 2018-01-17 266 struct list_head *resources, resource_size_t *io_base)
4670d610d Rob Herring 2018-01-17 267 {
67ed62eb9 Jan Kiszka 2018-05-09 268 struct device_node *dev_node = dev->of_node;
4670d610d Rob Herring 2018-01-17 269 struct resource_entry *window;
4670d610d Rob Herring 2018-01-17 270 struct resource *res;
4670d610d Rob Herring 2018-01-17 271 struct resource *bus_range;
4670d610d Rob Herring 2018-01-17 272 struct of_pci_range range;
4670d610d Rob Herring 2018-01-17 273 struct of_pci_range_parser parser;
4670d610d Rob Herring 2018-01-17 274 char range_type[4];
4670d610d Rob Herring 2018-01-17 275 int err;
4670d610d Rob Herring 2018-01-17 276
4670d610d Rob Herring 2018-01-17 277 if (io_base)
4670d610d Rob Herring 2018-01-17 278 *io_base = (resource_size_t)OF_BAD_ADDR;
4670d610d Rob Herring 2018-01-17 279
07adab611 Jan Kiszka 2018-04-30 280 bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
4670d610d Rob Herring 2018-01-17 281 if (!bus_range)
4670d610d Rob Herring 2018-01-17 282 return -ENOMEM;
4670d610d Rob Herring 2018-01-17 283
877a66649 Jan Kiszka 2018-04-30 284 dev_info(dev, "host bridge %pOF ranges:\n", dev_node);
4670d610d Rob Herring 2018-01-17 285
08c1b3b10 Jan Kiszka 2018-04-30 286 err = of_pci_parse_bus_range(dev_node, bus_range);
4670d610d Rob Herring 2018-01-17 287 if (err) {
4670d610d Rob Herring 2018-01-17 288 bus_range->start = busno;
4670d610d Rob Herring 2018-01-17 289 bus_range->end = bus_max;
4670d610d Rob Herring 2018-01-17 290 bus_range->flags = IORESOURCE_BUS;
877a66649 Jan Kiszka 2018-04-30 291 dev_info(dev, " No bus range found for %pOF, using %pR\n",
08c1b3b10 Jan Kiszka 2018-04-30 292 dev_node, bus_range);
4670d610d Rob Herring 2018-01-17 293 } else {
4670d610d Rob Herring 2018-01-17 294 if (bus_range->end > bus_range->start + bus_max)
4670d610d Rob Herring 2018-01-17 295 bus_range->end = bus_range->start + bus_max;
4670d610d Rob Herring 2018-01-17 296 }
4670d610d Rob Herring 2018-01-17 297 pci_add_resource(resources, bus_range);
4670d610d Rob Herring 2018-01-17 298
4670d610d Rob Herring 2018-01-17 299 /* Check for ranges property */
08c1b3b10 Jan Kiszka 2018-04-30 300 err = of_pci_range_parser_init(&parser, dev_node);
4670d610d Rob Herring 2018-01-17 301 if (err)
4670d610d Rob Herring 2018-01-17 302 goto parse_failed;
4670d610d Rob Herring 2018-01-17 303
877a66649 Jan Kiszka 2018-04-30 304 dev_dbg(dev, "Parsing ranges property...\n");
4670d610d Rob Herring 2018-01-17 305 for_each_of_pci_range(&parser, &range) {
4670d610d Rob Herring 2018-01-17 306 /* Read next ranges element */
4670d610d Rob Herring 2018-01-17 307 if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
4670d610d Rob Herring 2018-01-17 308 snprintf(range_type, 4, " IO");
4670d610d Rob Herring 2018-01-17 309 else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
4670d610d Rob Herring 2018-01-17 310 snprintf(range_type, 4, "MEM");
4670d610d Rob Herring 2018-01-17 311 else
4670d610d Rob Herring 2018-01-17 312 snprintf(range_type, 4, "err");
877a66649 Jan Kiszka 2018-04-30 313 dev_info(dev, " %s %#010llx..%#010llx -> %#010llx\n",
877a66649 Jan Kiszka 2018-04-30 314 range_type, range.cpu_addr,
877a66649 Jan Kiszka 2018-04-30 315 range.cpu_addr + range.size - 1, range.pci_addr);
4670d610d Rob Herring 2018-01-17 316
4670d610d Rob Herring 2018-01-17 317 /*
4670d610d Rob Herring 2018-01-17 318 * If we failed translation or got a zero-sized region
4670d610d Rob Herring 2018-01-17 319 * then skip this range
4670d610d Rob Herring 2018-01-17 320 */
4670d610d Rob Herring 2018-01-17 321 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
4670d610d Rob Herring 2018-01-17 322 continue;
4670d610d Rob Herring 2018-01-17 323
67ed62eb9 Jan Kiszka 2018-05-09 324 res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
4670d610d Rob Herring 2018-01-17 325 if (!res) {
4670d610d Rob Herring 2018-01-17 326 err = -ENOMEM;
4670d610d Rob Herring 2018-01-17 327 goto parse_failed;
4670d610d Rob Herring 2018-01-17 328 }
4670d610d Rob Herring 2018-01-17 329
08c1b3b10 Jan Kiszka 2018-04-30 330 err = of_pci_range_to_resource(&range, dev_node, res);
4670d610d Rob Herring 2018-01-17 331 if (err) {
4670d610d Rob Herring 2018-01-17 @332 kfree(res);
4670d610d Rob Herring 2018-01-17 333 continue;
4670d610d Rob Herring 2018-01-17 334 }
4670d610d Rob Herring 2018-01-17 335
4670d610d Rob Herring 2018-01-17 336 if (resource_type(res) == IORESOURCE_IO) {
4670d610d Rob Herring 2018-01-17 337 if (!io_base) {
877a66649 Jan Kiszka 2018-04-30 338 dev_err(dev, "I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
08c1b3b10 Jan Kiszka 2018-04-30 339 dev_node);
4670d610d Rob Herring 2018-01-17 340 err = -EINVAL;
4670d610d Rob Herring 2018-01-17 341 goto conversion_failed;
4670d610d Rob Herring 2018-01-17 342 }
4670d610d Rob Herring 2018-01-17 343 if (*io_base != (resource_size_t)OF_BAD_ADDR)
877a66649 Jan Kiszka 2018-04-30 344 dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
08c1b3b10 Jan Kiszka 2018-04-30 345 dev_node);
4670d610d Rob Herring 2018-01-17 346 *io_base = range.cpu_addr;
4670d610d Rob Herring 2018-01-17 347 }
4670d610d Rob Herring 2018-01-17 348
4670d610d Rob Herring 2018-01-17 349 pci_add_resource_offset(resources, res, res->start - range.pci_addr);
4670d610d Rob Herring 2018-01-17 350 }
4670d610d Rob Herring 2018-01-17 351
4670d610d Rob Herring 2018-01-17 352 return 0;
4670d610d Rob Herring 2018-01-17 353
4670d610d Rob Herring 2018-01-17 354 conversion_failed:
4670d610d Rob Herring 2018-01-17 355 kfree(res);
4670d610d Rob Herring 2018-01-17 356 parse_failed:
4670d610d Rob Herring 2018-01-17 357 resource_list_for_each_entry(window, resources)
4670d610d Rob Herring 2018-01-17 358 kfree(window->res);
4670d610d Rob Herring 2018-01-17 359 pci_free_resource_list(resources);
4670d610d Rob Herring 2018-01-17 360 return err;
4670d610d Rob Herring 2018-01-17 361 }
83d525c0f Jan Kiszka 2018-04-30 362 EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
4670d610d Rob Herring 2018-01-17 363 #endif /* CONFIG_OF_ADDRESS */
4670d610d Rob Herring 2018-01-17 364
:::::: The code at line 332 was first introduced by commit
:::::: 4670d610d59233b017a6ea1fa25bbf06dabbff42 PCI: Move OF-related PCI functions into PCI core
:::::: TO: Rob Herring <robh@kernel.org>
:::::: CC: Bjorn Helgaas <bhelgaas@google.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
2018-05-11 5:08 [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data Julia Lawall
@ 2018-05-11 5:45 ` Jan Kiszka
2018-05-11 5:53 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2018-05-11 5:45 UTC (permalink / raw)
To: Julia Lawall, Bjorn Helgaas; +Cc: linux-pci, kbuild-all
On 2018-05-11 07:08, Julia Lawall wrote:
> The devm_kzalloc on line 324 makes the kfrees on lines 332 and 355
> incorrect. They can just be removed.
True. In fact, 358 and 359 should go as well - making them obsolete is
the whole point of this conversion.
The issue comes already with patch "Add dev parameter to
__of_pci_get_host_bridge_resources()" when dev != NULL. Bjorn, should I
resend the series with that patch fixed and the rest rebased? Or do you
prefer when I update only this patch, fixing the issue here?
Jan
>
> julia
>
> ---------- Forwarded message ----------
> Date: Fri, 11 May 2018 10:49:38 +0800
> From: kbuild test robot <lkp@intel.com>
> To: kbuild@01.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Subject: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free
> of devm_ allocated data
>
> CC: kbuild-all@01.org
> CC: linux-pci@vger.kernel.org
> TO: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Bjorn Helgaas <helgaas@kernel.org>
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/resource
> head: f2837b7a1764f7b58d2febec2fe37fb0cd99de24
> commit: 67ed62eb921e31fd51c1d83ca6313b9aa3cead46 [8/10] PCI: Remove unused of_pci_get_host_bridge_resources()
> :::::: branch date: 4 hours ago
> :::::: commit date: 4 hours ago
>
>>> drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
> drivers/pci/of.c:355:1-6: WARNING: invalid free of devm_ allocated data
>
> # https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=67ed62eb921e31fd51c1d83ca6313b9aa3cead46
> git remote add pci https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
> git remote update pci
> git checkout 67ed62eb921e31fd51c1d83ca6313b9aa3cead46
> vim +332 drivers/pci/of.c
>
> 4670d610d Rob Herring 2018-01-17 244
> 4670d610d Rob Herring 2018-01-17 245 #if defined(CONFIG_OF_ADDRESS)
> 67ed62eb9 Jan Kiszka 2018-05-09 246 /**
> 67ed62eb9 Jan Kiszka 2018-05-09 247 * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
> 67ed62eb9 Jan Kiszka 2018-05-09 248 * host bridge resources from DT
> 67ed62eb9 Jan Kiszka 2018-05-09 249 * @dev: host bridge device
> 67ed62eb9 Jan Kiszka 2018-05-09 250 * @busno: bus number associated with the bridge root bus
> 67ed62eb9 Jan Kiszka 2018-05-09 251 * @bus_max: maximum number of buses for this bridge
> 67ed62eb9 Jan Kiszka 2018-05-09 252 * @resources: list where the range of resources will be added after DT parsing
> 67ed62eb9 Jan Kiszka 2018-05-09 253 * @io_base: pointer to a variable that will contain on return the physical
> 67ed62eb9 Jan Kiszka 2018-05-09 254 * address for the start of the I/O range. Can be NULL if the caller doesn't
> 67ed62eb9 Jan Kiszka 2018-05-09 255 * expect I/O ranges to be present in the device tree.
> 67ed62eb9 Jan Kiszka 2018-05-09 256 *
> 67ed62eb9 Jan Kiszka 2018-05-09 257 * This function will parse the "ranges" property of a PCI host bridge device
> 67ed62eb9 Jan Kiszka 2018-05-09 258 * node and setup the resource mapping based on its content. It is expected
> 67ed62eb9 Jan Kiszka 2018-05-09 259 * that the property conforms with the Power ePAPR document.
> 67ed62eb9 Jan Kiszka 2018-05-09 260 *
> 67ed62eb9 Jan Kiszka 2018-05-09 261 * It returns zero if the range parsing has been successful or a standard error
> 67ed62eb9 Jan Kiszka 2018-05-09 262 * value if it failed.
> 67ed62eb9 Jan Kiszka 2018-05-09 263 */
> 67ed62eb9 Jan Kiszka 2018-05-09 264 int devm_of_pci_get_host_bridge_resources(struct device *dev,
> 4670d610d Rob Herring 2018-01-17 265 unsigned char busno, unsigned char bus_max,
> 4670d610d Rob Herring 2018-01-17 266 struct list_head *resources, resource_size_t *io_base)
> 4670d610d Rob Herring 2018-01-17 267 {
> 67ed62eb9 Jan Kiszka 2018-05-09 268 struct device_node *dev_node = dev->of_node;
> 4670d610d Rob Herring 2018-01-17 269 struct resource_entry *window;
> 4670d610d Rob Herring 2018-01-17 270 struct resource *res;
> 4670d610d Rob Herring 2018-01-17 271 struct resource *bus_range;
> 4670d610d Rob Herring 2018-01-17 272 struct of_pci_range range;
> 4670d610d Rob Herring 2018-01-17 273 struct of_pci_range_parser parser;
> 4670d610d Rob Herring 2018-01-17 274 char range_type[4];
> 4670d610d Rob Herring 2018-01-17 275 int err;
> 4670d610d Rob Herring 2018-01-17 276
> 4670d610d Rob Herring 2018-01-17 277 if (io_base)
> 4670d610d Rob Herring 2018-01-17 278 *io_base = (resource_size_t)OF_BAD_ADDR;
> 4670d610d Rob Herring 2018-01-17 279
> 07adab611 Jan Kiszka 2018-04-30 280 bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
> 4670d610d Rob Herring 2018-01-17 281 if (!bus_range)
> 4670d610d Rob Herring 2018-01-17 282 return -ENOMEM;
> 4670d610d Rob Herring 2018-01-17 283
> 877a66649 Jan Kiszka 2018-04-30 284 dev_info(dev, "host bridge %pOF ranges:\n", dev_node);
> 4670d610d Rob Herring 2018-01-17 285
> 08c1b3b10 Jan Kiszka 2018-04-30 286 err = of_pci_parse_bus_range(dev_node, bus_range);
> 4670d610d Rob Herring 2018-01-17 287 if (err) {
> 4670d610d Rob Herring 2018-01-17 288 bus_range->start = busno;
> 4670d610d Rob Herring 2018-01-17 289 bus_range->end = bus_max;
> 4670d610d Rob Herring 2018-01-17 290 bus_range->flags = IORESOURCE_BUS;
> 877a66649 Jan Kiszka 2018-04-30 291 dev_info(dev, " No bus range found for %pOF, using %pR\n",
> 08c1b3b10 Jan Kiszka 2018-04-30 292 dev_node, bus_range);
> 4670d610d Rob Herring 2018-01-17 293 } else {
> 4670d610d Rob Herring 2018-01-17 294 if (bus_range->end > bus_range->start + bus_max)
> 4670d610d Rob Herring 2018-01-17 295 bus_range->end = bus_range->start + bus_max;
> 4670d610d Rob Herring 2018-01-17 296 }
> 4670d610d Rob Herring 2018-01-17 297 pci_add_resource(resources, bus_range);
> 4670d610d Rob Herring 2018-01-17 298
> 4670d610d Rob Herring 2018-01-17 299 /* Check for ranges property */
> 08c1b3b10 Jan Kiszka 2018-04-30 300 err = of_pci_range_parser_init(&parser, dev_node);
> 4670d610d Rob Herring 2018-01-17 301 if (err)
> 4670d610d Rob Herring 2018-01-17 302 goto parse_failed;
> 4670d610d Rob Herring 2018-01-17 303
> 877a66649 Jan Kiszka 2018-04-30 304 dev_dbg(dev, "Parsing ranges property...\n");
> 4670d610d Rob Herring 2018-01-17 305 for_each_of_pci_range(&parser, &range) {
> 4670d610d Rob Herring 2018-01-17 306 /* Read next ranges element */
> 4670d610d Rob Herring 2018-01-17 307 if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
> 4670d610d Rob Herring 2018-01-17 308 snprintf(range_type, 4, " IO");
> 4670d610d Rob Herring 2018-01-17 309 else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
> 4670d610d Rob Herring 2018-01-17 310 snprintf(range_type, 4, "MEM");
> 4670d610d Rob Herring 2018-01-17 311 else
> 4670d610d Rob Herring 2018-01-17 312 snprintf(range_type, 4, "err");
> 877a66649 Jan Kiszka 2018-04-30 313 dev_info(dev, " %s %#010llx..%#010llx -> %#010llx\n",
> 877a66649 Jan Kiszka 2018-04-30 314 range_type, range.cpu_addr,
> 877a66649 Jan Kiszka 2018-04-30 315 range.cpu_addr + range.size - 1, range.pci_addr);
> 4670d610d Rob Herring 2018-01-17 316
> 4670d610d Rob Herring 2018-01-17 317 /*
> 4670d610d Rob Herring 2018-01-17 318 * If we failed translation or got a zero-sized region
> 4670d610d Rob Herring 2018-01-17 319 * then skip this range
> 4670d610d Rob Herring 2018-01-17 320 */
> 4670d610d Rob Herring 2018-01-17 321 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> 4670d610d Rob Herring 2018-01-17 322 continue;
> 4670d610d Rob Herring 2018-01-17 323
> 67ed62eb9 Jan Kiszka 2018-05-09 324 res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> 4670d610d Rob Herring 2018-01-17 325 if (!res) {
> 4670d610d Rob Herring 2018-01-17 326 err = -ENOMEM;
> 4670d610d Rob Herring 2018-01-17 327 goto parse_failed;
> 4670d610d Rob Herring 2018-01-17 328 }
> 4670d610d Rob Herring 2018-01-17 329
> 08c1b3b10 Jan Kiszka 2018-04-30 330 err = of_pci_range_to_resource(&range, dev_node, res);
> 4670d610d Rob Herring 2018-01-17 331 if (err) {
> 4670d610d Rob Herring 2018-01-17 @332 kfree(res);
> 4670d610d Rob Herring 2018-01-17 333 continue;
> 4670d610d Rob Herring 2018-01-17 334 }
> 4670d610d Rob Herring 2018-01-17 335
> 4670d610d Rob Herring 2018-01-17 336 if (resource_type(res) == IORESOURCE_IO) {
> 4670d610d Rob Herring 2018-01-17 337 if (!io_base) {
> 877a66649 Jan Kiszka 2018-04-30 338 dev_err(dev, "I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
> 08c1b3b10 Jan Kiszka 2018-04-30 339 dev_node);
> 4670d610d Rob Herring 2018-01-17 340 err = -EINVAL;
> 4670d610d Rob Herring 2018-01-17 341 goto conversion_failed;
> 4670d610d Rob Herring 2018-01-17 342 }
> 4670d610d Rob Herring 2018-01-17 343 if (*io_base != (resource_size_t)OF_BAD_ADDR)
> 877a66649 Jan Kiszka 2018-04-30 344 dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
> 08c1b3b10 Jan Kiszka 2018-04-30 345 dev_node);
> 4670d610d Rob Herring 2018-01-17 346 *io_base = range.cpu_addr;
> 4670d610d Rob Herring 2018-01-17 347 }
> 4670d610d Rob Herring 2018-01-17 348
> 4670d610d Rob Herring 2018-01-17 349 pci_add_resource_offset(resources, res, res->start - range.pci_addr);
> 4670d610d Rob Herring 2018-01-17 350 }
> 4670d610d Rob Herring 2018-01-17 351
> 4670d610d Rob Herring 2018-01-17 352 return 0;
> 4670d610d Rob Herring 2018-01-17 353
> 4670d610d Rob Herring 2018-01-17 354 conversion_failed:
> 4670d610d Rob Herring 2018-01-17 355 kfree(res);
> 4670d610d Rob Herring 2018-01-17 356 parse_failed:
> 4670d610d Rob Herring 2018-01-17 357 resource_list_for_each_entry(window, resources)
> 4670d610d Rob Herring 2018-01-17 358 kfree(window->res);
> 4670d610d Rob Herring 2018-01-17 359 pci_free_resource_list(resources);
> 4670d610d Rob Herring 2018-01-17 360 return err;
> 4670d610d Rob Herring 2018-01-17 361 }
> 83d525c0f Jan Kiszka 2018-04-30 362 EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
> 4670d610d Rob Herring 2018-01-17 363 #endif /* CONFIG_OF_ADDRESS */
> 4670d610d Rob Herring 2018-01-17 364
>
> :::::: The code at line 332 was first introduced by commit
> :::::: 4670d610d59233b017a6ea1fa25bbf06dabbff42 PCI: Move OF-related PCI functions into PCI core
>
> :::::: TO: Rob Herring <robh@kernel.org>
> :::::: CC: Bjorn Helgaas <bhelgaas@google.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
2018-05-11 5:45 ` Jan Kiszka
@ 2018-05-11 5:53 ` Jan Kiszka
2018-05-11 22:27 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2018-05-11 5:53 UTC (permalink / raw)
To: Julia Lawall, Bjorn Helgaas; +Cc: linux-pci, kbuild-all
On 2018-05-11 07:45, Jan Kiszka wrote:
> On 2018-05-11 07:08, Julia Lawall wrote:
>> The devm_kzalloc on line 324 makes the kfrees on lines 332 and 355
>> incorrect. They can just be removed.
>
> True. In fact, 358 and 359 should go as well - making them obsolete is
> the whole point of this conversion.
More precisely, 332 needs to become devm_kfree because we skip over one
resource without failing.
>
> The issue comes already with patch "Add dev parameter to
> __of_pci_get_host_bridge_resources()" when dev != NULL. Bjorn, should I
> resend the series with that patch fixed and the rest rebased? Or do you
> prefer when I update only this patch, fixing the issue here?
...or is a fixup patch on top preferred?
Jan
>
> Jan
>
>>
>> julia
>>
>> ---------- Forwarded message ----------
>> Date: Fri, 11 May 2018 10:49:38 +0800
>> From: kbuild test robot <lkp@intel.com>
>> To: kbuild@01.org
>> Cc: Julia Lawall <julia.lawall@lip6.fr>
>> Subject: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free
>> of devm_ allocated data
>>
>> CC: kbuild-all@01.org
>> CC: linux-pci@vger.kernel.org
>> TO: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Bjorn Helgaas <helgaas@kernel.org>
>>
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/resource
>> head: f2837b7a1764f7b58d2febec2fe37fb0cd99de24
>> commit: 67ed62eb921e31fd51c1d83ca6313b9aa3cead46 [8/10] PCI: Remove unused of_pci_get_host_bridge_resources()
>> :::::: branch date: 4 hours ago
>> :::::: commit date: 4 hours ago
>>
>>>> drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
>> drivers/pci/of.c:355:1-6: WARNING: invalid free of devm_ allocated data
>>
>> # https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=67ed62eb921e31fd51c1d83ca6313b9aa3cead46
>> git remote add pci https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
>> git remote update pci
>> git checkout 67ed62eb921e31fd51c1d83ca6313b9aa3cead46
>> vim +332 drivers/pci/of.c
>>
>> 4670d610d Rob Herring 2018-01-17 244
>> 4670d610d Rob Herring 2018-01-17 245 #if defined(CONFIG_OF_ADDRESS)
>> 67ed62eb9 Jan Kiszka 2018-05-09 246 /**
>> 67ed62eb9 Jan Kiszka 2018-05-09 247 * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
>> 67ed62eb9 Jan Kiszka 2018-05-09 248 * host bridge resources from DT
>> 67ed62eb9 Jan Kiszka 2018-05-09 249 * @dev: host bridge device
>> 67ed62eb9 Jan Kiszka 2018-05-09 250 * @busno: bus number associated with the bridge root bus
>> 67ed62eb9 Jan Kiszka 2018-05-09 251 * @bus_max: maximum number of buses for this bridge
>> 67ed62eb9 Jan Kiszka 2018-05-09 252 * @resources: list where the range of resources will be added after DT parsing
>> 67ed62eb9 Jan Kiszka 2018-05-09 253 * @io_base: pointer to a variable that will contain on return the physical
>> 67ed62eb9 Jan Kiszka 2018-05-09 254 * address for the start of the I/O range. Can be NULL if the caller doesn't
>> 67ed62eb9 Jan Kiszka 2018-05-09 255 * expect I/O ranges to be present in the device tree.
>> 67ed62eb9 Jan Kiszka 2018-05-09 256 *
>> 67ed62eb9 Jan Kiszka 2018-05-09 257 * This function will parse the "ranges" property of a PCI host bridge device
>> 67ed62eb9 Jan Kiszka 2018-05-09 258 * node and setup the resource mapping based on its content. It is expected
>> 67ed62eb9 Jan Kiszka 2018-05-09 259 * that the property conforms with the Power ePAPR document.
>> 67ed62eb9 Jan Kiszka 2018-05-09 260 *
>> 67ed62eb9 Jan Kiszka 2018-05-09 261 * It returns zero if the range parsing has been successful or a standard error
>> 67ed62eb9 Jan Kiszka 2018-05-09 262 * value if it failed.
>> 67ed62eb9 Jan Kiszka 2018-05-09 263 */
>> 67ed62eb9 Jan Kiszka 2018-05-09 264 int devm_of_pci_get_host_bridge_resources(struct device *dev,
>> 4670d610d Rob Herring 2018-01-17 265 unsigned char busno, unsigned char bus_max,
>> 4670d610d Rob Herring 2018-01-17 266 struct list_head *resources, resource_size_t *io_base)
>> 4670d610d Rob Herring 2018-01-17 267 {
>> 67ed62eb9 Jan Kiszka 2018-05-09 268 struct device_node *dev_node = dev->of_node;
>> 4670d610d Rob Herring 2018-01-17 269 struct resource_entry *window;
>> 4670d610d Rob Herring 2018-01-17 270 struct resource *res;
>> 4670d610d Rob Herring 2018-01-17 271 struct resource *bus_range;
>> 4670d610d Rob Herring 2018-01-17 272 struct of_pci_range range;
>> 4670d610d Rob Herring 2018-01-17 273 struct of_pci_range_parser parser;
>> 4670d610d Rob Herring 2018-01-17 274 char range_type[4];
>> 4670d610d Rob Herring 2018-01-17 275 int err;
>> 4670d610d Rob Herring 2018-01-17 276
>> 4670d610d Rob Herring 2018-01-17 277 if (io_base)
>> 4670d610d Rob Herring 2018-01-17 278 *io_base = (resource_size_t)OF_BAD_ADDR;
>> 4670d610d Rob Herring 2018-01-17 279
>> 07adab611 Jan Kiszka 2018-04-30 280 bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
>> 4670d610d Rob Herring 2018-01-17 281 if (!bus_range)
>> 4670d610d Rob Herring 2018-01-17 282 return -ENOMEM;
>> 4670d610d Rob Herring 2018-01-17 283
>> 877a66649 Jan Kiszka 2018-04-30 284 dev_info(dev, "host bridge %pOF ranges:\n", dev_node);
>> 4670d610d Rob Herring 2018-01-17 285
>> 08c1b3b10 Jan Kiszka 2018-04-30 286 err = of_pci_parse_bus_range(dev_node, bus_range);
>> 4670d610d Rob Herring 2018-01-17 287 if (err) {
>> 4670d610d Rob Herring 2018-01-17 288 bus_range->start = busno;
>> 4670d610d Rob Herring 2018-01-17 289 bus_range->end = bus_max;
>> 4670d610d Rob Herring 2018-01-17 290 bus_range->flags = IORESOURCE_BUS;
>> 877a66649 Jan Kiszka 2018-04-30 291 dev_info(dev, " No bus range found for %pOF, using %pR\n",
>> 08c1b3b10 Jan Kiszka 2018-04-30 292 dev_node, bus_range);
>> 4670d610d Rob Herring 2018-01-17 293 } else {
>> 4670d610d Rob Herring 2018-01-17 294 if (bus_range->end > bus_range->start + bus_max)
>> 4670d610d Rob Herring 2018-01-17 295 bus_range->end = bus_range->start + bus_max;
>> 4670d610d Rob Herring 2018-01-17 296 }
>> 4670d610d Rob Herring 2018-01-17 297 pci_add_resource(resources, bus_range);
>> 4670d610d Rob Herring 2018-01-17 298
>> 4670d610d Rob Herring 2018-01-17 299 /* Check for ranges property */
>> 08c1b3b10 Jan Kiszka 2018-04-30 300 err = of_pci_range_parser_init(&parser, dev_node);
>> 4670d610d Rob Herring 2018-01-17 301 if (err)
>> 4670d610d Rob Herring 2018-01-17 302 goto parse_failed;
>> 4670d610d Rob Herring 2018-01-17 303
>> 877a66649 Jan Kiszka 2018-04-30 304 dev_dbg(dev, "Parsing ranges property...\n");
>> 4670d610d Rob Herring 2018-01-17 305 for_each_of_pci_range(&parser, &range) {
>> 4670d610d Rob Herring 2018-01-17 306 /* Read next ranges element */
>> 4670d610d Rob Herring 2018-01-17 307 if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
>> 4670d610d Rob Herring 2018-01-17 308 snprintf(range_type, 4, " IO");
>> 4670d610d Rob Herring 2018-01-17 309 else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
>> 4670d610d Rob Herring 2018-01-17 310 snprintf(range_type, 4, "MEM");
>> 4670d610d Rob Herring 2018-01-17 311 else
>> 4670d610d Rob Herring 2018-01-17 312 snprintf(range_type, 4, "err");
>> 877a66649 Jan Kiszka 2018-04-30 313 dev_info(dev, " %s %#010llx..%#010llx -> %#010llx\n",
>> 877a66649 Jan Kiszka 2018-04-30 314 range_type, range.cpu_addr,
>> 877a66649 Jan Kiszka 2018-04-30 315 range.cpu_addr + range.size - 1, range.pci_addr);
>> 4670d610d Rob Herring 2018-01-17 316
>> 4670d610d Rob Herring 2018-01-17 317 /*
>> 4670d610d Rob Herring 2018-01-17 318 * If we failed translation or got a zero-sized region
>> 4670d610d Rob Herring 2018-01-17 319 * then skip this range
>> 4670d610d Rob Herring 2018-01-17 320 */
>> 4670d610d Rob Herring 2018-01-17 321 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>> 4670d610d Rob Herring 2018-01-17 322 continue;
>> 4670d610d Rob Herring 2018-01-17 323
>> 67ed62eb9 Jan Kiszka 2018-05-09 324 res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
>> 4670d610d Rob Herring 2018-01-17 325 if (!res) {
>> 4670d610d Rob Herring 2018-01-17 326 err = -ENOMEM;
>> 4670d610d Rob Herring 2018-01-17 327 goto parse_failed;
>> 4670d610d Rob Herring 2018-01-17 328 }
>> 4670d610d Rob Herring 2018-01-17 329
>> 08c1b3b10 Jan Kiszka 2018-04-30 330 err = of_pci_range_to_resource(&range, dev_node, res);
>> 4670d610d Rob Herring 2018-01-17 331 if (err) {
>> 4670d610d Rob Herring 2018-01-17 @332 kfree(res);
>> 4670d610d Rob Herring 2018-01-17 333 continue;
>> 4670d610d Rob Herring 2018-01-17 334 }
>> 4670d610d Rob Herring 2018-01-17 335
>> 4670d610d Rob Herring 2018-01-17 336 if (resource_type(res) == IORESOURCE_IO) {
>> 4670d610d Rob Herring 2018-01-17 337 if (!io_base) {
>> 877a66649 Jan Kiszka 2018-04-30 338 dev_err(dev, "I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
>> 08c1b3b10 Jan Kiszka 2018-04-30 339 dev_node);
>> 4670d610d Rob Herring 2018-01-17 340 err = -EINVAL;
>> 4670d610d Rob Herring 2018-01-17 341 goto conversion_failed;
>> 4670d610d Rob Herring 2018-01-17 342 }
>> 4670d610d Rob Herring 2018-01-17 343 if (*io_base != (resource_size_t)OF_BAD_ADDR)
>> 877a66649 Jan Kiszka 2018-04-30 344 dev_warn(dev, "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
>> 08c1b3b10 Jan Kiszka 2018-04-30 345 dev_node);
>> 4670d610d Rob Herring 2018-01-17 346 *io_base = range.cpu_addr;
>> 4670d610d Rob Herring 2018-01-17 347 }
>> 4670d610d Rob Herring 2018-01-17 348
>> 4670d610d Rob Herring 2018-01-17 349 pci_add_resource_offset(resources, res, res->start - range.pci_addr);
>> 4670d610d Rob Herring 2018-01-17 350 }
>> 4670d610d Rob Herring 2018-01-17 351
>> 4670d610d Rob Herring 2018-01-17 352 return 0;
>> 4670d610d Rob Herring 2018-01-17 353
>> 4670d610d Rob Herring 2018-01-17 354 conversion_failed:
>> 4670d610d Rob Herring 2018-01-17 355 kfree(res);
>> 4670d610d Rob Herring 2018-01-17 356 parse_failed:
>> 4670d610d Rob Herring 2018-01-17 357 resource_list_for_each_entry(window, resources)
>> 4670d610d Rob Herring 2018-01-17 358 kfree(window->res);
>> 4670d610d Rob Herring 2018-01-17 359 pci_free_resource_list(resources);
>> 4670d610d Rob Herring 2018-01-17 360 return err;
>> 4670d610d Rob Herring 2018-01-17 361 }
>> 83d525c0f Jan Kiszka 2018-04-30 362 EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
>> 4670d610d Rob Herring 2018-01-17 363 #endif /* CONFIG_OF_ADDRESS */
>> 4670d610d Rob Herring 2018-01-17 364
>>
>> :::::: The code at line 332 was first introduced by commit
>> :::::: 4670d610d59233b017a6ea1fa25bbf06dabbff42 PCI: Move OF-related PCI functions into PCI core
>>
>> :::::: TO: Rob Herring <robh@kernel.org>
>> :::::: CC: Bjorn Helgaas <bhelgaas@google.com>
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>>
>
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data
2018-05-11 5:53 ` Jan Kiszka
@ 2018-05-11 22:27 ` Bjorn Helgaas
2018-05-13 18:22 ` [PATCH] PCI: Fix error cleanup paths in devm_of_pci_get_host_bridge_resources() Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2018-05-11 22:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Julia Lawall, linux-pci, kbuild-all
On Fri, May 11, 2018 at 07:53:21AM +0200, Jan Kiszka wrote:
> On 2018-05-11 07:45, Jan Kiszka wrote:
> > On 2018-05-11 07:08, Julia Lawall wrote:
> >> The devm_kzalloc on line 324 makes the kfrees on lines 332 and 355
> >> incorrect. They can just be removed.
> >
> > True. In fact, 358 and 359 should go as well - making them obsolete is
> > the whole point of this conversion.
>
> More precisely, 332 needs to become devm_kfree because we skip over one
> resource without failing.
>
> >
> > The issue comes already with patch "Add dev parameter to
> > __of_pci_get_host_bridge_resources()" when dev != NULL. Bjorn, should I
> > resend the series with that patch fixed and the rest rebased? Or do you
> > prefer when I update only this patch, fixing the issue here?
>
> ...or is a fixup patch on top preferred?
Sorry, I missed this until now. If you send me a fixup patch on top of my
pci/resource branch, I'll just fold it in.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] PCI: Fix error cleanup paths in devm_of_pci_get_host_bridge_resources()
2018-05-11 22:27 ` Bjorn Helgaas
@ 2018-05-13 18:22 ` Jan Kiszka
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2018-05-13 18:22 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Julia Lawall, linux-pci, kbuild-all
Fallouts from the conversion to devm_kzalloc: In case the function
fails, we no longer need to clean up managed allocations. In fact, we
must not. Only one case requires explicit freeing, and that is when
of_pci_range_to_resource() fails and we simply skip over the related
resource entry.
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Fixes: 07adab611304 ("PCI: Add dev parameter to __of_pci_get_host_bridge_resources()")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/pci/of.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 38469ffd1d7e..ad8cc1d677f3 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -266,7 +266,6 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
struct list_head *resources, resource_size_t *io_base)
{
struct device_node *dev_node = dev->of_node;
- struct resource_entry *window;
struct resource *res;
struct resource *bus_range;
struct of_pci_range range;
@@ -299,7 +298,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
/* Check for ranges property */
err = of_pci_range_parser_init(&parser, dev_node);
if (err)
- goto parse_failed;
+ return err;
dev_dbg(dev, "Parsing ranges property...\n");
for_each_of_pci_range(&parser, &range) {
@@ -322,14 +321,12 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
continue;
res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
- if (!res) {
- err = -ENOMEM;
- goto parse_failed;
- }
+ if (!res)
+ return -ENOMEM;
err = of_pci_range_to_resource(&range, dev_node, res);
if (err) {
- kfree(res);
+ devm_kfree(dev, res);
continue;
}
@@ -338,8 +335,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
dev_err(dev,
"I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
dev_node);
- err = -EINVAL;
- goto conversion_failed;
+ return -EINVAL;
}
if (*io_base != (resource_size_t)OF_BAD_ADDR)
dev_warn(dev,
@@ -352,14 +348,6 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
}
return 0;
-
-conversion_failed:
- kfree(res);
-parse_failed:
- resource_list_for_each_entry(window, resources)
- kfree(window->res);
- pci_free_resource_list(resources);
- return err;
}
EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
#endif /* CONFIG_OF_ADDRESS */
--
2.13.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-13 18:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11 5:08 [pci:pci/resource 8/10] drivers/pci/of.c:332:3-8: WARNING: invalid free of devm_ allocated data Julia Lawall
2018-05-11 5:45 ` Jan Kiszka
2018-05-11 5:53 ` Jan Kiszka
2018-05-11 22:27 ` Bjorn Helgaas
2018-05-13 18:22 ` [PATCH] PCI: Fix error cleanup paths in devm_of_pci_get_host_bridge_resources() Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.