[bug report] kmemleak from driver i2c_piix4

[bug report] kmemleak from driver i2c_piix4

[Yi Zhang]

Hello

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
unreferenced object 0xffff8882be7fab80 (size 64):
  comm "systemd-udevd", pid 851, jiffies 4294724195 (age 1880.026s)
  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]
    [<00000000ef955e5e>] piix4_probe+0x38c/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

-- 
Best Regards,
  Yi Zhang

Re: [bug report] kmemleak from driver i2c_piix4

[Jean Delvare]

Hi Yi Zhang,

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?

Which line of the source code does
piix4_sb800_region_request+0x69/0x150 resolve to?

Which PCI device does the driver bind to? "lspci -nn | grep" SMBus
should tell.

Which messages are printed to the kernel log when you load and unload
the i2c-piix4 driver?

-- 
Jean Delvare
SUSE L3 Support

Re: [bug report] kmemleak from driver i2c_piix4

[Yi Zhang]

On Fri, Jun 17, 2022 at 3:48 PM Jean Delvare <jdelvare@suse.de> wrote:
>
> Hi Yi Zhang,
>
> 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,

>
> Which PCI device does the driver bind to? "lspci -nn | grep" SMBus
> should tell.
# lspci -nn | grep SMBus
00:14.0 SMBus [0c05]: Advanced Micro Devices, Inc. [AMD] FCH SMBus
Controller [1022:790b] (rev 61)
>
> Which messages are printed to the kernel log when you load and unload
> the i2c-piix4 driver?

[ 1400.379156] piix4_smbus 0000:00:14.0: SMBus Host Controller at
0xb00, revision 0
[ 1400.379164] piix4_smbus 0000:00:14.0: Using register 0x02 for SMBus
port selection
[ 1400.381759] piix4_smbus 0000:00:14.0: Auxiliary SMBus Host
Controller at 0xb20

>
> --
> Jean Delvare
> SUSE L3 Support
>


-- 
Best Regards,
  Yi Zhang

Re: [bug report] kmemleak from driver i2c_piix4

[Jean Delvare]

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@suse.de> 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@suse.de>
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@suse.de>
Fixes: 7c148722d074 ("i2c: piix4: Add EFCH MMIO support to region request and release")
Cc: Terry Bowman <terry.bowman@amd.com>
---
 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

Re: [bug report] kmemleak from driver i2c_piix4

[Terry Bowman]

On 6/21/22 07:48, Jean Delvare wrote:

> 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.
> 

Hi Jean,

Your analysis is correct. The kfree() call is missing during the release in i2c-piix4 and 
sp5100_tco driver patches. Let me know if there is anything I can do.

Re: [bug report] kmemleak from driver i2c_piix4

[Yi Zhang]

On Tue, Jun 21, 2022 at 10:29 PM Terry Bowman <Terry.Bowman@amd.com> wrote:
>
>
>
> On 6/21/22 07:48, Jean Delvare wrote:
>
> > Yi Zhang, can you please test this patch and confirm that it solves the
> > memory leak?

Yes, it was fixed by your patch, feel free to add
Tested-by: Yi Zhang <yi.zhang@redhat.com>

> >
> > 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.
> >
>
> Hi Jean,
>
> Your analysis is correct. The kfree() call is missing during the release in i2c-piix4 and
> sp5100_tco driver patches. Let me know if there is anything I can do.

I guess I reproduced you mentioned sp5100_tco driver mem leak?

