Hi Yi Zhang,
On Fri, 17 Jun 2022 21:33:16 +0800, Yi Zhang wrote:
> On Fri, Jun 17, 2022 at 3:48 PM Jean Delvare <jdelvare@xxxxxxx> wrote:
> > On Sat, 11 Jun 2022 17:08:20 +0800, Yi Zhang wrote:
> > > I found this kmemleak from dmesg, pls help check it, thanks.
> > >
> > > unreferenced object 0xffff8882be7fa500 (size 64):
> > > comm "systemd-udevd", pid 851, jiffies 4294724190 (age 1880.031s)
> > > hex dump (first 32 bytes):
> > > 00 03 d8 fe 00 00 00 00 07 03 d8 fe 00 00 00 00 ................
> > > 20 95 85 c0 ff ff ff ff 00 02 40 80 00 00 00 00 .........@.....
> > > backtrace:
> > > [<00000000ee7a7c0d>] __request_region+0x4f/0xc0
> > > [<000000000a0d9a20>] piix4_sb800_region_request+0x69/0x150 [i2c_piix4]
> > > [<00000000bbbc5f63>] piix4_setup_sb800.constprop.0+0xfd/0x4a0 [i2c_piix4]
> > > [<0000000060da9710>] piix4_probe+0x111/0x780 [i2c_piix4]
> > > [<0000000061a2fccd>] local_pci_probe+0xdf/0x170
> > > [<00000000f879d262>] pci_call_probe+0x15f/0x4b0
> > > [<00000000b1b4235f>] pci_device_probe+0xee/0x230
> > > [<000000007b0612f3>] really_probe+0x3d7/0xa10
> > > [<0000000016a94cde>] __driver_probe_device+0x2ab/0x460
> > > [<00000000fc08f31f>] driver_probe_device+0x49/0x120
> > > [<00000000c7600ea6>] __driver_attach+0x1c1/0x420
> > > [<00000000d075fad5>] bus_for_each_dev+0x121/0x1a0
> > > [<000000003a0c2b72>] bus_add_driver+0x39f/0x570
> > > [<00000000389c6619>] driver_register+0x20f/0x390
> > > [<00000000e1871c0e>] do_one_initcall+0xfc/0x560
> > > [<00000000899e6968>] do_init_module+0x190/0x620
> >
> > Thanks for reporting.
> >
> > Which kernel version are you running? Are there patches applied to the
> > i2c-piix4 driver?
> The latest linux tree
>
> > Which line of the source code does
> > piix4_sb800_region_request+0x69/0x150 resolve to?
>
> (gdb) l *(piix4_sb800_region_request+0x69)
> 0x2b9 is in piix4_sb800_region_request (drivers/i2c/busses/i2c-piix4.c:185).
> 180 {
> 181 if (mmio_cfg->use_mmio) {
> 182 struct resource *res;
> 183 void __iomem *addr;
> 184
> 185 res = request_mem_region_muxed(SB800_PIIX4_FCH_PM_ADDR,
> 186 SB800_PIIX4_FCH_PM_SIZE,
> 187 "sb800_piix4_smb");
> 188 if (!res) {
> 189 dev_err(dev,
OK, I think I see what's going on here. There is an asymmetry between
the legacy path and the MMIO path. The legacy path is OK but the MMIO
path indeed has a memory leak, caused by the direct call to
release_resource() in the cleanup-path, when the legacy path calls
release_region() instead.
Adding Terry Bowman to Cc as he wrote the code.
Here's a candidate fix:
From: Jean Delvare <jdelvare@xxxxxxx>
Subject: i2c: piix4: Fix a memory leak in the EFCH MMIO support
The recently added support for EFCH MMIO regions introduced a memory
leak in that code path. The leak is caused by the fact that
release_resource() merely removes the resource from the tree but does
not free its memory. We need to call release_mem_region() instead,
which does free the memory. As a nice side effect, this brings back
some symmetry between the legacy and MMIO paths.
Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
Fixes: 7c148722d074 ("i2c: piix4: Add EFCH MMIO support to region request and release")
Cc: Terry Bowman <terry.bowman@xxxxxxx>
---
drivers/i2c/busses/i2c-piix4.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- linux-5.18.orig/drivers/i2c/busses/i2c-piix4.c 2022-05-22 21:52:31.000000000 +0200
+++ linux-5.18/drivers/i2c/busses/i2c-piix4.c 2022-06-21 13:45:14.745708589 +0200
@@ -161,7 +161,6 @@ static const char *piix4_aux_port_name_s
struct sb800_mmio_cfg {
void __iomem *addr;
- struct resource *res;
bool use_mmio;
};
@@ -195,12 +194,12 @@ static int piix4_sb800_region_request(st
addr = ioremap(SB800_PIIX4_FCH_PM_ADDR,
SB800_PIIX4_FCH_PM_SIZE);
if (!addr) {
- release_resource(res);
+ release_mem_region(SB800_PIIX4_FCH_PM_ADDR,
+ SB800_PIIX4_FCH_PM_SIZE);
dev_err(dev, "SMBus base address mapping failed.\n");
return -ENOMEM;
}
- mmio_cfg->res = res;
mmio_cfg->addr = addr;
return 0;
@@ -222,7 +221,8 @@ static void piix4_sb800_region_release(s
{
if (mmio_cfg->use_mmio) {
iounmap(mmio_cfg->addr);
- release_resource(mmio_cfg->res);
+ release_mem_region(SB800_PIIX4_FCH_PM_ADDR,
+ SB800_PIIX4_FCH_PM_SIZE);
return;
}
Yi Zhang, can you please test this patch and confirm that it solves the
memory leak?
Terry, please review/comment.
If my analysis is correct then the sp5100_wdt and thinkpad_acpi drivers
suffer from a similar leak and need to be fixed the same way.
--
Jean Delvare
SUSE L3 Support
