github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-09-13 06:17:27 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

ppp: ppp_async: simplify tty disc_data access

Browse Source 
tty_ldisc_hangup() invokes the hangup callback while holding only a read
lock on tty->ldisc_sem, so it can run concurrently with other line
discipline callbacks. This currently forces async PPP to maintain
separate lifetime protection around tty->disc_data.

Line discipline close is called under the write lock during hangup
processing. Remove the hangup callback and rely on close for teardown,
as done for SLIP by commit 23c53269f2 ("slip: remove slip_hangup() to
fix use-after-free in slip_receive_buf()"). This serializes teardown
with all other line discipline operations.

disc_data_lock, refcount and completion are redundant with that
serialization. Remove them and access tty->disc_data directly.

This also eliminates a lockdep warning reported by syzbot. The warning
does not indicate a real deadlock because the write side runs only in
process context with hardirqs disabled.

Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Reported-by: syzbot+8e808eb853386f575d86@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/0000000000002fbad30611e25849@google.com/
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260828073245.126804-1-qingfang.deng@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Qingfang Deng 2026-08-28 15:32:36 +08:00 committed by Jakub Kicinski
parent 2987ee196c
commit 9feb069e5e
1 changed files with 7 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
drivers/net/ppp/ppp_async.c
View File

@ -63,8 +63,6 @@ struct asyncppp {
struct tasklet_struct tsk;
refcount_t refcnt;
struct completion dead;
struct ppp_channel chan; /* interface to generic ppp layer */
unsigned char obuf[OBUFSIZE];
};
@ -114,38 +112,6 @@ static const struct ppp_channel_ops async_ops = {
* Routines implementing the PPP line discipline.
*/
/*
* We have a potential race on dereferencing tty->disc_data,
* because the tty layer provides no locking at all - thus one
* cpu could be running ppp_asynctty_receive while another
* calls ppp_asynctty_close, which zeroes tty->disc_data and
* frees the memory that ppp_asynctty_receive is using. The best
* way to fix this is to use a rwlock in the tty struct, but for now
* we use a single global rwlock for all ttys in ppp line discipline.
*
* FIXME: this is no longer true. The _close path for the ldisc is
* now guaranteed to be sane.
*/
static DEFINE_RWLOCK(disc_data_lock);
static struct asyncppp *ap_get(struct tty_struct *tty)
{
struct asyncppp *ap;
read_lock(&disc_data_lock);
ap = tty->disc_data;
if (ap != NULL)
refcount_inc(&ap->refcnt);
read_unlock(&disc_data_lock);
return ap;
}
static void ap_put(struct asyncppp *ap)
{
if (refcount_dec_and_test(&ap->refcnt))
complete(&ap->dead);
}
/*
* Called when a tty is put into PPP line discipline. Called in process
* context.
@ -180,9 +146,6 @@ ppp_asynctty_open(struct tty_struct *tty)
skb_queue_head_init(&ap->rqueue);
tasklet_setup(&ap->tsk, ppp_async_process);
refcount_set(&ap->refcnt, 1);
init_completion(&ap->dead);
ap->chan.private = ap;
ap->chan.ops = &async_ops;
ap->chan.mtu = PPP_MRU;
@ -203,34 +166,18 @@ ppp_asynctty_open(struct tty_struct *tty)
}
/*
* Called when the tty is put into another line discipline
* or it hangs up. We have to wait for any cpu currently
* executing in any of the other ppp_asynctty_* routines to
* finish before we can call ppp_unregister_channel and free
* the asyncppp struct. This routine must be called from
* process context, not interrupt or softirq context.
* Called when the tty is put into another line discipline or it hangs up.
* This call is serialized against other ldisc functions.
*/
static void
ppp_asynctty_close(struct tty_struct *tty)
{
struct asyncppp *ap;
struct asyncppp *ap = tty->disc_data;
write_lock_irq(&disc_data_lock);
ap = tty->disc_data;
tty->disc_data = NULL;
write_unlock_irq(&disc_data_lock);
if (!ap)
return;
/*
* We have now ensured that nobody can start using ap from now
* on, but we have to wait for all existing users to finish.
* Note that ppp_unregister_channel ensures that no calls to
* our channel ops (i.e. ppp_async_send/ioctl) are in progress
* by the time it returns.
*/
if (!refcount_dec_and_test(&ap->refcnt))
wait_for_completion(&ap->dead);
tty->disc_data = NULL;
tasklet_kill(&ap->tsk);
ppp_unregister_channel(&ap->chan);
@ -240,17 +187,6 @@ ppp_asynctty_close(struct tty_struct *tty)
kfree(ap);
}
/*
* Called on tty hangup in process context.
*
* Wait for I/O to driver to complete and unregister PPP channel.
* This is already done by the close routine, so just call that.
*/
static void ppp_asynctty_hangup(struct tty_struct *tty)
{
ppp_asynctty_close(tty);
}
/*
* Read does nothing - no data is ever available this way.
* Pppd reads and writes packets via /dev/ppp instead.
@ -281,7 +217,7 @@ ppp_asynctty_write(struct tty_struct *tty, struct file *file, const u8 *buf,
static int
ppp_asynctty_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
{
struct asyncppp *ap = ap_get(tty);
struct asyncppp *ap = tty->disc_data;
int err, val;
int __user *p = (int __user *)arg;
@ -322,7 +258,6 @@ ppp_asynctty_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
err = tty_mode_ioctl(tty, cmd, arg);
}
ap_put(ap);
return err;
}
@ -331,7 +266,7 @@ static void
ppp_asynctty_receive(struct tty_struct *tty, const u8 *buf, const u8 *cflags,
size_t count)
{
struct asyncppp *ap = ap_get(tty);
struct asyncppp *ap = tty->disc_data;
unsigned long flags;
if (!ap)
@ -341,21 +276,19 @@ ppp_asynctty_receive(struct tty_struct *tty, const u8 *buf, const u8 *cflags,
spin_unlock_irqrestore(&ap->recv_lock, flags);
if (!skb_queue_empty(&ap->rqueue))
tasklet_schedule(&ap->tsk);
ap_put(ap);
tty_unthrottle(tty);
}
static void
ppp_asynctty_wakeup(struct tty_struct *tty)
{
struct asyncppp *ap = ap_get(tty);
struct asyncppp *ap = tty->disc_data;
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
if (!ap)
return;
set_bit(XMIT_WAKEUP, &ap->xmit_flags);
tasklet_schedule(&ap->tsk);
ap_put(ap);
}
@ -365,7 +298,6 @@ static struct tty_ldisc_ops ppp_ldisc = {
.name = "ppp",
.open = ppp_asynctty_open,
.close = ppp_asynctty_close,
.hangup = ppp_asynctty_hangup,
.read = ppp_asynctty_read,
.write = ppp_asynctty_write,
.ioctl = ppp_asynctty_ioctl,