unreferenced object 0xffff8882aa28e580 (size 64):
  comm "systemd-udevd", pid 909, jiffies 4294723115 (age 1091.326s)
  hex dump (first 32 bytes):
    00 03 d8 fe 00 00 00 00 07 03 d8 fe 00 00 00 00  ................
    60 a3 83 c0 ff ff ff ff 00 02 40 80 00 00 00 00  `.........@.....
  backtrace:
    [<0000000000f660c7>] kmem_cache_alloc_trace+0x169/0x2a0
    [<0000000072801446>] __request_region+0x52/0xb0
    [<00000000b6580dac>] sp5100_tco_probe+0x5c7/0x98e [sp5100_tco]
    [<00000000a4d2b832>] platform_probe+0xad/0x200
    [<00000000de5f0bc2>] really_probe+0x389/0x9b0
    [<0000000066128dc6>] __driver_probe_device+0x2bc/0x470
    [<00000000c11a18e4>] driver_probe_device+0x4a/0x120
    [<0000000030500515>] __device_attach_driver+0x1b3/0x280
    [<000000003bdf095d>] bus_for_each_drv+0x11f/0x1b0
    [<00000000c321e46b>] __device_attach+0x205/0x3c0
    [<000000003d858a16>] bus_probe_device+0x1a6/0x260
    [<000000005df24665>] device_add+0xa2c/0x1ae0
    [<000000000ce86b32>] platform_device_add+0x2c2/0x710
    [<000000009c49c0a6>] platform_device_register_full+0x34b/0x470
    [<00000000cfb75755>] 0xffffffffc08e80fa
    [<000000002fe1be8f>] do_one_initcall+0xfb/0x580

(gdb) l *(sp5100_tco_probe+0x5c7)
0xc17 is in sp5100_tco_probe (drivers/watchdog/sp5100_tco.c:349).
344 struct resource *res;
345 void __iomem *addr;
346 int ret;
347 u32 val;
348
349 res = request_mem_region_muxed(EFCH_PM_ACPI_MMIO_PM_ADDR,
350        EFCH_PM_ACPI_MMIO_PM_SIZE,
351        "sp5100_tco");
352
353 if (!res) {

>


-- 
Best Regards,
  Yi Zhang

Re: [bug report] kmemleak from driver i2c_piix4

[Jean Delvare]

Hi Terry,

On Tue, 21 Jun 2022 09:29:44 -0500, Terry Bowman wrote:
> On 6/21/22 07:48, Jean Delvare wrote:
> 
> > 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.
> 
> Your analysis is correct. The kfree() call is missing during the release in i2c-piix4 and 
> sp5100_tco driver patches. Let me know if there is anything I can do.

I've submitted patches for the 3 drivers. They are pretty
straightforward, but anything you'd be able to review or test would be
welcome.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [bug report] kmemleak from driver i2c_piix4

[Terry Bowman]

Hi Jean,

Yes, I'll respond with a tested-by and reviewed-by later this morning.

Regards,
Terry

On 6/22/22 01:39, Jean Delvare wrote:
> Hi Terry,
> 
> On Tue, 21 Jun 2022 09:29:44 -0500, Terry Bowman wrote:
>> On 6/21/22 07:48, Jean Delvare wrote:
>>
>>> 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.
>>
>> Your analysis is correct. The kfree() call is missing during the release in i2c-piix4 and 
>> sp5100_tco driver patches. Let me know if there is anything I can do.
> 
> I've submitted patches for the 3 drivers. They are pretty
> straightforward, but anything you'd be able to review or test would be
> welcome.
> 
> Thanks,

Re: [bug report] kmemleak from driver i2c_piix4

[Terry Bowman]

Hi Jean,

My testing shows this patch fixes the issue. I recreated the leak issue using 
v5.19.0-rc2 without this patch. I added this patch, retested using kmemleak, 
and the problem was not created. 

Patch inspection shows this fix properly frees the memoru resource structure.

You may add my 'tested-by' and/or 'reviewed-by'.

Regards,
Terry


On 6/22/22 01:39, Jean Delvare wrote:
> Hi Terry,
> 
> On Tue, 21 Jun 2022 09:29:44 -0500, Terry Bowman wrote:
>> On 6/21/22 07:48, Jean Delvare wrote:
>>
>>> 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.
>>
>> Your analysis is correct. The kfree() call is missing during the release in i2c-piix4 and 
>> sp5100_tco driver patches. Let me know if there is anything I can do.
> 
> I've submitted patches for the 3 drivers. They are pretty
> straightforward, but anything you'd be able to review or test would be
> welcome.
> 
> Thanks,