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 --- 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