Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
@ 2026-08-17 13:55 Xin Chen
  2026-08-17 14:14 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Xin Chen @ 2026-08-17 13:55 UTC (permalink / raw)
  To: gregkh, jirislaby
  Cc: linux-kernel, linux-serial, liulzhao, cheng.jiang, Xin Chen

BT enable fails intermittently with -ETIMEDOUT (-110).  The kernel log
shows the HCI Read Local Version command was sent and the firmware
replied with status 0x00 (logged by hci_req_cmd_complete() BT_DBG),
but the waiter in __hci_cmd_sync_sk() never woke up and timed out
after 10 s:

  bluetooth hci0: Opcode 0xfc00              // __hci_cmd_sync_sk
  bluetooth hci0: opcode 0xfc00 plen 1       // hci_cmd_sync_add
  bluetooth hci0: skb len 4                  // hci_cmd_sync_alloc
  bluetooth hci0: length 1                   // hci_req_sync_run
  Bluetooth: hci0 cmd_cnt 1 cmd queued 1     // hci_cmd_work
  Bluetooth: hci0 type 1 len 4               // hci_send_frame
  Bluetooth: opcode 0xfc00 status 0x00       // hci_req_cmd_complete
  <-- req_skb NULL: req_complete_skb not set,
      hci_cmd_sync_complete() never called,
      req_status stays HCI_REQ_PEND            -->
  <-- 10 s later: wait_event_interruptible_timeout expires -->
  bluetooth hci0: end: err -110              // __hci_cmd_sync_sk

The root cause is that hci_send_cmd_sync() clones the sent command
into hdev->req_skb so that hci_req_cmd_complete() can locate the
registered completion callback.  Under memory pressure this
skb_clone() fails, leaving hdev->req_skb NULL.  The firmware reply
is received and processed, but hci_req_cmd_complete() finds NULL
req_skb, so hci_cmd_sync_complete() is never called, req_status
stays HCI_REQ_PEND, and the waiter times out with -ETIMEDOUT.

The memory pressure is caused by n_tty_open().  When a BT UART
transport is opened, serdev_device_open() may be called multiple
times in quick succession, each triggering n_tty_open().  n_tty_open()
uses vzalloc() for the ~10 KB n_tty_data structure, which always
allocates page-by-page from the buddy order-0 free list.  Repeated
vzalloc() calls drain enough order-0 pages that the subsequent
skb_clone(GFP_KERNEL) in hci_send_cmd_sync() cannot get a page.

Replace vzalloc/vfree with kvzalloc/kvfree.  kvzalloc() tries
kmalloc first and falls back to vmalloc only on failure.  The
~10 KB n_tty_data is served from the kmalloc-16384 slab (backed
by an order-2 compound page), leaving the order-0 free list intact
for the subsequent skb_clone() calls.

Note: checkpatch warns "Prefer kvzalloc_obj over kvzalloc with
sizeof", but kvzalloc_obj() was introduced after v6.6 and is not
available in the target kernel, so kvzalloc(sizeof(*ldata),
GFP_KERNEL) is used intentionally.

This issue was first observed as a use-after-free in ttyport_close()
when ttyport_open() failed, which was investigated in an earlier
patch series [1].  That investigation led to the discovery of the
true root cause described above.

[1] https://lore.kernel.org/all/20250430111617.1151390-1-quic_cxin@quicinc.com/

Signed-off-by: Xin Chen <xin.chen2@oss.qualcomm.com>
---
 drivers/tty/n_tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..7ace11f6d600 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1870,7 +1870,7 @@ static void n_tty_close(struct tty_struct *tty)
 		n_tty_packet_mode_flush(tty);
 
 	guard(rwsem_write)(&tty->termios_rwsem);
-	vfree(ldata);
+	kvfree(ldata);
 	tty->disc_data = NULL;
 }
 
@@ -1887,7 +1887,7 @@ static int n_tty_open(struct tty_struct *tty)
 	struct n_tty_data *ldata;
 
 	/* Currently a malloc failure here can panic */
