* [PATCH v4 0/1] Fix memory leak in of_pci_get_host_bridge_resources @ 2017-04-05 2:15 Jeffy Chen 2017-04-05 2:15 ` [PATCH v4] of/pci: " Jeffy Chen 0 siblings, 1 reply; 4+ messages in thread From: Jeffy Chen @ 2017-04-05 2:15 UTC (permalink / raw) To: linux-pci Cc: robh, toshi.kani, shawn.lin, briannorris, linux-kernel, dianders, bhelgaas, dtor, Jeffy Chen, devicetree, Rob Herring, Frank Rowand In of_pci_get_host_bridge_resources, we alloced some struct resource variables, and they would cause memory leak since no where to free them. Changes in v4: Use IORESOURCE_AUTO to mark struct resources which could be copied. Changes in v3: Add some comments. Changes in v2: Don't change the resource_list_create_entry's behavior. Jeffy Chen (1): of/pci: Fix memory leak in of_pci_get_host_bridge_resources drivers/of/of_pci.c | 51 ++++++++++++++++++--------------------------------- drivers/pci/bus.c | 7 ++++++- 2 files changed, 24 insertions(+), 34 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v4] of/pci: Fix memory leak in of_pci_get_host_bridge_resources 2017-04-05 2:15 [PATCH v4 0/1] Fix memory leak in of_pci_get_host_bridge_resources Jeffy Chen @ 2017-04-05 2:15 ` Jeffy Chen 2017-04-05 17:36 ` kbuild test robot 2017-04-05 17:39 ` kbuild test robot 0 siblings, 2 replies; 4+ messages in thread From: Jeffy Chen @ 2017-04-05 2:15 UTC (permalink / raw) To: linux-pci Cc: robh, toshi.kani, shawn.lin, briannorris, linux-kernel, dianders, bhelgaas, dtor, Jeffy Chen, devicetree, Rob Herring, Frank Rowand Currently we only free the allocated resource struct when error. This would cause memory leak after pci_free_resource_list. Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- Changes in v4: Use IORESOURCE_AUTO to mark struct resources which could be copied. Changes in v3: Add some comments. Changes in v2: Don't change the resource_list_create_entry's behavior. drivers/of/of_pci.c | 51 ++++++++++++++++++--------------------------------- drivers/pci/bus.c | 7 ++++++- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c index 0ee42c3..1ecbd79 100644 --- a/drivers/of/of_pci.c +++ b/drivers/of/of_pci.c @@ -189,9 +189,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, unsigned char busno, unsigned char bus_max, struct list_head *resources, resource_size_t *io_base) { - struct resource_entry *window; - struct resource *res; - struct resource *bus_range; + struct resource res; struct of_pci_range range; struct of_pci_range_parser parser; char range_type[4]; @@ -200,24 +198,21 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, if (io_base) *io_base = (resource_size_t)OF_BAD_ADDR; - bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL); - if (!bus_range) - return -ENOMEM; - pr_info("host bridge %s ranges:\n", dev->full_name); - err = of_pci_parse_bus_range(dev, bus_range); + err = of_pci_parse_bus_range(dev, &res); if (err) { - bus_range->start = busno; - bus_range->end = bus_max; - bus_range->flags = IORESOURCE_BUS; - pr_info(" No bus range found for %s, using %pR\n", - dev->full_name, bus_range); + res.start = busno; + res.end = bus_max; + res.flags = IORESOURCE_BUS; + pr_info(" No bus range found for %s\n", dev->full_name); } else { - if (bus_range->end > bus_range->start + bus_max) - bus_range->end = bus_range->start + bus_max; + if (res.end > res.start + bus_max) + res.end = res.start + bus_max; } - pci_add_resource(resources, bus_range); + + res.flags |= IORESOURCE_AUTO; + pci_add_resource(resources, res); /* Check for ranges property */ err = of_pci_range_parser_init(&parser, dev); @@ -244,24 +239,16 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, if (range.cpu_addr == OF_BAD_ADDR || range.size == 0) continue; - res = kzalloc(sizeof(struct resource), GFP_KERNEL); - if (!res) { - err = -ENOMEM; - goto parse_failed; - } - - err = of_pci_range_to_resource(&range, dev, res); - if (err) { - kfree(res); + err = of_pci_range_to_resource(&range, dev, &res); + if (err) continue; - } - if (resource_type(res) == IORESOURCE_IO) { + if (resource_type(&res) == IORESOURCE_IO) { if (!io_base) { pr_err("I/O range found for %s. Please provide an io_base pointer to save CPU base address\n", dev->full_name); err = -EINVAL; - goto conversion_failed; + goto parse_failed; } if (*io_base != (resource_size_t)OF_BAD_ADDR) pr_warn("More than one I/O resource converted for %s. CPU base address for old range lost!\n", @@ -269,16 +256,14 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, *io_base = range.cpu_addr; } - pci_add_resource_offset(resources, res, res->start - range.pci_addr); + res.flags |= IORESOURCE_AUTO; + pci_add_resource_offset(resources, res, + res->start - range.pci_addr); } 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; } diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index bc56cf1..5f32bfd 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -22,12 +22,17 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res, { struct resource_entry *entry; - entry = resource_list_create_entry(res, 0); + entry = resource_list_create_entry(NULL, 0); if (!entry) { printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res); return; } + if (res->flags & IORESOURCE_AUTO) + *entry->res = *res; + else + entry->res = res + entry->offset = offset; resource_list_add_tail(entry, resources); } -- 2.1.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] of/pci: Fix memory leak in of_pci_get_host_bridge_resources 2017-04-05 2:15 ` [PATCH v4] of/pci: " Jeffy Chen @ 2017-04-05 17:36 ` kbuild test robot 2017-04-05 17:39 ` kbuild test robot 1 sibling, 0 replies; 4+ messages in thread From: kbuild test robot @ 2017-04-05 17:36 UTC (permalink / raw) To: Jeffy Chen Cc: kbuild-all, linux-pci, robh, toshi.kani, shawn.lin, briannorris, linux-kernel, dianders, bhelgaas, dtor, Jeffy Chen, devicetree, Rob Herring, Frank Rowand [-- Attachment #1: Type: text/plain, Size: 4036 bytes --] Hi Jeffy, [auto build test ERROR on pci/next] [also build test ERROR on v4.11-rc5 next-20170405] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jeffy-Chen/of-pci-Fix-memory-leak-in-of_pci_get_host_bridge_resources/20170406-005941 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: i386-randconfig-n0-04051821 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers//of/of_pci.c: In function 'of_pci_get_host_bridge_resources': >> drivers//of/of_pci.c:215:30: error: incompatible type for argument 2 of 'pci_add_resource' pci_add_resource(resources, res); ^~~ In file included from include/linux/of_pci.h:4:0, from drivers//of/of_pci.c:8: include/linux/pci.h:1152:6: note: expected 'struct resource *' but argument is of type 'struct resource' void pci_add_resource(struct list_head *resources, struct resource *res); ^~~~~~~~~~~~~~~~ >> drivers//of/of_pci.c:261:9: error: invalid type argument of '->' (have 'struct resource') res->start - range.pci_addr); ^~ >> drivers//of/of_pci.c:260:38: error: incompatible type for argument 2 of 'pci_add_resource_offset' pci_add_resource_offset(resources, res, ^~~ In file included from include/linux/of_pci.h:4:0, from drivers//of/of_pci.c:8: include/linux/pci.h:1153:6: note: expected 'struct resource *' but argument is of type 'struct resource' void pci_add_resource_offset(struct list_head *resources, struct resource *res, ^~~~~~~~~~~~~~~~~~~~~~~ vim +/pci_add_resource +215 drivers//of/of_pci.c 209 } else { 210 if (res.end > res.start + bus_max) 211 res.end = res.start + bus_max; 212 } 213 214 res.flags |= IORESOURCE_AUTO; > 215 pci_add_resource(resources, res); 216 217 /* Check for ranges property */ 218 err = of_pci_range_parser_init(&parser, dev); 219 if (err) 220 goto parse_failed; 221 222 pr_debug("Parsing ranges property...\n"); 223 for_each_of_pci_range(&parser, &range) { 224 /* Read next ranges element */ 225 if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO) 226 snprintf(range_type, 4, " IO"); 227 else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM) 228 snprintf(range_type, 4, "MEM"); 229 else 230 snprintf(range_type, 4, "err"); 231 pr_info(" %s %#010llx..%#010llx -> %#010llx\n", range_type, 232 range.cpu_addr, range.cpu_addr + range.size - 1, 233 range.pci_addr); 234 235 /* 236 * If we failed translation or got a zero-sized region 237 * then skip this range 238 */ 239 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0) 240 continue; 241 242 err = of_pci_range_to_resource(&range, dev, &res); 243 if (err) 244 continue; 245 246 if (resource_type(&res) == IORESOURCE_IO) { 247 if (!io_base) { 248 pr_err("I/O range found for %s. Please provide an io_base pointer to save CPU base address\n", 249 dev->full_name); 250 err = -EINVAL; 251 goto parse_failed; 252 } 253 if (*io_base != (resource_size_t)OF_BAD_ADDR) 254 pr_warn("More than one I/O resource converted for %s. CPU base address for old range lost!\n", 255 dev->full_name); 256 *io_base = range.cpu_addr; 257 } 258 259 res.flags |= IORESOURCE_AUTO; > 260 pci_add_resource_offset(resources, res, > 261 res->start - range.pci_addr); 262 } 263 264 return 0; --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 25202 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] of/pci: Fix memory leak in of_pci_get_host_bridge_resources 2017-04-05 2:15 ` [PATCH v4] of/pci: " Jeffy Chen 2017-04-05 17:36 ` kbuild test robot @ 2017-04-05 17:39 ` kbuild test robot 1 sibling, 0 replies; 4+ messages in thread From: kbuild test robot @ 2017-04-05 17:39 UTC (permalink / raw) To: Jeffy Chen Cc: kbuild-all, linux-pci, robh, toshi.kani, shawn.lin, briannorris, linux-kernel, dianders, bhelgaas, dtor, Jeffy Chen, devicetree, Rob Herring, Frank Rowand [-- Attachment #1: Type: text/plain, Size: 2861 bytes --] Hi Jeffy, [auto build test ERROR on pci/next] [also build test ERROR on v4.11-rc5 next-20170405] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jeffy-Chen/of-pci-Fix-memory-leak-in-of_pci_get_host_bridge_resources/20170406-005941 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: x86_64-lkp (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers//pci/bus.c: In function 'pci_add_resource_offset': >> drivers//pci/bus.c:36:2: error: expected ';' before 'entry' entry->offset = offset; ^~~~~ vim +36 drivers//pci/bus.c 0efd5aab Bjorn Helgaas 2012-02-23 20 void pci_add_resource_offset(struct list_head *resources, struct resource *res, 0efd5aab Bjorn Helgaas 2012-02-23 21 resource_size_t offset) 45ca9e97 Bjorn Helgaas 2011-10-28 22 { 14d76b68 Jiang Liu 2015-02-05 23 struct resource_entry *entry; 45ca9e97 Bjorn Helgaas 2011-10-28 24 fa904d0d Jeffy Chen 2017-04-05 25 entry = resource_list_create_entry(NULL, 0); 14d76b68 Jiang Liu 2015-02-05 26 if (!entry) { 0efd5aab Bjorn Helgaas 2012-02-23 27 printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res); 45ca9e97 Bjorn Helgaas 2011-10-28 28 return; 45ca9e97 Bjorn Helgaas 2011-10-28 29 } 45ca9e97 Bjorn Helgaas 2011-10-28 30 fa904d0d Jeffy Chen 2017-04-05 31 if (res->flags & IORESOURCE_AUTO) fa904d0d Jeffy Chen 2017-04-05 32 *entry->res = *res; fa904d0d Jeffy Chen 2017-04-05 33 else fa904d0d Jeffy Chen 2017-04-05 34 entry->res = res fa904d0d Jeffy Chen 2017-04-05 35 14d76b68 Jiang Liu 2015-02-05 @36 entry->offset = offset; 14d76b68 Jiang Liu 2015-02-05 37 resource_list_add_tail(entry, resources); 0efd5aab Bjorn Helgaas 2012-02-23 38 } 0efd5aab Bjorn Helgaas 2012-02-23 39 EXPORT_SYMBOL(pci_add_resource_offset); 0efd5aab Bjorn Helgaas 2012-02-23 40 0efd5aab Bjorn Helgaas 2012-02-23 41 void pci_add_resource(struct list_head *resources, struct resource *res) 0efd5aab Bjorn Helgaas 2012-02-23 42 { 0efd5aab Bjorn Helgaas 2012-02-23 43 pci_add_resource_offset(resources, res, 0); 45ca9e97 Bjorn Helgaas 2011-10-28 44 } :::::: The code at line 36 was first introduced by commit :::::: 14d76b68f2819a1d0b50236a7e9e9f2ea69869d9 PCI: Use common resource list management code instead of private implementation :::::: TO: Jiang Liu <jiang.liu@linux.intel.com> :::::: CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 25168 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-05 17:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-05 2:15 [PATCH v4 0/1] Fix memory leak in of_pci_get_host_bridge_resources Jeffy Chen 2017-04-05 2:15 ` [PATCH v4] of/pci: " Jeffy Chen 2017-04-05 17:36 ` kbuild test robot 2017-04-05 17:39 ` kbuild test robot
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).