BUG: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

BUG: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

[Mirsad Goran Todorovac]

Hi all!

Here are again the good news and the bad news:

BAD:  another kernel memory leak detected (one more to hunt down and fix)
GOOD: another kernel memory leak detected (one less unaccounted for)

I tried to make some fun, but maintainers are busy folks, so let's get down
to business:

---
Nine (9) new systemd-udevd kernel memory leaks occurred (unable to reproduce).

The platform is Ubuntu 22.10 with (relatively recent) systemd 251.4-1ubuntu7.1
on LENOVO_MT_82H8_BU_idea_FM_IdeaPad 3 15ITL6 with BIOS GGCN51WW from 11/16/2022.

The symptom (/sys/kernel/debug/kmemleak output):

unreferenced object 0xffff909698ff9280 (size 64):
  comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
  hex dump (first 32 bytes):
    e0 51 bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  .Q..............
    40 5b bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  @[..............
  backtrace:
    [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffb31af2ec>] xhci_alloc_command+0x7c/0x1b0
    [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
    [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
    [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
    [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
    [<ffffffffb3022295>] device_remove+0x65/0x70
    [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
unreferenced object 0xffff909699bb5b40 (size 32):
  comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    50 5b bb 99 96 90 ff ff 50 5b bb 99 96 90 ff ff  P[......P[......
  backtrace:
    [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffb31af364>] xhci_alloc_command+0xf4/0x1b0
    [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
    [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
    [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
    [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
    [<ffffffffb3022295>] device_remove+0x65/0x70
    [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
unreferenced object 0xffff909699bb51e0 (size 32):
  comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
  hex dump (first 32 bytes):
    02 00 00 00 20 04 00 00 00 a0 ff 98 96 90 ff ff  .... ...........
    00 a0 ff 18 01 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffb31ad86e>] xhci_alloc_container_ctx+0x7e/0x140
    [<ffffffffb31af469>] xhci_alloc_command_with_ctx+0x39/0x70
    [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
    [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
    [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
    [<ffffffffb3022295>] device_remove+0x65/0x70
    [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
.
.
.

Please find the config, lshw output and complete /sys/kernel/debug/kmemleak
output here:

https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/kmemleak.log

https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/lshw.txt
https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/config-6.3.0-rc3-kobj-rlse-00317-g65aca32efdcb

The systemd issue tracker said they accept issues only for the most recent 253 and
252, 251.4 seems too old for them despite being issued on May 21, 2022
(Source: https://github.com/systemd/systemd/releases).

It is not that I want to dump this on Linux kernel developers, but I felt
like it is a kernel memory leak problem rather than a bug in systemd-udevd.

Of course, my hunch might be wrong ...

As per Code of Conduct, I have checked for the developers and maintainers with
scripts/get_maintainers.pl.

Best regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

Re: BUG: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

[Mirsad Goran Todorovac]

On 25. 03. 2023. 12:27, Mirsad Goran Todorovac wrote:
> Hi all!
> 
> Here are again the good news and the bad news:
> 
> BAD:  another kernel memory leak detected (one more to hunt down and fix)
> GOOD: another kernel memory leak detected (one less unaccounted for)
> 
> I tried to make some fun, but maintainers are busy folks, so let's get down
> to business:
> 
> ---
> Nine (9) new systemd-udevd kernel memory leaks occurred (unable to reproduce).
> 
> The platform is Ubuntu 22.10 with (relatively recent) systemd 251.4-1ubuntu7.1
> on LENOVO_MT_82H8_BU_idea_FM_IdeaPad 3 15ITL6 with BIOS GGCN51WW from 11/16/2022.
> 
> The symptom (/sys/kernel/debug/kmemleak output):
> 
> unreferenced object 0xffff909698ff9280 (size 64):
>   comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>   hex dump (first 32 bytes):
>     e0 51 bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  .Q..............
>     40 5b bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  @[..............
>   backtrace:
>     [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>     [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>     [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>     [<ffffffffb31af2ec>] xhci_alloc_command+0x7c/0x1b0
>     [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
>     [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
>     [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>     [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
>     [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
>     [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
>     [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
>     [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
>     [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
>     [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
>     [<ffffffffb3022295>] device_remove+0x65/0x70
>     [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
> unreferenced object 0xffff909699bb5b40 (size 32):
>   comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     50 5b bb 99 96 90 ff ff 50 5b bb 99 96 90 ff ff  P[......P[......
>   backtrace:
>     [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>     [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>     [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>     [<ffffffffb31af364>] xhci_alloc_command+0xf4/0x1b0
>     [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
>     [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
>     [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>     [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
>     [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
>     [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
>     [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
>     [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
>     [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
>     [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
>     [<ffffffffb3022295>] device_remove+0x65/0x70
>     [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
> unreferenced object 0xffff909699bb51e0 (size 32):
>   comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>   hex dump (first 32 bytes):
>     02 00 00 00 20 04 00 00 00 a0 ff 98 96 90 ff ff  .... ...........
>     00 a0 ff 18 01 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>     [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>     [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>     [<ffffffffb31ad86e>] xhci_alloc_container_ctx+0x7e/0x140
>     [<ffffffffb31af469>] xhci_alloc_command_with_ctx+0x39/0x70
>     [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
>     [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>     [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
>     [<ffffffffb3145f46>] usb_disable_lpm+0x86/0xc0
>     [<ffffffffb3145fc1>] usb_unlocked_disable_lpm+0x31/0x60
>     [<ffffffffb3155db6>] usb_disable_device+0x136/0x250
>     [<ffffffffb3156b23>] usb_set_configuration+0x583/0xa70
>     [<ffffffffb3164c6d>] usb_generic_driver_disconnect+0x2d/0x40
>     [<ffffffffb3158612>] usb_unbind_device+0x32/0x90
>     [<ffffffffb3022295>] device_remove+0x65/0x70
>     [<ffffffffb3023903>] device_release_driver_internal+0xc3/0x140
> .
> .
> .
> 
> Please find the config, lshw output and complete /sys/kernel/debug/kmemleak
> output here:
> 
> https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/kmemleak.log
> 
> https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/lshw.txt
> https://domac.alu.unizg.hr/~mtodorov/linux/bugreports/systemd-udevd/config-6.3.0-rc3-kobj-rlse-00317-g65aca32efdcb
> 
> The systemd issue tracker said they accept issues only for the most recent 253 and
> 252, 251.4 seems too old for them despite being issued on May 21, 2022
> (Source: https://github.com/systemd/systemd/releases).
> 
> It is not that I want to dump this on Linux kernel developers, but I felt
> like it is a kernel memory leak problem rather than a bug in systemd-udevd.
> 
> Of course, my hunch might be wrong ...
> 
> As per Code of Conduct, I have checked for the developers and maintainers with
> scripts/get_maintainers.pl.

By the Murphy's Law, it appears that I forgot the most impotant thing:
the kernel is 6.3-rc+ commit 65aca32efdcb from Torvalds tree, with
KMEMLEAK, CONFIG_DEBUG_{KOBJECT|KOBJECT_RELEASE} enabled.

Have a nice day.

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

Re: BUG: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

[Mathias Nyman]

On 25.3.2023 13.33, Mirsad Goran Todorovac wrote:
> On 25. 03. 2023. 12:27, Mirsad Goran Todorovac wrote:
>> Hi all!
>>
>> Here are again the good news and the bad news:
>>
>> BAD:  another kernel memory leak detected (one more to hunt down and fix)
>> GOOD: another kernel memory leak detected (one less unaccounted for)
>>
>> I tried to make some fun, but maintainers are busy folks, so let's get down
>> to business:
>>
>> ---
>> Nine (9) new systemd-udevd kernel memory leaks occurred (unable to reproduce).
>>
>> The platform is Ubuntu 22.10 with (relatively recent) systemd 251.4-1ubuntu7.1
>> on LENOVO_MT_82H8_BU_idea_FM_IdeaPad 3 15ITL6 with BIOS GGCN51WW from 11/16/2022.
>>
>> The symptom (/sys/kernel/debug/kmemleak output):
>>
>> unreferenced object 0xffff909698ff9280 (size 64):
>>    comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>>    hex dump (first 32 bytes):
>>      e0 51 bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  .Q..............
>>      40 5b bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  @[..............
>>    backtrace:
>>      [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>>      [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>>      [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>>      [<ffffffffb31af2ec>] xhci_alloc_command+0x7c/0x1b0
>>      [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
>>      [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0>>      [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>>      [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0

Thanks for the report.

I think I found the leak, and wrote a patch for it.
Any chance you could test it with the same setup?

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=8bacee588602ed74cc22aaf4c56b796300e5a943

Thanks
-Mathias

[PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mathias Nyman]

The command allocated to set exit latency LPM values need to be freed in
case the command is never queued. This would be the case if there is no
change in exit latency values, or device is missing.

Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
Cc: <Stable@vger.kernel.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index bdb6dd819a3b..6307bae9cddf 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
 
 	if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
 		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_free_command(xhci, command);
 		return 0;
 	}
 
-- 
2.25.1

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Greg KH]

On Mon, Mar 27, 2023 at 12:50:19PM +0300, Mathias Nyman wrote:
> The command allocated to set exit latency LPM values need to be freed in
> case the command is never queued. This would be the case if there is no
> change in exit latency values, or device is missing.
> 
> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
> Cc: <Stable@vger.kernel.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 1 +
>  1 file changed, 1 insertion(+)

Do you want me to take this now, or will you be sending this to me in a
separate series of xhci fixes?  Either is fine with me.

thanks,

greg k-h

Re: BUG: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

[Mirsad Goran Todorovac]

On 27.3.2023. 11:41, Mathias Nyman wrote:
> On 25.3.2023 13.33, Mirsad Goran Todorovac wrote:
>> On 25. 03. 2023. 12:27, Mirsad Goran Todorovac wrote:
>>> Hi all!
>>>
>>> Here are again the good news and the bad news:
>>>
>>> BAD:  another kernel memory leak detected (one more to hunt down and fix)
>>> GOOD: another kernel memory leak detected (one less unaccounted for)
>>>
>>> I tried to make some fun, but maintainers are busy folks, so let's get down
>>> to business:
>>>
>>> ---
>>> Nine (9) new systemd-udevd kernel memory leaks occurred (unable to reproduce).
>>>
>>> The platform is Ubuntu 22.10 with (relatively recent) systemd 251.4-1ubuntu7.1
>>> on LENOVO_MT_82H8_BU_idea_FM_IdeaPad 3 15ITL6 with BIOS GGCN51WW from 11/16/2022.
>>>
>>> The symptom (/sys/kernel/debug/kmemleak output):
>>>
>>> unreferenced object 0xffff909698ff9280 (size 64):
>>>    comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>>>    hex dump (first 32 bytes):
>>>      e0 51 bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  .Q..............
>>>      40 5b bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  @[..............
>>>    backtrace:
>>>      [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>>>      [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>>>      [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>>>      [<ffffffffb31af2ec>] xhci_alloc_command+0x7c/0x1b0
>>>      [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
>>>      [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0>>      [<ffffffffb31a8c5b>] 
>>> xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>>>      [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
> 
> Thanks for the report.
> 
> I think I found the leak, and wrote a patch for it.
> Any chance you could test it with the same setup?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=8bacee588602ed74cc22aaf4c56b796300e5a943

Hi, Mathias,

Great you have found the leak!

I cannot make testing in the same setup because I can access that particular box
until after I finish my day job.

I will prioritise it.

If this is the catch, it will save me almost a dozen bisect builds. :-)

Best regards,
Mirsad

-- 
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union

Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mathias Nyman]

On 27.3.2023 14.51, Greg KH wrote:
> On Mon, Mar 27, 2023 at 12:50:19PM +0300, Mathias Nyman wrote:
>> The command allocated to set exit latency LPM values need to be freed in
>> case the command is never queued. This would be the case if there is no
>> change in exit latency values, or device is missing.
>>
>> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
>> Cc: <Stable@vger.kernel.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci.c | 1 +
>>   1 file changed, 1 insertion(+)
> 
> Do you want me to take this now, or will you be sending this to me in a
> separate series of xhci fixes?  Either is fine with me.

I can send a separate series this week, there are some other fixes as well.

Thanks
Mathias

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mirsad Goran Todorovac]

On 27. 03. 2023. 15:31, Mathias Nyman wrote:
> On 27.3.2023 14.51, Greg KH wrote:
>> On Mon, Mar 27, 2023 at 12:50:19PM +0300, Mathias Nyman wrote:
>>> The command allocated to set exit latency LPM values need to be freed in
>>> case the command is never queued. This would be the case if there is no
>>> change in exit latency values, or device is missing.
>>>
>>> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
>>> Cc: <Stable@vger.kernel.org>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> ---
>>>   drivers/usb/host/xhci.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>
>> Do you want me to take this now, or will you be sending this to me in a
>> separate series of xhci fixes?  Either is fine with me.
> 
> I can send a separate series this week, there are some other fixes as well.

Hi, Mathias,

I can confirm from the original setup that triggered the bug:

root@marvin-IdeaPad-3-15ITL6:~# uname -rms
Linux 6.3.0-rc3-kobj-rlse-00317-g65aca32efdcb-dirty x86_64
root@marvin-IdeaPad-3-15ITL6:~# 

The version without the patch still manifests the issue:

root@marvin-IdeaPad-3-15ITL6:/home/marvin# uname -rms
Linux 6.3.0-rc3-kobj-rlse-wop-00317-g65aca32efdcb x86_64
root@marvin-IdeaPad-3-15ITL6:/home/marvin# echo scan > /sys/kernel/debug/kmemleak 
root@marvin-IdeaPad-3-15ITL6:/home/marvin# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff96e59c4e1400 (size 64):
  comm "systemd-udevd", pid 420, jiffies 4294893221 (age 260.340s)
  hex dump (first 32 bytes):
    c0 8b c3 98 e5 96 ff ff 00 00 00 00 00 00 00 00  ................
    60 8c c3 98 e5 96 ff ff 00 00 00 00 00 00 00 00  `...............
  backtrace:
    [<ffffffffacbde94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffacbe5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffacb62f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffad3af2ec>] xhci_alloc_command+0x7c/0x1b0
    [<ffffffffad3af451>] xhci_alloc_command_with_ctx+0x21/0x70
    [<ffffffffad3a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffad3a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffad3457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffad345f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffad345fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffad355db6>] usb_disable_device+0x136/0x250
    [<ffffffffad356b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffad364c6d>] usb_generic_driver_disconnect+0x2d/0x40
    [<ffffffffad358612>] usb_unbind_device+0x32/0x90
    [<ffffffffad222295>] device_remove+0x65/0x70
    [<ffffffffad223903>] device_release_driver_internal+0xc3/0x140
unreferenced object 0xffff96e598c38c60 (size 32):
  comm "systemd-udevd", pid 420, jiffies 4294893221 (age 260.340s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    70 8c c3 98 e5 96 ff ff 70 8c c3 98 e5 96 ff ff  p.......p.......
  backtrace:
    [<ffffffffacbde94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffacbe5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffacb62f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffad3af364>] xhci_alloc_command+0xf4/0x1b0
    [<ffffffffad3af451>] xhci_alloc_command_with_ctx+0x21/0x70
    [<ffffffffad3a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffad3a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffad3457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffad345f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffad345fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffad355db6>] usb_disable_device+0x136/0x250
    [<ffffffffad356b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffad364c6d>] usb_generic_driver_disconnect+0x2d/0x40
    [<ffffffffad358612>] usb_unbind_device+0x32/0x90
    [<ffffffffad222295>] device_remove+0x65/0x70
    [<ffffffffad223903>] device_release_driver_internal+0xc3/0x140
unreferenced object 0xffff96e598c38bc0 (size 32):
  comm "systemd-udevd", pid 420, jiffies 4294893221 (age 260.340s)
  hex dump (first 32 bytes):
    02 00 00 00 20 04 00 00 00 90 79 9c e5 96 ff ff  .... .....y.....
    00 90 79 1c 01 00 00 00 00 00 00 00 00 00 00 00  ..y.............
  backtrace:
    [<ffffffffacbde94c>] slab_post_alloc_hook+0x8c/0x320
    [<ffffffffacbe5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
    [<ffffffffacb62f3b>] kmalloc_node_trace+0x2b/0xa0
    [<ffffffffad3ad86e>] xhci_alloc_container_ctx+0x7e/0x140
    [<ffffffffad3af469>] xhci_alloc_command_with_ctx+0x39/0x70
    [<ffffffffad3a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0
    [<ffffffffad3a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
    [<ffffffffad3457a7>] usb_disable_link_state+0x57/0xe0
    [<ffffffffad345f46>] usb_disable_lpm+0x86/0xc0
    [<ffffffffad345fc1>] usb_unlocked_disable_lpm+0x31/0x60
    [<ffffffffad355db6>] usb_disable_device+0x136/0x250
    [<ffffffffad356b23>] usb_set_configuration+0x583/0xa70
    [<ffffffffad364c6d>] usb_generic_driver_disconnect+0x2d/0x40
.
.
.

It is completely the same commit save to the difference of applying your patch
kobj-rlse-dirty version and removing it and rebuilding -kobj-rlse-wop- version

Congratulations, for further bisect appears obsoleted.

I haven't been able to iterate the bug, or cause more leaks by unplugging and
plugging in again USB devices, so I cannot estimate severity of this bug, but
I really wouldn't have an idea without bisecting first.

Best regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

Re: BUG: BISECTED: drivers/usb/host/xhci: memleak in alloc from xhci_disable_usb3_lpm_timeout()

[Mirsad Goran Todorovac]

On 27. 03. 2023. 11:41, Mathias Nyman wrote:
> On 25.3.2023 13.33, Mirsad Goran Todorovac wrote:
>> On 25. 03. 2023. 12:27, Mirsad Goran Todorovac wrote:
>>> Hi all!
>>>
>>> Here are again the good news and the bad news:
>>>
>>> BAD:  another kernel memory leak detected (one more to hunt down and fix)
>>> GOOD: another kernel memory leak detected (one less unaccounted for)
>>>
>>> I tried to make some fun, but maintainers are busy folks, so let's get down
>>> to business:
>>>
>>> ---
>>> Nine (9) new systemd-udevd kernel memory leaks occurred (unable to reproduce).
>>>
>>> The platform is Ubuntu 22.10 with (relatively recent) systemd 251.4-1ubuntu7.1
>>> on LENOVO_MT_82H8_BU_idea_FM_IdeaPad 3 15ITL6 with BIOS GGCN51WW from 11/16/2022.
>>>
>>> The symptom (/sys/kernel/debug/kmemleak output):
>>>
>>> unreferenced object 0xffff909698ff9280 (size 64):
>>>    comm "systemd-udevd", pid 436, jiffies 4294893239 (age 6287.088s)
>>>    hex dump (first 32 bytes):
>>>      e0 51 bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  .Q..............
>>>      40 5b bb 99 96 90 ff ff 00 00 00 00 00 00 00 00  @[..............
>>>    backtrace:
>>>      [<ffffffffb29de94c>] slab_post_alloc_hook+0x8c/0x320
>>>      [<ffffffffb29e5107>] __kmem_cache_alloc_node+0x1c7/0x2b0
>>>      [<ffffffffb2962f3b>] kmalloc_node_trace+0x2b/0xa0
>>>      [<ffffffffb31af2ec>] xhci_alloc_command+0x7c/0x1b0
>>>      [<ffffffffb31af451>] xhci_alloc_command_with_ctx+0x21/0x70
>>>      [<ffffffffb31a8a3e>] xhci_change_max_exit_latency+0x2e/0x1c0>>      [<ffffffffb31a8c5b>] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
>>>      [<ffffffffb31457a7>] usb_disable_link_state+0x57/0xe0
> 
> Thanks for the report.
> 
> I think I found the leak, and wrote a patch for it.
> Any chance you could test it with the same setup?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=8bacee588602ed74cc22aaf4c56b796300e5a943

As I have already been half-through bisect, I took the liberty to finish it.

# good: [5ce036b98dd3301fc43bb06a6383ef07b6c776bc] xhci: dbc: create and remove dbc structure in dbgtty driver.
git bisect good 5ce036b98dd3301fc43bb06a6383ef07b6c776bc
# bad: [d016cbe4d7acf5100df83ecf4d02db4e9f607c1d] usb: typec: Support the WUSB3801 port controller
git bisect bad d016cbe4d7acf5100df83ecf4d02db4e9f607c1d
# bad: [cd36facf104afbde7e8fa25cd6f5b6dd9fa97bb2] usb: remove Link Powermanagement (LPM) disable before port reset.
git bisect bad cd36facf104afbde7e8fa25cd6f5b6dd9fa97bb2
# good: [6aec50009d52f28ef8b512cba0f5078b3928064d] xhci: dbc: Don't call dbc_tty_init() on every dbc tty probe
git bisect good 6aec50009d52f28ef8b512cba0f5078b3928064d
# bad: [5c2a380a5aa8c15985359904b6d47466528d2993] xhci: Allocate separate command structures for each LPM command
git bisect bad 5c2a380a5aa8c15985359904b6d47466528d2993
# good: [e1ec140f273e1e30cea7e6d5f50934d877232121] xhci: dbgtty: use IDR to support several dbc instances.
git bisect good e1ec140f273e1e30cea7e6d5f50934d877232121
# first bad commit: [5c2a380a5aa8c15985359904b6d47466528d2993] xhci: Allocate separate command structures for each LPM command

Interesting enough, Mr. Greg predicted this is an xhci problem already in November [1],
but I did not embolden myself to bisect until this weekend, seeing that it was
still leaking.

But then I was brand new to the CONFIG_DEBUG_KMEMLEAK feature.

[1] https://lore.kernel.org/lkml/Y2zCYwNNvQWppLWZ@kroah.com/

I think the culprit patch is otherwise awesome, reducing latency and locking, especially
welcome in multimedia use.

So far, I was unable to exploit this leak as non-superuser or automate it like gpio-sim
to exhaust the kernel's limited memory, but this doesn't prove that smarter hackers
couldn't devise some means to do that exploit.

BTW: Full designation of the patch is 5.17.0-rc4-kmemlk-xhci-00071-g5c2a380a5aa8, so all
kernels 5.17-rc4+ appear affected by the issue.

Thank you and if you will need any more testing, I am available in my off hours.

Really nice working to assist your dynamic team.

To compare, another developer from another project was desperate about a software giant
being unwilling to abandon deprecated MODP 1024 DH renegotiation for its native VPN,
now for a couple of years ...

Best regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mirsad Goran Todorovac]

On 27. 03. 2023. 11:50, Mathias Nyman wrote:
> The command allocated to set exit latency LPM values need to be freed in
> case the command is never queued. This would be the case if there is no
> change in exit latency values, or device is missing.
> 
> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
> Cc: <Stable@vger.kernel.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index bdb6dd819a3b..6307bae9cddf 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
>  
>  	if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
>  		spin_unlock_irqrestore(&xhci->lock, flags);
> +		xhci_free_command(xhci, command);
>  		return 0;
>  	}
>  

After more testing, I can confirm that your patch fixes the leak in the original
environment.

And I see that you independently already have bisected the culprit commit, so I
have needlessly duplicated the work.

However, I consider myself still a learner and an absolute beginner in the Linux
kernel world ...

Best regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mathias Nyman]

On 28.3.2023 1.25, Mirsad Goran Todorovac wrote:
> On 27. 03. 2023. 11:50, Mathias Nyman wrote:
>> The command allocated to set exit latency LPM values need to be freed in
>> case the command is never queued. This would be the case if there is no
>> change in exit latency values, or device is missing.
>>
>> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
>> Cc: <Stable@vger.kernel.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index bdb6dd819a3b..6307bae9cddf 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
>>   
>>   	if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
>>   		spin_unlock_irqrestore(&xhci->lock, flags);
>> +		xhci_free_command(xhci, command);
>>   		return 0;
>>   	}
>>   
> 
> After more testing, I can confirm that your patch fixes the leak in the original
> environment.

Thanks for testing.
Can I add the tags below to the patch?
  
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>

Thanks
Mathias

Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early

[Mirsad Goran Todorovac]

On 28.3.2023. 9:57, Mathias Nyman wrote:
> On 28.3.2023 1.25, Mirsad Goran Todorovac wrote:
>> On 27. 03. 2023. 11:50, Mathias Nyman wrote:
>>> The command allocated to set exit latency LPM values need to be freed in
>>> case the command is never queued. This would be the case if there is no
>>> change in exit latency values, or device is missing.
>>>
>>> Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
>>> Cc: <Stable@vger.kernel.org>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> ---
>>>   drivers/usb/host/xhci.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index bdb6dd819a3b..6307bae9cddf 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
>>>       if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
>>>           spin_unlock_irqrestore(&xhci->lock, flags);
>>> +        xhci_free_command(xhci, command);
>>>           return 0;
>>>       }
>>
>> After more testing, I can confirm that your patch fixes the leak in the original
>> environment.
> 
> Thanks for testing.
> Can I add the tags below to the patch?
> 
> Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> 
> Thanks
> Mathias

Sure, thanks for the thought. Sorry, my Thunderbird has hidden your message,
I saw it only on Lore and accidentally.

Regards,
Mirsad

-- 
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union

Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu