serial: caif: hold tty->link reference to avoid UAF in pty_write_room

Follow-up to the v2 discussion: further debugging shows the KASAN slab-use-after-free is triggered in pty_write_room(), and the faulting access is on tty->link->port.

The report points to the peer access in pty_write_room():

    tty_buffer_space_avail(tty->link->port);

This patch holds an extra kref on tty->link for the lifetime of the caif_serial line discipline: get the reference in ldisc_open() and drop it in ser_release(), and also drop it on the ldisc_open() error path.

Changes since v2:
- Update the analysis: the observed UAF is on tty->link, not ser->tty.
- Keep tty->link alive while the line discipline is active.

Shuangpeng Bai (1):
serial: caif: hold tty->link reference in ldisc_open and ser_release

 drivers/net/caif/caif_serial.c | 3 +++
 1 file changed, 3 insertions(+)

2.34.1

A reproducer triggers a KASAN slab-use-after-free in pty_write_room() when caif_serial's TX path calls tty_write_room(). The faulting access is on tty->link->port.

Hold an extra kref on tty->link for the lifetime of the caif_serial line
discipline: get it in ldisc_open() and drop it in ser_release(), and also drop it on the ldisc_open() error path.

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

This issue becomes reproducible on top of 308e7e4d0a84. Before that, the reproducer typically hits another bug first, so this UAF is not observable there.

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

index b90890030751..1873d8287bb9 100644
@@ -297,6 +297,7 @@ static void ser_release(struct work_struct *work)
 			dev_close(ser->dev);
 			unregister_netdevice(ser->dev);
 			debugfs_deinit(ser);
+			tty_kref_put(tty->link);
 			tty_kref_put(tty);
 		}
 		rtnl_unlock();

@@ -331,6 +332,7 @@ static int ldisc_open(struct tty_struct *tty)
 
 	ser = netdev_priv(dev);
 	ser->tty = tty_kref_get(tty);
+	tty_kref_get(tty->link);
 	ser->dev = dev;
 	debugfs_init(ser, tty);
 	tty->receive_room = 4096;

@@ -339,6 +341,7 @@ static int ldisc_open(struct tty_struct *tty)
 	rtnl_lock();
 	result = register_netdevice(dev);
 	if (result) {
+		tty_kref_put(tty->link);
 		tty_kref_put(tty);
 		rtnl_unlock();
 		free_netdev(dev);

2.34.1

Please repost and make sure you CC everyone who has given tags to this commit. The get_maintainers script will point them out to you.

And there's no need for a cover letter on a single patch submission, just add the extra info under the --- marker in the patch

Also [email protected] is not a real address, horms@

pw-bot: cr

Hi Jakub,

Thanks for the guidance. I've reposted as v4 without a cover letter and CC'd everyone pointed out by get_maintainer.pl, including the taggers of the Fixes commit. I also fixed the [email protected] address.

Thanks,
Shuangpeng