-	ldata = vzalloc(sizeof(*ldata));
+	ldata = kvzalloc(sizeof(*ldata), GFP_KERNEL);
 	if (!ldata)
 		return -ENOMEM;
 
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 13:55 [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data Xin Chen
@ 2026-08-17 14:14 ` Greg KH
  2026-08-18  3:09   ` Xin Chen
  2026-08-17 14:16 ` Greg KH
  2026-08-18  7:03 ` [PATCH v2] " Xin Chen
  2 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2026-08-17 14:14 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang

On Mon, Aug 17, 2026 at 09:55:26PM +0800, Xin Chen wrote:
> Note: checkpatch warns "Prefer kvzalloc_obj over kvzalloc with
> sizeof", but kvzalloc_obj() was introduced after v6.6 and is not
> available in the target kernel, so kvzalloc(sizeof(*ldata),
> GFP_KERNEL) is used intentionally.

What do you mean "target kernel"?  This is for 7.3-final, right?

confused,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 13:55 [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data Xin Chen
  2026-08-17 14:14 ` Greg KH
@ 2026-08-17 14:16 ` Greg KH
  2026-08-17 14:49   ` Greg KH
  2026-08-18  3:31   ` Xin Chen
  2026-08-18  7:03 ` [PATCH v2] " Xin Chen
  2 siblings, 2 replies; 17+ messages in thread
From: Greg KH @ 2026-08-17 14:16 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang

On Mon, Aug 17, 2026 at 09:55:26PM +0800, Xin Chen wrote:
> BT enable fails intermittently with -ETIMEDOUT (-110).  The kernel log
> shows the HCI Read Local Version command was sent and the firmware
> replied with status 0x00 (logged by hci_req_cmd_complete() BT_DBG),
> but the waiter in __hci_cmd_sync_sk() never woke up and timed out
> after 10 s:
> 
>   bluetooth hci0: Opcode 0xfc00              // __hci_cmd_sync_sk
>   bluetooth hci0: opcode 0xfc00 plen 1       // hci_cmd_sync_add
>   bluetooth hci0: skb len 4                  // hci_cmd_sync_alloc
>   bluetooth hci0: length 1                   // hci_req_sync_run
>   Bluetooth: hci0 cmd_cnt 1 cmd queued 1     // hci_cmd_work
>   Bluetooth: hci0 type 1 len 4               // hci_send_frame
>   Bluetooth: opcode 0xfc00 status 0x00       // hci_req_cmd_complete
>   <-- req_skb NULL: req_complete_skb not set,
>       hci_cmd_sync_complete() never called,
>       req_status stays HCI_REQ_PEND            -->
>   <-- 10 s later: wait_event_interruptible_timeout expires -->
>   bluetooth hci0: end: err -110              // __hci_cmd_sync_sk
> 
> The root cause is that hci_send_cmd_sync() clones the sent command
> into hdev->req_skb so that hci_req_cmd_complete() can locate the
> registered completion callback.  Under memory pressure this
> skb_clone() fails, leaving hdev->req_skb NULL.  The firmware reply
> is received and processed, but hci_req_cmd_complete() finds NULL
> req_skb, so hci_cmd_sync_complete() is never called, req_status
> stays HCI_REQ_PEND, and the waiter times out with -ETIMEDOUT.
> 
> The memory pressure is caused by n_tty_open().  When a BT UART
> transport is opened, serdev_device_open() may be called multiple
> times in quick succession, each triggering n_tty_open().  n_tty_open()
> uses vzalloc() for the ~10 KB n_tty_data structure, which always
> allocates page-by-page from the buddy order-0 free list.  Repeated
> vzalloc() calls drain enough order-0 pages that the subsequent
> skb_clone(GFP_KERNEL) in hci_send_cmd_sync() cannot get a page.

So you run out of memory?  That feels wrong.

Why not just use a specific slab for this one structure if it is so
important that it never run out?  Why was this using vzalloc() in the
first place if it could fail?

And if it does fail, doesn't everything work properly, you just need to
handle that failure in userspace correctly, right?  What is failing that
you can not recover?  If we are running out of memory here for such a
tiny allocation, odds are other things are going to go wrong so
userspace better handle that.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 14:16 ` Greg KH
@ 2026-08-17 14:49   ` Greg KH
  2026-08-18  6:02     ` Xin Chen
  2026-08-18  3:31   ` Xin Chen
  1 sibling, 1 reply; 17+ messages in thread
From: Greg KH @ 2026-08-17 14:49 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang

On Mon, Aug 17, 2026 at 04:16:53PM +0200, Greg KH wrote:
> On Mon, Aug 17, 2026 at 09:55:26PM +0800, Xin Chen wrote:
> > BT enable fails intermittently with -ETIMEDOUT (-110).  The kernel log
> > shows the HCI Read Local Version command was sent and the firmware
> > replied with status 0x00 (logged by hci_req_cmd_complete() BT_DBG),
> > but the waiter in __hci_cmd_sync_sk() never woke up and timed out
> > after 10 s:
> > 
> >   bluetooth hci0: Opcode 0xfc00              // __hci_cmd_sync_sk
> >   bluetooth hci0: opcode 0xfc00 plen 1       // hci_cmd_sync_add
> >   bluetooth hci0: skb len 4                  // hci_cmd_sync_alloc
> >   bluetooth hci0: length 1                   // hci_req_sync_run
> >   Bluetooth: hci0 cmd_cnt 1 cmd queued 1     // hci_cmd_work
> >   Bluetooth: hci0 type 1 len 4               // hci_send_frame
> >   Bluetooth: opcode 0xfc00 status 0x00       // hci_req_cmd_complete
> >   <-- req_skb NULL: req_complete_skb not set,
> >       hci_cmd_sync_complete() never called,
> >       req_status stays HCI_REQ_PEND            -->
> >   <-- 10 s later: wait_event_interruptible_timeout expires -->
> >   bluetooth hci0: end: err -110              // __hci_cmd_sync_sk
> > 
> > The root cause is that hci_send_cmd_sync() clones the sent command
> > into hdev->req_skb so that hci_req_cmd_complete() can locate the
> > registered completion callback.  Under memory pressure this
> > skb_clone() fails, leaving hdev->req_skb NULL.  The firmware reply
> > is received and processed, but hci_req_cmd_complete() finds NULL
> > req_skb, so hci_cmd_sync_complete() is never called, req_status
> > stays HCI_REQ_PEND, and the waiter times out with -ETIMEDOUT.
> > 
> > The memory pressure is caused by n_tty_open().  When a BT UART
> > transport is opened, serdev_device_open() may be called multiple
> > times in quick succession, each triggering n_tty_open().  n_tty_open()
> > uses vzalloc() for the ~10 KB n_tty_data structure, which always
> > allocates page-by-page from the buddy order-0 free list.  Repeated
> > vzalloc() calls drain enough order-0 pages that the subsequent
> > skb_clone(GFP_KERNEL) in hci_send_cmd_sync() cannot get a page.
> 
> So you run out of memory?  That feels wrong.

Also, you are papering over the real problem here.  If this one
allocation is failing, what keeps the next one from failing and then the
skb will not be able to be allocated?

Why is the system so out of memory in this slab that this is happening?
What changed in the tty layer to cause this?  Or did it happen
elsewhere?

And no cc: stable or Fixes: tag?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 14:14 ` Greg KH
@ 2026-08-18  3:09   ` Xin Chen
  2026-08-18  6:05     ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Xin Chen @ 2026-08-18  3:09 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin


On 8/17/2026 10:14 PM, Greg KH wrote:
> What do you mean "target kernel"?  This is for 7.3-final, right?

Sorry for the confusion. This patch is based on linux-next 7.2.0-rc7,
targeting 7.2-final (or 7.3 if the merge window has already closed).
The wording "target kernel" was poor; what I meant is that
kvzalloc(sizeof(*ldata), GFP_KERNEL) is used intentionally to keep
this patch cleanly backportable to kernels older than v6.6 where
kvzalloc_obj() is not yet available.

Thanks,
Xin Chen


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 14:16 ` Greg KH
  2026-08-17 14:49   ` Greg KH
@ 2026-08-18  3:31   ` Xin Chen
  2026-08-18  6:07     ` Greg KH
  1 sibling, 1 reply; 17+ messages in thread
From: Xin Chen @ 2026-08-18  3:31 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Mon, Aug 17, 2026, Greg KH wrote:
 > So you run out of memory?  That feels wrong.

Not a full OOM — just a transient exhaustion of order-0 pages caused
by repeated vzalloc() calls each draining the buddy order-0 free list.
The system recovers quickly, but the damage is already done by then.

 > Why not just use a specific slab for this one structure if it is so
 > important that it never run out?

kvzalloc() already achieves that: it tries kmalloc() first, which
serves the ~10 KB n_tty_data from the kmalloc-16384 slab (an order-2
compound page), leaving the order-0 free list intact. A dedicated slab
would add complexity without further benefit.

 > Why was this using vzalloc() in the first place if it could fail?

Historically, ~10 KB was considered too large for kmalloc(), so
vzalloc() was used. kvzalloc() is the natural modern replacement: it
tries kmalloc() first and falls back to vmalloc() only on failure,
which is strictly better.

 > And if it does fail, doesn't everything work properly, you just need
 > to handle that failure in userspace correctly, right?

Even if the error were surfaced correctly to userspace, there is
nothing useful it can do. The actual failure here is that skb_clone()
in hci_send_cmd_sync() silently fails due to the depleted order-0
free list, leaving hdev->req_skb NULL. The firmware reply arrives and
is processed, but hci_req_cmd_complete() cannot find the completion
callback, so the waiter times out with -ETIMEDOUT. From userspace's
perspective this looks like a hardware or firmware timeout, not a
memory issue. Even if userspace retried BT enable, it would trigger
serdev_device_open() again, which calls n_tty_open() again, which
calls vzalloc() again — further draining the order-0 free list and
making recovery harder. The root fix is to stop consuming order-0
pages unnecessarily in the first place, which is exactly what
switching to kvzalloc() achieves.

Thanks,
Xin Chen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 14:49   ` Greg KH
@ 2026-08-18  6:02     ` Xin Chen
  2026-08-18  6:34       ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Xin Chen @ 2026-08-18  6:02 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang

On Mon, Aug 17, 2026, Greg KH wrote:
 > Also, you are papering over the real problem here.  If this one
 > allocation is failing, what keeps the next one from failing and then
 > the skb will not be able to be allocated?

The key difference is allocation size and allocator behavior.
vzalloc() always allocates page-by-page from the buddy order-0 free
list, so two back-to-back vzalloc() calls for ~10 KB each consume
~5 order-0 pages each, transiently depleting the order-0 free list.
kvzalloc() serves the same ~10 KB from the kmalloc-16384 slab, which
is backed by order-2 compound pages — a completely separate pool from
the order-0 pages that skb_clone(GFP_KERNEL) needs. So switching to
kvzalloc() eliminates the interference between n_tty_open() and
skb_clone().

 > Why is the system so out of memory in this slab that this is
 > happening?  What changed in the tty layer to cause this?  Or did it
 > happen elsewhere?

Nothing changed recently in the tty layer. vzalloc() has been used
here since commit ebec3f8f5271 ("n_tty: Access echo_* variables
carefully.", 2018), which replaced vmalloc() with vzalloc(). The
issue surfaces only when serdev_device_open() is called multiple times
in quick succession (as happens during BT UART transport init),
triggering multiple n_tty_open() calls back-to-back. Each vzalloc()
drains order-0 pages, and the window where skb_clone() fails is
narrow but reproducible under this specific pattern. It was found
during a BT enable-disable sanity test that repeatedly cycles BT on
and off, which consistently triggers the back-to-back n_tty_open()
calls that expose the issue.

 > And no cc: stable or Fixes: tag?

Both will be added in v2:

   Fixes: ebec3f8f5271 ("n_tty: Access echo_* variables carefully.")
   Cc: stable@vger.kernel.org

Thanks,
Xin Chen


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  3:09   ` Xin Chen
@ 2026-08-18  6:05     ` Greg KH
  2026-08-18  6:22       ` Xin Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2026-08-18  6:05 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Tue, Aug 18, 2026 at 11:09:49AM +0800, Xin Chen wrote:
> 
> On 8/17/2026 10:14 PM, Greg KH wrote:
> > What do you mean "target kernel"?  This is for 7.3-final, right?
> 
> Sorry for the confusion. This patch is based on linux-next 7.2.0-rc7,
> targeting 7.2-final (or 7.3 if the merge window has already closed).
> The wording "target kernel" was poor; what I meant is that
> kvzalloc(sizeof(*ldata), GFP_KERNEL) is used intentionally to keep
> this patch cleanly backportable to kernels older than v6.6 where
> kvzalloc_obj() is not yet available.

That doesn't matter when getting patches merged, otherwise no one would
ever use the newer and proper apis.  Please write code for Linus's tree
first.  Worry about older kernels later after changes are merged
properly.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  3:31   ` Xin Chen
@ 2026-08-18  6:07     ` Greg KH
  2026-08-18  6:39       ` Xin Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2026-08-18  6:07 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Tue, Aug 18, 2026 at 11:31:14AM +0800, Xin Chen wrote:
> On Mon, Aug 17, 2026, Greg KH wrote:
> > So you run out of memory?  That feels wrong.
> 
> Not a full OOM — just a transient exhaustion of order-0 pages caused
> by repeated vzalloc() calls each draining the buddy order-0 free list.
> The system recovers quickly, but the damage is already done by then.

What specific "damage"?

> > Why not just use a specific slab for this one structure if it is so
> > important that it never run out?
> 
> kvzalloc() already achieves that: it tries kmalloc() first, which
> serves the ~10 KB n_tty_data from the kmalloc-16384 slab (an order-2
> compound page), leaving the order-0 free list intact. A dedicated slab
> would add complexity without further benefit.
> 
> > Why was this using vzalloc() in the first place if it could fail?
> 
> Historically, ~10 KB was considered too large for kmalloc(), so
> vzalloc() was used. kvzalloc() is the natural modern replacement: it
> tries kmalloc() first and falls back to vmalloc() only on failure,
> which is strictly better.
> 
> > And if it does fail, doesn't everything work properly, you just need
> > to handle that failure in userspace correctly, right?
> 
> Even if the error were surfaced correctly to userspace, there is
> nothing useful it can do. The actual failure here is that skb_clone()
> in hci_send_cmd_sync() silently fails due to the depleted order-0
> free list, leaving hdev->req_skb NULL. The firmware reply arrives and
> is processed, but hci_req_cmd_complete() cannot find the completion
> callback, so the waiter times out with -ETIMEDOUT. From userspace's
> perspective this looks like a hardware or firmware timeout, not a
> memory issue. Even if userspace retried BT enable, it would trigger
> serdev_device_open() again, which calls n_tty_open() again, which
> calls vzalloc() again — further draining the order-0 free list and
> making recovery harder. The root fix is to stop consuming order-0
> pages unnecessarily in the first place, which is exactly what
> switching to kvzalloc() achieves.

But it's not consuming them "unnecessarily" as the memory is needed.
Why not fix the root problem here of having this be called so many times
that you are running out of memory?

And why isn't memory being reclaimed properly if we do not have any left
in that free list?  The allocation can sleep, so it should be always
succeeding if the system isn't truely out of memory, as you imply it is
not.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  6:05     ` Greg KH
@ 2026-08-18  6:22       ` Xin Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Xin Chen @ 2026-08-18  6:22 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Tue, Aug 18, 2026, Greg KH wrote:
 > That doesn't matter when getting patches merged, otherwise no one would
 > ever use the newer and proper apis.  Please write code for Linus's tree
 > first.  Worry about older kernels later after changes are merged
 > properly.

Understood, thanks for the clarification. Switched to kvzalloc_obj() in
v2.

Thanks,
Xin Chen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  6:02     ` Xin Chen
@ 2026-08-18  6:34       ` Greg KH
  2026-08-18  6:58         ` Xin Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2026-08-18  6:34 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang

On Tue, Aug 18, 2026 at 02:02:43PM +0800, Xin Chen wrote:
> On Mon, Aug 17, 2026, Greg KH wrote:
> > Also, you are papering over the real problem here.  If this one
> > allocation is failing, what keeps the next one from failing and then
> > the skb will not be able to be allocated?
> 
> The key difference is allocation size and allocator behavior.
> vzalloc() always allocates page-by-page from the buddy order-0 free
> list, so two back-to-back vzalloc() calls for ~10 KB each consume
> ~5 order-0 pages each, transiently depleting the order-0 free list.
> kvzalloc() serves the same ~10 KB from the kmalloc-16384 slab, which
> is backed by order-2 compound pages — a completely separate pool from
> the order-0 pages that skb_clone(GFP_KERNEL) needs. So switching to
> kvzalloc() eliminates the interference between n_tty_open() and
> skb_clone().

But that's not a problem with the tty layer, if something else happens
to "drain" the pool again you can not create a skb.  You are not solving
the root problem here.

> > Why is the system so out of memory in this slab that this is
> > happening?  What changed in the tty layer to cause this?  Or did it
> > happen elsewhere?
> 
> Nothing changed recently in the tty layer. vzalloc() has been used
> here since commit ebec3f8f5271 ("n_tty: Access echo_* variables
> carefully.", 2018), which replaced vmalloc() with vzalloc().

But that's not really a change, when was vmalloc() first used?

As nothing has changed here, then why is this suddenly showing up now?

> The
> issue surfaces only when serdev_device_open() is called multiple times
> in quick succession (as happens during BT UART transport init),
> triggering multiple n_tty_open() calls back-to-back. Each vzalloc()
> drains order-0 pages, and the window where skb_clone() fails is
> narrow but reproducible under this specific pattern. It was found
> during a BT enable-disable sanity test that repeatedly cycles BT on
> and off, which consistently triggers the back-to-back n_tty_open()
> calls that expose the issue.

Again, that sounds like a bluetooth issue, and why can't you just
properly handle the skb out of memory issue?

This feels like papering over the real problem.

> > And no cc: stable or Fixes: tag?
> 
> Both will be added in v2:
> 
>   Fixes: ebec3f8f5271 ("n_tty: Access echo_* variables carefully.")

No, that did not change the behavior of the tty call here to use a
different pool, all it did was change the zeroing out of the buffer
allocated.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  6:07     ` Greg KH
@ 2026-08-18  6:39       ` Xin Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Xin Chen @ 2026-08-18  6:39 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Mon, Aug 18, 2026, Greg KH wrote:
 > What specific "damage"?

The specific damage is that hci_send_cmd_sync() calls
skb_clone(hdev->sent_cmd, GFP_KERNEL) to set hdev->req_skb. If
that clone fails, hdev->req_skb stays NULL.  When the firmware reply
arrives, hci_req_cmd_complete() checks hdev->req_skb to find the
registered completion callback; finding NULL, it never calls
hci_cmd_sync_complete(), req_status stays HCI_REQ_PEND, and the
waiter in __hci_cmd_sync_sk() times out with -ETIMEDOUT.  The command
was sent and the firmware replied successfully — the damage is purely
that the completion path is broken.

 > But it's not consuming them "unnecessarily" as the memory is needed.
 > Why not fix the root problem here of having this be called so many
 > times that you are running out of memory?

The repeated calls are a normal consequence of the serdev open/close
retry logic in the BT transport layer; constraining that would require
changes in a different subsystem and would not address the underlying
fragility.  The real question is why skb_clone(GFP_KERNEL) fails at
all when the system is not truly OOM.

 > And why isn't memory being reclaimed properly if we do not have any
 > left in that free list?  The allocation can sleep, so it should be
 > always succeeding if the system isn't truly out of memory, as you
 > imply it is not.

This is the crux of the issue.  vzalloc() allocates order-0 pages
through vm_area_alloc_pages(), which takes the bulk allocation path
(alloc_pages_bulk_noprof) for order-0.  The bulk allocator uses
ALLOC_WMARK_LOW and does not perform direct reclaim — it is
intentionally a fast, non-sleeping path.  When the low watermark check
fails it falls through to goto failed, and vm_area_alloc_pages()
falls back to single-page alloc_pages() calls which can reclaim.
So vzalloc() itself can succeed even under pressure, but only after
consuming whatever order-0 pages were available via the bulk path first.

The problem is that skb_clone(GFP_KERNEL) in hci_send_cmd_sync()
is a single kmalloc-backed allocation.  It does not retry on failure
and has no reclaim loop of its own; if the zone is below the low
watermark at the moment it runs, it fails and returns NULL.  The window
is narrow but real: the bulk path in vzalloc() drains the PCP/buddy
order-0 lists below the low watermark; skb_clone() runs before
kswapd has had a chance to refill them; it fails silently.

Switching to kvzalloc_obj() serves the ~10 KB n_tty_data from the
kmalloc-16384 slab (an order-2 compound page), which does not touch
the order-0 free list at all, eliminating the pressure window entirely.

Thanks,
Xin Chen


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  6:34       ` Greg KH
@ 2026-08-18  6:58         ` Xin Chen
  2026-08-18  7:34           ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Xin Chen @ 2026-08-18  6:58 UTC (permalink / raw)
  To: Greg KH; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Tue, Aug 18, 2026, Greg KH wrote:
 > But that's not a problem with the tty layer, if something else happens
 > to "drain" the pool again you can not create a skb.  You are not
 > solving the root problem here.

You are right that this does not prevent every possible order-0
exhaustion. However, the specific and reproducible trigger is
n_tty_open() consuming order-0 pages via vzalloc() immediately before
skb_clone() runs. Eliminating that unnecessary pressure removes the
failure in practice, even if it does not make skb_clone() immune to
all possible memory pressure.

 > But that's not really a change, when was vmalloc() first used?
 > As nothing has changed here, then why is this suddenly showing up now?

ldata was originally allocated with kzalloc() (introduced in commit
70ece7a73159, "TTY: n_tty, add ldisc data to n_tty", 2012).  Commit
ebec3f8f5271 switched it to vmalloc()/vzalloc() in 2018 as a side
effect of fixing an echo buffer race — the allocation change was
incidental, not intentional.  The issue surfaces now because the BT
enable-disable sanity test exercises a back-to-back open pattern that
was not common before serdev-based UART transports became widespread.

 > Again, that sounds like a bluetooth issue, and why can't you just
 > properly handle the skb out of memory issue?

The skb_clone() failure is silent — it returns NULL and the code
continues without error, leaving hdev->req_skb NULL.  By the time
the BT layer observes the problem (a -ETIMEDOUT 10 seconds later),
it is several layers removed from the skb_clone() failure: the
firmware has already replied successfully, hci_req_cmd_complete()
has already run and found req_skb NULL, and the completion callback
was never invoked.  At that point the BT layer has no way to
distinguish a memory failure from a genuine firmware timeout, let
alone recover from it.  And even if the NULL req_skb were detected
and surfaced as an error immediately, there is nothing the BT layer
could do to recover — it cannot reclaim memory or retry the
allocation itself.  The only option would be to wait for the memory
to be reclaimed and retry the entire BT enable sequence from
userspace, which is exactly the kind of fragile error handling we
want to avoid.  The tty change is simpler and correct: ldata was
originally a kzalloc() allocation and there is no reason for it to
use vmalloc-backed pages that interfere with unrelated allocations.

 > No, that did not change the behavior of the tty call here to use a
 > different pool, all it did was change the zeroing out of the buffer
 > allocated.

You are correct, I apologize for the wrong Fixes: tag.  The switch
from kzalloc() to vmalloc() was introduced by commit 20bafb3d23d1
("n_tty: Move buffers into n_tty_data", 2013), which merged the
read_buf and echo_buf (each 4 KB) into n_tty_data, making the
structure too large for kmalloc() at the time.  That is the correct
Fixes: tag.  I will update it in v2:

   Fixes: 20bafb3d23d1 ("n_tty: Move buffers into n_tty_data")

Thanks,
Xin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-17 13:55 [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data Xin Chen
  2026-08-17 14:14 ` Greg KH
  2026-08-17 14:16 ` Greg KH
@ 2026-08-18  7:03 ` Xin Chen
  2026-08-18  7:15   ` Jiri Slaby
  2 siblings, 1 reply; 17+ messages in thread
From: Xin Chen @ 2026-08-18  7:03 UTC (permalink / raw)
  To: gregkh, jirislaby
  Cc: linux-kernel, linux-serial, stable, liulzhao, cheng.jiang, cxin,
	Xin Chen

BT enable fails intermittently with -ETIMEDOUT (-110).  The kernel log
shows the HCI Read Local Version command was sent and the firmware
replied with status 0x00 (logged by hci_req_cmd_complete() BT_DBG),
but the waiter in __hci_cmd_sync_sk() never woke up and timed out
after 10 s:

  bluetooth hci0: Opcode 0xfc00              // __hci_cmd_sync_sk
  bluetooth hci0: opcode 0xfc00 plen 1       // hci_cmd_sync_add
  bluetooth hci0: skb len 4                  // hci_cmd_sync_alloc
  bluetooth hci0: length 1                   // hci_req_sync_run
  Bluetooth: hci0 cmd_cnt 1 cmd queued 1     // hci_cmd_work
  Bluetooth: hci0 type 1 len 4               // hci_send_frame
  Bluetooth: opcode 0xfc00 status 0x00       // hci_req_cmd_complete
  <-- req_skb NULL: req_complete_skb not set,
      hci_cmd_sync_complete() never called,
      req_status stays HCI_REQ_PEND            -->
  <-- 10 s later: wait_event_interruptible_timeout expires -->
  bluetooth hci0: end: err -110              // __hci_cmd_sync_sk

The root cause is that hci_send_cmd_sync() clones the sent command
into hdev->req_skb so that hci_req_cmd_complete() can locate the
registered completion callback.  Under memory pressure this
skb_clone() fails, leaving hdev->req_skb NULL.  The firmware reply
is received and processed, but hci_req_cmd_complete() finds NULL
req_skb, so hci_cmd_sync_complete() is never called, req_status
stays HCI_REQ_PEND, and the waiter times out with -ETIMEDOUT.

The memory pressure is caused by n_tty_open().  When a BT UART
transport is opened, serdev_device_open() may be called multiple
times in quick succession, each triggering n_tty_open().  n_tty_open()
uses vzalloc() for the ~10 KB n_tty_data structure, which always
allocates page-by-page from the buddy order-0 free list.  Repeated
vzalloc() calls drain enough order-0 pages that the subsequent
skb_clone(GFP_KERNEL) in hci_send_cmd_sync() cannot get a page.

Replace vzalloc/vfree with kvzalloc_obj/kvfree.  kvzalloc_obj() tries
kmalloc first and falls back to vmalloc only on failure.  The
~10 KB n_tty_data is served from the kmalloc-16384 slab (backed
by an order-2 compound page), leaving the order-0 free list intact
for the subsequent skb_clone() calls.

This issue was first observed as a use-after-free in ttyport_close()
when ttyport_open() failed, which was investigated in an earlier
patch series [1].  That investigation led to the discovery of the
true root cause described above.

[1] https://lore.kernel.org/all/20250430111617.1151390-1-quic_cxin@quicinc.com/

Fixes: 20bafb3d23d1 ("n_tty: Move buffers into n_tty_data")
Cc: stable@vger.kernel.org
Signed-off-by: Xin Chen <xin.chen2@oss.qualcomm.com>
---
Changes in v2:
- Add Fixes: tag pointing to 20bafb3d23d1
- Add Cc: stable@vger.kernel.org
- Drop the Note paragraph about kvzalloc_obj
- Use kvzalloc_obj() instead of kvzalloc(sizeof(*ldata), GFP_KERNEL)

 drivers/tty/n_tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e6a0f5b40d0a..81baef6110c0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1870,7 +1870,7 @@ static void n_tty_close(struct tty_struct *tty)
 		n_tty_packet_mode_flush(tty);
 
 	guard(rwsem_write)(&tty->termios_rwsem);
-	vfree(ldata);
+	kvfree(ldata);
 	tty->disc_data = NULL;
 }
 
@@ -1887,7 +1887,7 @@ static int n_tty_open(struct tty_struct *tty)
 	struct n_tty_data *ldata;
 
 	/* Currently a malloc failure here can panic */
-	ldata = vzalloc(sizeof(*ldata));
+	ldata = kvzalloc_obj(ldata);
 	if (!ldata)
 		return -ENOMEM;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  7:03 ` [PATCH v2] " Xin Chen
@ 2026-08-18  7:15   ` Jiri Slaby
  2026-08-18  7:23     ` Xin Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Jiri Slaby @ 2026-08-18  7:15 UTC (permalink / raw)
  To: Xin Chen, gregkh
  Cc: linux-kernel, linux-serial, stable, liulzhao, cheng.jiang, cxin

On 18. 08. 26, 9:03, Xin Chen wrote:
> Replace vzalloc/vfree with kvzalloc_obj/kvfree.  kvzalloc_obj() tries
> kmalloc first and falls back to vmalloc only on failure.  The
> ~10 KB n_tty_data is served from the kmalloc-16384 slab (backed
> by an order-2 compound page), leaving the order-0 free list intact
> for the subsequent skb_clone() calls.

Switching from order-0 to order-2? If you ran out of vmspace on some 
32bit platform, perhaps. But you apparently did not. So all this feels odd.

NACK

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  7:15   ` Jiri Slaby
@ 2026-08-18  7:23     ` Xin Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Xin Chen @ 2026-08-18  7:23 UTC (permalink / raw)
  To: Jiri Slaby, gregkh
  Cc: linux-kernel, linux-serial, stable, liulzhao, cheng.jiang, cxin

On Tue, Aug 18, 2026, Jiri Slaby wrote:
 > Switching from order-0 to order-2? If you ran out of vmspace on some
 > 32bit platform, perhaps. But you apparently did not. So all this
 > feels odd.

To clarify: the concern is not vmalloc address space exhaustion.
The issue is that vzalloc() allocates order-0 pages from the buddy
allocator via the bulk allocation path (alloc_pages_bulk_noprof),
which uses ALLOC_WMARK_LOW and does not perform direct reclaim.
When two back-to-back vzalloc() calls drain enough order-0 pages
to push the zone below the low watermark, a subsequent
skb_clone(GFP_KERNEL) in hci_send_cmd_sync() fails silently,
leaving hdev->req_skb NULL and causing BT enable to time out with
-ETIMEDOUT.

kvzalloc_obj() serves the ~10 KB n_tty_data from the kmalloc-16384
slab, which is backed by order-2 compound pages — a separate pool
that does not deplete the order-0 free list that skb_clone() depends
on.  The commit message could have been clearer on this point; I will
improve it in the next version.

Thanks,
Xin Chen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
  2026-08-18  6:58         ` Xin Chen
@ 2026-08-18  7:34           ` Greg KH
  0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2026-08-18  7:34 UTC (permalink / raw)
  To: Xin Chen; +Cc: jirislaby, linux-kernel, linux-serial, liulzhao, cheng.jiang,
	cxin

On Tue, Aug 18, 2026 at 02:58:41PM +0800, Xin Chen wrote:
> On Tue, Aug 18, 2026, Greg KH wrote:
> > But that's not a problem with the tty layer, if something else happens
> > to "drain" the pool again you can not create a skb.  You are not
> > solving the root problem here.
> 
> You are right that this does not prevent every possible order-0
> exhaustion. However, the specific and reproducible trigger is
> n_tty_open() consuming order-0 pages via vzalloc() immediately before
> skb_clone() runs. Eliminating that unnecessary pressure removes the
> failure in practice, even if it does not make skb_clone() immune to
> all possible memory pressure.

Then you are going to play whack-a-mole on your very memory-constrained
system in order to work around the root problem here.  Please don't do
that, solve the real problem you are having.

> > But that's not really a change, when was vmalloc() first used?
> > As nothing has changed here, then why is this suddenly showing up now?
> 
> ldata was originally allocated with kzalloc() (introduced in commit
> 70ece7a73159, "TTY: n_tty, add ldisc data to n_tty", 2012).  Commit
> ebec3f8f5271 switched it to vmalloc()/vzalloc() in 2018 as a side
> effect of fixing an echo buffer race — the allocation change was
> incidental, not intentional.  The issue surfaces now because the BT
> enable-disable sanity test exercises a back-to-back open pattern that
> was not common before serdev-based UART transports became widespread.

So this is purely because you are stress-testing the BT stack now,
right?

> > Again, that sounds like a bluetooth issue, and why can't you just
> > properly handle the skb out of memory issue?
> 
> The skb_clone() failure is silent — it returns NULL and the code
> continues without error, leaving hdev->req_skb NULL.

Why not fix that?

> By the time
> the BT layer observes the problem (a -ETIMEDOUT 10 seconds later),
> it is several layers removed from the skb_clone() failure: the
> firmware has already replied successfully, hci_req_cmd_complete()
> has already run and found req_skb NULL, and the completion callback
> was never invoked.

Again, fix that.

> At that point the BT layer has no way to
> distinguish a memory failure from a genuine firmware timeout, let
> alone recover from it. 

But that's not a TTY layer issue, it's a BT issue.  Please fix that.

> And even if the NULL req_skb were detected
> and surfaced as an error immediately, there is nothing the BT layer
> could do to recover — it cannot reclaim memory or retry the
> allocation itself.

Nor should it, it should handle the error properly and recover
correctly.

>  The only option would be to wait for the memory
> to be reclaimed and retry the entire BT enable sequence from
> userspace, which is exactly the kind of fragile error handling we
> want to avoid.

No, you need to handle the error properly because it could happen at any
point in time.

> The tty change is simpler and correct: ldata was originally a
> kzalloc() allocation and there is no reason for it to use
> vmalloc-backed pages that interfere with unrelated allocations.

It is not "correct", but rather papering over the root problem.

Fix the bluetooth stack please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-08-18  7:34 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 13:55 [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data Xin Chen
2026-08-17 14:14 ` Greg KH
2026-08-18  3:09   ` Xin Chen
2026-08-18  6:05     ` Greg KH
2026-08-18  6:22       ` Xin Chen
2026-08-17 14:16 ` Greg KH
2026-08-17 14:49   ` Greg KH
2026-08-18  6:02     ` Xin Chen
2026-08-18  6:34       ` Greg KH
2026-08-18  6:58         ` Xin Chen
2026-08-18  7:34           ` Greg KH
2026-08-18  3:31   ` Xin Chen
2026-08-18  6:07     ` Greg KH
2026-08-18  6:39       ` Xin Chen
2026-08-18  7:03 ` [PATCH v2] " Xin Chen
2026-08-18  7:15   ` Jiri Slaby
2026-08-18  7:23     ` Xin Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox