Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
> Signed-off-by: Mathias Nyman
> ---
> 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
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
>>> Signed-off-by: Mathias Nyman
>>> ---
>>> 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 0x96e59c4e1400 (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:
[] slab_post_alloc_hook+0x8c/0x320
[] __kmem_cache_alloc_node+0x1c7/0x2b0
[] kmalloc_node_trace+0x2b/0xa0
[] xhci_alloc_command+0x7c/0x1b0
[] xhci_alloc_command_with_ctx+0x21/0x70
[] xhci_change_max_exit_latency+0x2e/0x1c0
[] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
[] usb_disable_link_state+0x57/0xe0
[] usb_disable_lpm+0x86/0xc0
[] usb_unlocked_disable_lpm+0x31/0x60
[] usb_disable_device+0x136/0x250
[] usb_set_configuration+0x583/0xa70
[] usb_generic_driver_disconnect+0x2d/0x40
[] usb_unbind_device+0x32/0x90
[] device_remove+0x65/0x70
[] device_release_driver_internal+0xc3/0x140
unreferenced object 0x96e598c38c60 (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:
[] slab_post_alloc_hook+0x8c/0x320
[] __kmem_cache_alloc_node+0x1c7/0x2b0
[] kmalloc_node_trace+0x2b/0xa0
[] xhci_alloc_command+0xf4/0x1b0
[] xhci_alloc_command_with_ctx+0x21/0x70
[] xhci_change_max_exit_latency+0x2e/0x1c0
[] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
[] usb_disable_link_state+0x57/0xe0
[] usb_disable_lpm+0x86/0xc0
[] usb_unlocked_disable_lpm+0x31/0x60
[] usb_disable_device+0x136/0x250
[] usb_set_configuration+0x583/0xa70
[] usb_generic_driver_disconnect+0x2d/0x40
[] usb_unbind_device+0x32/0x90
[] device_remove+0x65/0x70
[] device_release_driver_internal+0xc3/0x140
unreferenced object 0x96e598c38bc0 (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:
[] slab_post_alloc_hook+0x8c/0x320
[] __kmem_cache_alloc_node+0x1c7/0x2b0
[] kmalloc_node_trace+0x2b/0xa0
[] xhci_alloc_container_ctx+0x7e/0x140
[] xhci_alloc_command_with_ctx+0x39/0x70
[] xhci_change_max_exit_latency+0x2e/0x1c0
[] xhci_disable_usb3_lpm_timeout+0x7b/0xb0
[] usb_disable_link_state+0x57/0xe0
[] usb_disable_lpm+0x86/0xc0
[] usb_unlocked_disable_lpm+0x31/0x60
[] usb_disable_device+0x136/0x250
[] usb_set_configuration+0x583/0xa70
[] 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
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
> Signed-off-by: Mathias Nyman
> ---
> 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?"
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
Signed-off-by: Mathias Nyman
---
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
Tested-by: Mirsad Goran Todorovac
Thanks
Mathias
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
Signed-off-by: Mathias Nyman
---
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
Tested-by: Mirsad Goran Todorovac
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
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
Re: [PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
Signed-off-by: Mathias Nyman
---
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
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
[PATCH] xhci: Free the command allocated for setting LPM if we return early
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:
Signed-off-by: Mathias Nyman
---
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
--
Ubuntu-devel-discuss mailing list
Ubuntu-devel-discuss@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss
