[Bug 220052] New: The usb/cdc-acm driver uses memory after it is freed

[Bug 220052] New: The usb/cdc-acm driver uses memory after it is freed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=220052

            Bug ID: 220052
           Summary: The usb/cdc-acm driver uses memory after it is freed
           Product: Drivers
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: USB
          Assignee: drivers_usb@kernel-bugs.kernel.org
          Reporter: ben.maan@aimvalley.com
        Regression: No

Created attachment 308013
  --> https://bugzilla.kernel.org/attachment.cgi?id=308013&action=edit
A patch file that solves the kernel panic.

In the cdc-acm driver, memory is used after it is freed. The code is in
drivers/usb/class/cdc-acm.c, in the acm_softint function. The problem manifests
when a usb connection is disconnected by unplugging a cable and can result in a
kernel panic. The panic is not always seen but only when the freed memory is
claimed and used by an other process, somewhere between the for loop and the
acm_submit_read_urbs.

A solution is attached in the form of a patch where 2 lines of code are
exchanged with 2 other lines. The patch is based on kernel version
linux-6.15-rc3.

The kernel panic is shown below.

cdc_acm 1-1.3:1.2: urb 0 failed submission with -2
Unable to handle kernel NULL pointer dereference at virtual address 00000030
Mem abort info:
  Exception class = DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
Data abort info:
  ISV = 0, ISS = 0x00000004
  CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgd = ffff8000bbab6000
[0000000000000030] *pgd=0000000000000000
Internal error: Oops: 96000004 [#1] PREEMPT SMP
Modules linked in: 8021q garp stp mrp ax88179_178a usbnet crc32_ce crct10dif_ce
galcore(O)
CPU: 0 PID: 1740 Comm: kworker/0:3 Tainted: G           O   
4.14.98-imx_4.14.98_2.0.0_ga+g5d6cbeafb80c #1
Hardware name: Freescale i.MX8MQ EVK (DT)
Workqueue: events acm_softint
task: ffff8000baa12880 task.stack: ffff00000d448000
PC is at usb_autopm_put_interface+0x14/0x48
LR is at acm_softint+0xb8/0xd8
pc : [<ffff0000088e3abc>] lr : [<ffff0000089480b8>] pstate: 40000145
sp : ffff00000d44bda0
x29: ffff00000d44bda0 x28: 0000000000000000 
x27: 0000000000000000 x26: ffff000009222ec0 
x25: ffff0000080e7760 x24: ffff8000b6386788 
x23: 0000000000000000 x22: ffff8000bff60580 
x21: ffff8000b6385f88 x20: ffff8000b6386000 
x19: ffff8000b6386788 x18: 0000000000000001 
x17: 0000ffffb6667098 x16: ffff000008261920 
x15: ffff000009752000 x14: 00000000fffffff0 
x13: ffff0000098e5150 x12: ffff000009738df8 
x11: ffff000008663128 x10: ffff0000098e4000 
x9 : 0000000000000006 x8 : 6d6275732064656c 
x7 : 6961662030206272 x6 : 0000000000000514 
x5 : 0000000000000000 x4 : 0000000000000000 
x3 : 0000000000000330 x2 : 00000000fffefa67 
x1 : ffff000009736000 x0 : 0000000000000000 
Process kworker/0:3 (pid: 1740, stack limit = 0xffff00000d448000)
Call trace:
Exception stack(0xffff00000d44bc60 to 0xffff00000d44bda0)
bc60: 0000000000000000 ffff000009736000 00000000fffefa67 0000000000000330
bc80: 0000000000000000 0000000000000000 0000000000000514 6961662030206272
bca0: 6d6275732064656c 0000000000000006 ffff0000098e4000 ffff000008663128
bcc0: ffff000009738df8 ffff0000098e5150 00000000fffffff0 ffff000009752000
bce0: ffff000008261920 0000ffffb6667098 0000000000000001 ffff8000b6386788
bd00: ffff8000b6386000 ffff8000b6385f88 ffff8000bff60580 0000000000000000
bd20: ffff8000b6386788 ffff0000080e7760 ffff000009222ec0 0000000000000000
bd40: 0000000000000000 ffff00000d44bda0 ffff0000089480b8 ffff00000d44bda0
bd60: ffff0000088e3abc 0000000040000145 00000000014000c0 ffff8000bff60580
bd80: ffffffffffffffff ffff0000089480b0 ffff00000d44bda0 ffff0000088e3abc
[<ffff0000088e3abc>] usb_autopm_put_interface+0x14/0x48
[<ffff0000089480b8>] acm_softint+0xb8/0xd8
[<ffff0000080e75ec>] process_one_work+0x1d4/0x348
[<ffff0000080e77a8>] worker_thread+0x48/0x470
[<ffff0000080edaac>] kthread+0x12c/0x130
[<ffff000008084ed8>] ret_from_fork+0x10/0x18
Code: f0007281 910cc003 910003fd f9454022 (f9401801) 
---[ end trace 1b12fec59341c199 ]---

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [Bug 220052] New: The usb/cdc-acm driver uses memory after it is freed

[Greg KH]

On Thu, Apr 24, 2025 at 02:31:20PM +0000, bugzilla-daemon@kernel.org wrote:
> Created attachment 308013
>   --> https://bugzilla.kernel.org/attachment.cgi?id=308013&action=edit
> A patch file that solves the kernel panic.

Please submit this properly through email so that we can review and
apply it if it is correct.

thanks,

greg k-h

Re: [Bug 220052] New: The usb/cdc-acm driver uses memory after it is freed

[Ben Maan]

One of our customers complained about a kernel panic that occurs now and than after disconnecting a usb-cable. However, most of the disconnects are without a panic. After some investigation the panic shows up between 1 and 100 disconnect attempts. From the panic output can be seen that the acm_softint is one of the last functions on the stack . This function is first calling a usb_kill_urb that frees the urb memory but 2 lines further the allocated urb memory is still used in the acm_submit_read_urbs->acm_submit_read_urb.

Exchanging the 2 lines as indicated in the patch solves the problem completely and the kernel panic is not seen anymore. This is proved by 7000 'virtual' disconnects.

[Bug 220052] The usb/cdc-acm driver uses memory after it is freed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=220052

--- Comment #1 from gregkh@linuxfoundation.org ---
On Thu, Apr 24, 2025 at 02:31:20PM +0000, bugzilla-daemon@kernel.org wrote:
> Created attachment 308013
>   --> https://bugzilla.kernel.org/attachment.cgi?id=308013&action=edit
> A patch file that solves the kernel panic.

Please submit this properly through email so that we can review and
apply it if it is correct.

thanks,

greg k-h

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 220052] The usb/cdc-acm driver uses memory after it is freed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=220052

--- Comment #2 from Ben (ben.maan@aimvalley.com) ---
One of our customers complained about a kernel panic that occurs now and than
after disconnecting a usb-cable. However, most of the disconnects are without a
panic. After some investigation the panic shows up between 1 and 100 disconnect
attempts. From the panic output can be seen that the acm_softint is one of the
last functions on the stack . This function is first calling a usb_kill_urb
that frees the urb memory but 2 lines further the allocated urb memory is still
used in the acm_submit_read_urbs->acm_submit_read_urb.

Exchanging the 2 lines as indicated in the patch solves the problem completely
and the kernel panic is not seen anymore. This is proved by 7000 'virtual'
disconnects.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 220052] The usb/cdc-acm driver uses memory after it is freed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=220052

--- Comment #3 from Ben (ben.maan@aimvalley.com) ---
One of our customers complained about a kernel panic that occurs now and than
after disconnecting a usb-cable. However, most of the disconnects are without a
panic. After some investigation the panic shows up between 1 and 100 disconnect
attempts. From the panic output can be seen that the acm_softint is one of the
last functions on the stack . This function is first calling a usb_kill_urb
that frees the urb memory but 2 lines further the allocated urb memory is still
used in the acm_submit_read_urbs->acm_submit_read_urb.

Exchanging the 2 lines as indicated in the patch solves the problem completely
and the kernel panic is not seen anymore. This is proved by 7000 'virtual'
disconnects.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [Bug 220052] The usb/cdc-acm driver uses memory after it is freed

[Oliver Neukum]

On 30.04.25 14:56, bugzilla-daemon@kernel.org wrote:
  
> Exchanging the 2 lines as indicated in the patch solves the problem completely
> and the kernel panic is not seen anymore. This is proved by 7000 'virtual'
> disconnects.
> 

Yes, it solves your particular problem.

Now, with your patch, you first call
acm_submit_read_urbs() -> acm_submit_read_urb():

res = usb_submit_urb(acm->read_urbs[index], mem_flags);

And then you do

+                       for (i = 0; i < acm->rx_buflimit; i++)
+                               usb_kill_urb(acm->read_urbs[i]);

In other words, you undo what you just did.

Your diagnosis of the issue may be good, but the fix is _not_.
The purpose of acm_softint() is to

1. kill outstanding IO
2. error handling with usb_clear_halt()
3. restart the outstanding IO

You cannot reverse steps 1 and 3 and expect that to work.
The issue must be one of refcounting.

	Sorry
		Oliver

[Bug 220052] The usb/cdc-acm driver uses memory after it is freed

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=220052

--- Comment #4 from oneukum@suse.com ---
On 30.04.25 14:56, bugzilla-daemon@kernel.org wrote:

> Exchanging the 2 lines as indicated in the patch solves the problem
> completely
> and the kernel panic is not seen anymore. This is proved by 7000 'virtual'
> disconnects.
> 

Yes, it solves your particular problem.

Now, with your patch, you first call
acm_submit_read_urbs() -> acm_submit_read_urb():

res = usb_submit_urb(acm->read_urbs[index], mem_flags);

And then you do

+                       for (i = 0; i < acm->rx_buflimit; i++)
+                               usb_kill_urb(acm->read_urbs[i]);

In other words, you undo what you just did.

Your diagnosis of the issue may be good, but the fix is _not_.
The purpose of acm_softint() is to

1. kill outstanding IO
2. error handling with usb_clear_halt()
3. restart the outstanding IO

You cannot reverse steps 1 and 3 and expect that to work.
The issue must be one of refcounting.

        Sorry
                Oliver

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.