Skip to content

Commit 38b04ed

Browse files
LGA1150Paolo Abeni
authored andcommitted
6pack: drop redundant locking and refcounting
The TTY layer already serializes line discipline operations with tty->ldisc_sem, so the extra disc_data_lock and refcnt in 6pack are unnecessary. Removing them simplifies the code and also resolves a lockdep warning reported by syzbot. The warning did not indicate a real deadlock, since the write-side lock was only taken in process context with hardirqs disabled. Reported-by: syzbot+5fd749c74105b0e1b302@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/68c858b0.050a0220.3c6139.0d1c.GAE@google.com/ Signed-off-by: Qingfang Deng <dqfext@gmail.com> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> Link: https://patch.msgid.link/20250925051059.26876-1-dqfext@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
1 parent 7bd80ed commit 38b04ed

1 file changed

Lines changed: 5 additions & 52 deletions

File tree

drivers/net/hamradio/6pack.c

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ struct sixpack {
115115

116116
struct timer_list tx_t;
117117
struct timer_list resync_t;
118-
refcount_t refcnt;
119-
struct completion dead;
120118
spinlock_t lock;
121119
};
122120

@@ -353,42 +351,13 @@ static void sp_bump(struct sixpack *sp, char cmd)
353351

354352
/* ----------------------------------------------------------------------- */
355353

356-
/*
357-
* We have a potential race on dereferencing tty->disc_data, because the tty
358-
* layer provides no locking at all - thus one cpu could be running
359-
* sixpack_receive_buf while another calls sixpack_close, which zeroes
360-
* tty->disc_data and frees the memory that sixpack_receive_buf is using. The
361-
* best way to fix this is to use a rwlock in the tty struct, but for now we
362-
* use a single global rwlock for all ttys in ppp line discipline.
363-
*/
364-
static DEFINE_RWLOCK(disc_data_lock);
365-
366-
static struct sixpack *sp_get(struct tty_struct *tty)
367-
{
368-
struct sixpack *sp;
369-
370-
read_lock(&disc_data_lock);
371-
sp = tty->disc_data;
372-
if (sp)
373-
refcount_inc(&sp->refcnt);
374-
read_unlock(&disc_data_lock);
375-
376-
return sp;
377-
}
378-
379-
static void sp_put(struct sixpack *sp)
380-
{
381-
if (refcount_dec_and_test(&sp->refcnt))
382-
complete(&sp->dead);
383-
}
384-
385354
/*
386355
* Called by the TTY driver when there's room for more data. If we have
387356
* more packets to send, we send them here.
388357
*/
389358
static void sixpack_write_wakeup(struct tty_struct *tty)
390359
{
391-
struct sixpack *sp = sp_get(tty);
360+
struct sixpack *sp = tty->disc_data;
392361
int actual;
393362

394363
if (!sp)
@@ -400,17 +369,14 @@ static void sixpack_write_wakeup(struct tty_struct *tty)
400369
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
401370
sp->tx_enable = 0;
402371
netif_wake_queue(sp->dev);
403-
goto out;
372+
return;
404373
}
405374

406375
if (sp->tx_enable) {
407376
actual = tty->ops->write(tty, sp->xhead, sp->xleft);
408377
sp->xleft -= actual;
409378
sp->xhead += actual;
410379
}
411-
412-
out:
413-
sp_put(sp);
414380
}
415381

416382
/* ----------------------------------------------------------------------- */
@@ -430,7 +396,7 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
430396
if (!count)
431397
return;
432398

433-
sp = sp_get(tty);
399+
sp = tty->disc_data;
434400
if (!sp)
435401
return;
436402

@@ -446,7 +412,6 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
446412
}
447413
sixpack_decode(sp, cp, count1);
448414

449-
sp_put(sp);
450415
tty_unthrottle(tty);
451416
}
452417

@@ -561,8 +526,6 @@ static int sixpack_open(struct tty_struct *tty)
561526

562527
spin_lock_init(&sp->lock);
563528
spin_lock_init(&sp->rxlock);
564-
refcount_set(&sp->refcnt, 1);
565-
init_completion(&sp->dead);
566529

567530
/* !!! length of the buffers. MTU is IP MTU, not PACLEN! */
568531

@@ -638,19 +601,11 @@ static void sixpack_close(struct tty_struct *tty)
638601
{
639602
struct sixpack *sp;
640603

641-
write_lock_irq(&disc_data_lock);
642604
sp = tty->disc_data;
643-
tty->disc_data = NULL;
644-
write_unlock_irq(&disc_data_lock);
645605
if (!sp)
646606
return;
647607

648-
/*
649-
* We have now ensured that nobody can start using ap from now on, but
650-
* we have to wait for all existing users to finish.
651-
*/
652-
if (!refcount_dec_and_test(&sp->refcnt))
653-
wait_for_completion(&sp->dead);
608+
tty->disc_data = NULL;
654609

655610
/* We must stop the queue to avoid potentially scribbling
656611
* on the free buffers. The sp->dead completion is not sufficient
@@ -673,7 +628,7 @@ static void sixpack_close(struct tty_struct *tty)
673628
static int sixpack_ioctl(struct tty_struct *tty, unsigned int cmd,
674629
unsigned long arg)
675630
{
676-
struct sixpack *sp = sp_get(tty);
631+
struct sixpack *sp = tty->disc_data;
677632
struct net_device *dev;
678633
unsigned int tmp, err;
679634

@@ -725,8 +680,6 @@ static int sixpack_ioctl(struct tty_struct *tty, unsigned int cmd,
725680
err = tty_mode_ioctl(tty, cmd, arg);
726681
}
727682

728-
sp_put(sp);
729-
730683
return err;
731684
}
732685

0 commit comments

Comments
 (0)