[PATCH net v2 0/1] serial: caif: fix remaining ser->tty UAF in TX

[PATCH net v2 0/1] serial: caif: fix remaining ser->tty UAF in TX

[Shuangpeng Bai]

This is v2 of the patch previously posted as
"[PATCH] net: caif: serial: fix TX UAF on ser->tty".

Changes since v1:
  - Send to the net tree as [PATCH net v2]
  - Base on netdev/main and ensure the patch applies cleanly
  - Update the commit message and tags to satisfy checkpatch and netdev
    submission requirements

Link: https://lore.kernel.org/all/20260212042236.639174-1-shuangpeng.kernel@gmail.com/

Shuangpeng Bai (1):
  serial: caif: fix remaining ser->tty UAF in TX path

 drivers/net/caif/caif_serial.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Shuangpeng Bai]

A reproducer exposes a KASAN use-after-free in caif_serial's TX path
(e.g., via tty_write_room() / tty->ops->write()) on top of commit
<308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
ldisc_close()").

That commit moved tty_kref_put() to ser_release(). There is still a race
because the TX path may fetch ser->tty and use it while ser_release()
drops the last tty reference:

    CPU 0 (ser_release worker)         CPU 1 (xmit)
    -------------------------        ------------
                                      caif_xmit()
                                      handle_tx()
                                        tty = ser->tty

    ser_release()
      tty = ser->tty
      dev_close(ser->dev)
      unregister_netdevice(ser->dev)
      debugfs_deinit(ser)
      tty_kref_put(tty)        // may drop the last ref
                     <-- race window -->
                                        tty->ops->write(tty, ...)  // UAF

Fix it by serializing accesses to ser->tty with a dedicated lock. The TX
path grabs a tty kref under the lock and drops it after the TX attempt,
while ser_release() clears ser->tty under the same lock before putting the
old tty reference. This prevents the TX path from observing a freed tty
object via ser->tty.

With this change applied, the reproducer no longer triggers the UAF in my
test.

One concern is that handle_tx() can be a hot path. This fix adds a short
lock-held section plus an extra tty kref get/put per TX run. Feedback on
the performance impact, or suggestions for a lower-overhead approach, are
welcome.

Link: https://groups.google.com/g/syzkaller/c/usNe0oKtoXw/m/x8qUc3yUAQAJ
Fixes: 308e7e4d0a84 ("serial: caif: fix use-after-free in caif_serial ldisc_close()")
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 drivers/net/caif/caif_serial.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index b90890030751..ddd90d01cd40 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -68,6 +68,7 @@ struct ser_device {
 	struct net_device *dev;
 	struct sk_buff_head head;
 	struct tty_struct *tty;
+	spinlock_t tty_lock; /* protects ser->tty */
 	bool tx_started;
 	unsigned long state;
 #ifdef CONFIG_DEBUG_FS
@@ -197,12 +198,21 @@ static int handle_tx(struct ser_device *ser)
 	struct sk_buff *skb;
 	int tty_wr, len, room;
 
+	spin_lock(&ser->tty_lock);
 	tty = ser->tty;
+	tty_kref_get(tty);
+	spin_unlock(&ser->tty_lock);
+
+	if (!tty)
+		return 0;
+
 	ser->tx_started = true;
 
 	/* Enter critical section */
-	if (test_and_set_bit(CAIF_SENDING, &ser->state))
+	if (test_and_set_bit(CAIF_SENDING, &ser->state)) {
+		tty_kref_put(tty);
 		return 0;
+	}
 
 	/* skb_peek is safe because handle_tx is called after skb_queue_tail */
 	while ((skb = skb_peek(&ser->head)) != NULL) {
@@ -245,9 +255,11 @@ static int handle_tx(struct ser_device *ser)
 		ser->common.flowctrl != NULL)
 				ser->common.flowctrl(ser->dev, ON);
 	clear_bit(CAIF_SENDING, &ser->state);
+	tty_kref_put(tty);
 	return 0;
 error:
 	clear_bit(CAIF_SENDING, &ser->state);
+	tty_kref_put(tty);
 	return tty_wr;
 }
 
@@ -293,7 +305,10 @@ static void ser_release(struct work_struct *work)
 	if (!list_empty(&list)) {
 		rtnl_lock();
 		list_for_each_entry_safe(ser, tmp, &list, node) {
+			spin_lock(&ser->tty_lock);
 			tty = ser->tty;
+			ser->tty = NULL;
+			spin_unlock(&ser->tty_lock);
 			dev_close(ser->dev);
 			unregister_netdevice(ser->dev);
 			debugfs_deinit(ser);
@@ -330,6 +345,7 @@ static int ldisc_open(struct tty_struct *tty)
 		return -ENOMEM;
 
 	ser = netdev_priv(dev);
+	spin_lock_init(&ser->tty_lock);
 	ser->tty = tty_kref_get(tty);
 	ser->dev = dev;
 	debugfs_init(ser, tty);
-- 
2.34.1

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Hillf Danton]

On Sat, 14 Feb 2026 21:51:41 -0500 Shuangpeng Bai wrote:
> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
> ldisc_close()").
> 
> That commit moved tty_kref_put() to ser_release(). There is still a race
> because the TX path may fetch ser->tty and use it while ser_release()
> drops the last tty reference:
> 
>     CPU 0 (ser_release worker)         CPU 1 (xmit)
>     -------------------------        ------------
>                                       caif_xmit()
>                                       handle_tx()
>                                         tty = ser->tty
> 
>     ser_release()
>       tty = ser->tty
>       dev_close(ser->dev)
>       unregister_netdevice(ser->dev)
>       debugfs_deinit(ser)
>       tty_kref_put(tty)        // may drop the last ref
>                      <-- race window -->
>                                         tty->ops->write(tty, ...)  // UAF
> 
What is unclear is -- why is the xmit callback still active after
unregister_netdevice().

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Shuangpeng]

> On Feb 15, 2026, at 03:55, Hillf Danton <hdanton@sina.com> wrote:
> 
> On Sat, 14 Feb 2026 21:51:41 -0500 Shuangpeng Bai wrote:
>> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
>> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
>> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
>> ldisc_close()").
>> 
>> That commit moved tty_kref_put() to ser_release(). There is still a race
>> because the TX path may fetch ser->tty and use it while ser_release()
>> drops the last tty reference:
>> 
>>    CPU 0 (ser_release worker)         CPU 1 (xmit)
>>    -------------------------        ------------
>>                                      caif_xmit()
>>                                      handle_tx()
>>                                        tty = ser->tty
>> 
>>    ser_release()
>>      tty = ser->tty
>>      dev_close(ser->dev)
>>      unregister_netdevice(ser->dev)
>>      debugfs_deinit(ser)
>>      tty_kref_put(tty)        // may drop the last ref
>>                     <-- race window -->
>>                                        tty->ops->write(tty, ...)  // UAF
>> 
> What is unclear is -- why is the xmit callback still active after
> unregister_netdevice().

In my understanding, no new ndo_start_xmit should begin after
unregister_netdevice() has completed, but an in-flight TX still needs
proper synchronization with teardown.

Concretely, CPU1 enters the TX path (ndo_start_xmit -> caif_xmit ->
handle_tx) and reads ser->tty to use it. Before CPU1 actually uses the
tty, CPU0 drops the last tty kref during teardown, so the tty may get
freed. CPU1 then continues and dereferences the stale tty in
tty_write_room() / tty->ops->write(), triggering the UAF.

The KASAN stack trace is in the report:
https://groups.google.com/g/syzkaller/c/usNe0oKtoXw/m/x8qUc3yUAQAJ

Please let me know if I’m missing anything.

Best,
Shuangpeng

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Hillf Danton]

On Sun, 15 Feb 2026 14:22:02 -0500 Shuangpeng Bai wrote:
>> On Feb 15, 2026, at 03:55, Hillf Danton <hdanton@sina.com> wrote:
>> On Sat, 14 Feb 2026 21:51:41 -0500 Shuangpeng Bai wrote:
>>> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
>>> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
>>> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
>>> ldisc_close()").
>>> 
>>> That commit moved tty_kref_put() to ser_release(). There is still a race
>>> because the TX path may fetch ser->tty and use it while ser_release()
>>> drops the last tty reference:
>>> 
>>>    CPU 0 (ser_release worker)         CPU 1 (xmit)
>>>    -------------------------        ------------
>>>                                      caif_xmit()
>>>                                      handle_tx()
>>>                                        tty = ser->tty
>>> 
>>>    ser_release()
>>>      tty = ser->tty
>>>      dev_close(ser->dev)
>>>      unregister_netdevice(ser->dev)
>>>      debugfs_deinit(ser)
>>>      tty_kref_put(tty)        // may drop the last ref
>>>                     <-- race window -->
>>>                                        tty->ops->write(tty, ...)  // UAF
>>> 
>> What is unclear is -- why is the xmit callback still active after
>> unregister_netdevice().
>
> In my understanding, no new ndo_start_xmit should begin after
> unregister_netdevice() has completed, but an in-flight TX still needs
> proper synchronization with teardown.
>
Could you shed some light on that sync Eric? Is it missed?

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Hillf Danton]

On Mon, 16 Feb 2026 08:24:23 +0800 Hillf Danton wrote:
>On Sun, 15 Feb 2026 14:22:02 -0500 Shuangpeng Bai wrote:
>>> On Feb 15, 2026, at 03:55, Hillf Danton <hdanton@sina.com> wrote:
>>> On Sat, 14 Feb 2026 21:51:41 -0500 Shuangpeng Bai wrote:
>>>> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
>>>> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
>>>> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
>>>> ldisc_close()").
>>>> 
>>>> That commit moved tty_kref_put() to ser_release(). There is still a race
>>>> because the TX path may fetch ser->tty and use it while ser_release()
>>>> drops the last tty reference:
>>>> 
>>>>    CPU 0 (ser_release worker)         CPU 1 (xmit)
>>>>    -------------------------        ------------
>>>>                                      caif_xmit()
>>>>                                      handle_tx()
>>>>                                        tty = ser->tty
>>>> 
>>>>    ser_release()
>>>>      tty = ser->tty
>>>>      dev_close(ser->dev)
>>>>      unregister_netdevice(ser->dev)
>>>>      debugfs_deinit(ser)
>>>>      tty_kref_put(tty)        // may drop the last ref
>>>>                     <-- race window -->
>>>>                                        tty->ops->write(tty, ...)  // UAF
>>>> 
>>> What is unclear is -- why is the xmit callback still active after
>>> unregister_netdevice().
>>
>> In my understanding, no new ndo_start_xmit should begin after
>> unregister_netdevice() has completed, but an in-flight TX still needs
>> proper synchronization with teardown.
>>
> Could you shed some light on that sync Eric? Is it missed?
>
So far I found a syzbot report [1] where ser->dev can not be closed with an
active xmit cb.

[1] Subject: [syzbot] [net?] BUG: soft lockup in ser_release (4)
https://lore.kernel.org/lkml/6758196a.050a0220.a30f1.01c9.GAE@google.com/

Re: [net,v2,1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
serial: caif: fix remaining ser->tty UAF in TX path

This patch addresses a KASAN use-after-free in the caif_serial TX path
by adding a spinlock to serialize access to ser->tty. The TX path now
takes a tty kref under the lock, and ser_release() clears ser->tty under
the same lock before dropping the reference.

Link: https://groups.google.com/g/syzkaller/c/usNe0oKtoXw/m/x8qUc3yUAQAJ

> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -68,6 +68,7 @@ struct ser_device {
>  	struct net_device *dev;
>  	struct sk_buff_head head;
>  	struct tty_struct *tty;
> +	spinlock_t tty_lock; /* protects ser->tty */
>  	bool tx_started;
>  	unsigned long state;
>  #ifdef CONFIG_DEBUG_FS
> @@ -197,12 +198,21 @@ static int handle_tx(struct ser_device *ser)
>  	struct sk_buff *skb;
>  	int tty_wr, len, room;
>
> +	spin_lock(&ser->tty_lock);
>  	tty = ser->tty;
> +	tty_kref_get(tty);
> +	spin_unlock(&ser->tty_lock);

handle_tx() is called from caif_xmit(), which is the ndo_start_xmit
handler. This can run in softirq context (NET_TX softirq). ser_release()
runs in process context (workqueue). If ser_release() holds tty_lock in
process context and a softirq fires on the same CPU, calling caif_xmit()
-> handle_tx() -> spin_lock(&ser->tty_lock), won't this deadlock?

Should this use spin_lock_bh()/spin_unlock_bh() instead?

> +
> +	if (!tty)
> +		return 0;
> +
>  	ser->tx_started = true;
>
>  	/* Enter critical section */
> -	if (test_and_set_bit(CAIF_SENDING, &ser->state))
> +	if (test_and_set_bit(CAIF_SENDING, &ser->state)) {
> +		tty_kref_put(tty);
>  		return 0;
> +	}
>
>  	/* skb_peek is safe because handle_tx is called after skb_queue_tail */
>  	while ((skb = skb_peek(&ser->head)) != NULL) {

[ ... ]

>  		if (!ser_loop) {
>  			tty_wr = tty->ops->write(tty, skb->data, len);
>  			update_tty_status(ser);
>  		} else {

After ser_release() sets ser->tty = NULL (the new code from this patch),
a concurrent handle_tx() that already passed the NULL check above can
reach update_tty_status(ser) here. Looking at update_tty_status(), it
dereferences ser->tty directly:

drivers/net/caif/caif_serial.c:update_tty_status() {
    inline_decode(ser->common.flowctrl,
        ser->tty->flow.stopped << 5 |
        ser->tty->flow.tco_stopped << 3 |
        ser->tty->ctrl.packet << 2);
}

Won't this cause a NULL pointer dereference? Should update_tty_status()
take the local tty pointer as a parameter instead of reading ser->tty?

> @@ -245,9 +255,11 @@
>  		ser->common.flowctrl != NULL)
>  				ser->common.flowctrl(ser->dev, ON);
>  	clear_bit(CAIF_SENDING, &ser->state);
> +	tty_kref_put(tty);
>  	return 0;
>  error:
>  	clear_bit(CAIF_SENDING, &ser->state);
> +	tty_kref_put(tty);
>  	return tty_wr;
>  }
>
> @@ -293,7 +305,10 @@ static void ser_release(struct work_struct *work)
>  	if (!list_empty(&list)) {
>  		rtnl_lock();
>  		list_for_each_entry_safe(ser, tmp, &list, node) {
> +			spin_lock(&ser->tty_lock);
>  			tty = ser->tty;
> +			ser->tty = NULL;
> +			spin_unlock(&ser->tty_lock);
>  			dev_close(ser->dev);
>  			unregister_netdevice(ser->dev);
>  			debugfs_deinit(ser);

[ ... ]

Jakub Kicinski's review of v1 (commit 308e7e4d0a84) pointed out that
moving tty_kref_put() to ser_release() doesn't fully prevent the UAF
because handle_tx() can still fetch ser->tty while ser_release() is
dropping the last tty reference. The current patch addresses this by
adding the tty_lock synchronization. While the v1/v2 review thread
didn't explicitly mark this as blocking, it represents a legitimate
race window that this patch fixes.

Reference: https://lore.kernel.org/netdev/20260206022526.3191971-1-kuba@kernel.org/
-- 
pw-bot: changes-requested

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Vadim Fedorenko]

On 15/02/2026 02:51, Shuangpeng Bai wrote:
> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
> ldisc_close()").
> 
> That commit moved tty_kref_put() to ser_release(). There is still a race
> because the TX path may fetch ser->tty and use it while ser_release()
> drops the last tty reference:
> 
>      CPU 0 (ser_release worker)         CPU 1 (xmit)
>      -------------------------        ------------
>                                        caif_xmit()
>                                        handle_tx()
>                                          tty = ser->tty
> 
>      ser_release()
>        tty = ser->tty
>        dev_close(ser->dev)
>        unregister_netdevice(ser->dev)
>        debugfs_deinit(ser)
>        tty_kref_put(tty)        // may drop the last ref
>                       <-- race window -->
>                                          tty->ops->write(tty, ...)  // UAF
> 
> Fix it by serializing accesses to ser->tty with a dedicated lock. The TX
> path grabs a tty kref under the lock and drops it after the TX attempt,
> while ser_release() clears ser->tty under the same lock before putting the
> old tty reference. This prevents the TX path from observing a freed tty
> object via ser->tty.
> 
> With this change applied, the reproducer no longer triggers the UAF in my
> test.
> 
> One concern is that handle_tx() can be a hot path. This fix adds a short
> lock-held section plus an extra tty kref get/put per TX run. Feedback on
> the performance impact, or suggestions for a lower-overhead approach, are
> welcome.

I'm not quite sure we actually need spinlock here. It looks like just 
adding simple helper tty_kref_get_unless_zero() and using it in 
handle_xt() will solve the problem?

Re: [PATCH net v2 1/1] serial: caif: fix remaining ser->tty UAF in TX path

[Shuangpeng]

Hi all,

Thanks for the previous feedback and suggestions.

We explored several mitigations to address the remaining UAF in the
caif_serial TX path, including:
1. Taking a tty reference in handle_tx() using a new
   tty_kref_get_unless_zero() helper.
2. Setting and checking ser->state to avoid internal races.

However, our reproducer still triggers a UAF.
Link: https://gist.github.com/shuangpengbai/c898debad6bdf170a84be7e6b3d8707f

After further debugging, the KASAN report consistently points to
the memory access in pty_write_room():

    tty_buffer_space_avail(tty->link->port);

Two different tty objects are dereferenced here: 'tty' and 'tty->link'.
In debugging, we found the invalid access is specifically on 'tty->link',
rather than 'tty' itself, based on comparing the runtime addresses.
Therefore, our previous analysis for the race could be inaccurate.

We will continue investigating the root cause and we are happy to 
provide any additional information for you to debug this bug.

Below is the report showing the fault location:

==================================================================
BUG: KASAN: slab-use-after-free in pty_write_room (drivers/tty/pty.c:131)
Read of size 8 at addr ffff888116df7018 by task a.out/8414

Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:122)
 print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:597)
 pty_write_room (drivers/tty/pty.c:131)
 handle_tx (drivers/net/caif/caif_serial.c:213)
 dev_hard_start_xmit (./include/linux/netdevice.h:5275 
    ./include/linux/netdevice.h:5284 net/core/dev.c:3871 net/core/dev.c:3887)
 __dev_queue_xmit (net/core/dev.c:?)
 transmit (net/caif/caif_dev.c:237)
 cfserl_transmit (net/caif/cfserl.c:185)
 cffrml_transmit (net/caif/cffrml.c:?)
 cfmuxl_transmit (net/caif/cfmuxl.c:240)
 caif_connect_client (net/caif/cfcnfg.c:355)
 caif_connect (net/caif/caif_socket.c:828)
 __sys_connect (net/socket.c:2089 net/socket.c:2108)
 __x64_sys_connect (net/socket.c:2114 net/socket.c:2111)
 do_syscall_64 (arch/x86/entry/syscall_64.c:?)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Freed by task 4526:
 kasan_save_track (mm/kasan/common.c:58 mm/kasan/common.c:78)
 kasan_save_free_info (mm/kasan/generic.c:587)
 __kasan_slab_free (mm/kasan/common.c:287)
 kfree (mm/slub.c:6082 mm/slub.c:6399)
 process_scheduled_works (kernel/workqueue.c:3280 kernel/workqueue.c:3358)
 worker_thread (./include/linux/list.h:381 kernel/workqueue.c:3440)
 kthread (kernel/kthread.c:468)
 ret_from_fork (arch/x86/kernel/process.c:164)



> On Feb 18, 2026, at 09:25, Vadim Fedorenko <vadim.fedorenko@linux.dev> wrote:
> 
> On 15/02/2026 02:51, Shuangpeng Bai wrote:
>> A reproducer exposes a KASAN use-after-free in caif_serial's TX path
>> (e.g., via tty_write_room() / tty->ops->write()) on top of commit
>> <308e7e4d0a84> ("serial: caif: fix use-after-free in caif_serial
>> ldisc_close()").
>> That commit moved tty_kref_put() to ser_release(). There is still a race
>> because the TX path may fetch ser->tty and use it while ser_release()
>> drops the last tty reference:
>>     CPU 0 (ser_release worker)         CPU 1 (xmit)
>>     -------------------------        ------------
>>                                       caif_xmit()
>>                                       handle_tx()
>>                                         tty = ser->tty
>>     ser_release()
>>       tty = ser->tty
>>       dev_close(ser->dev)
>>       unregister_netdevice(ser->dev)
>>       debugfs_deinit(ser)
>>       tty_kref_put(tty)        // may drop the last ref
>>                      <-- race window -->
>>                                         tty->ops->write(tty, ...)  // UAF
>> Fix it by serializing accesses to ser->tty with a dedicated lock. The TX
>> path grabs a tty kref under the lock and drops it after the TX attempt,
>> while ser_release() clears ser->tty under the same lock before putting the
>> old tty reference. This prevents the TX path from observing a freed tty
>> object via ser->tty.
>> With this change applied, the reproducer no longer triggers the UAF in my
>> test.
>> One concern is that handle_tx() can be a hot path. This fix adds a short
>> lock-held section plus an extra tty kref get/put per TX run. Feedback on
>> the performance impact, or suggestions for a lower-overhead approach, are
>> welcome.
> 
> I'm not quite sure we actually need spinlock here. It looks like just adding simple helper tty_kref_get_unless_zero() and using it in handle_xt() will solve the problem?
> 
>