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

[Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>]

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