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>

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index c5e5423e18633a25e3c5d9925ba6a7c10cf531bf..885992951e8a65ae8420682a50037d63404447f6 100644 (file)
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -115,8 +115,6 @@ struct sixpack {
 
        struct timer_list       tx_t;
        struct timer_list       resync_t;
-       refcount_t              refcnt;
-       struct completion       dead;
        spinlock_t              lock;
 };
 
@@ -353,42 +351,13 @@ out_mem:
 
 /* ----------------------------------------------------------------------- */
 
-/*
- * 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
- * sixpack_receive_buf while another calls sixpack_close, which zeroes
- * tty->disc_data and frees the memory that sixpack_receive_buf 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.
- */
-static DEFINE_RWLOCK(disc_data_lock);
-                                                                                
-static struct sixpack *sp_get(struct tty_struct *tty)
-{
-       struct sixpack *sp;
-
-       read_lock(&disc_data_lock);
-       sp = tty->disc_data;
-       if (sp)
-               refcount_inc(&sp->refcnt);
-       read_unlock(&disc_data_lock);
-
-       return sp;
-}
-
-static void sp_put(struct sixpack *sp)
-{
-       if (refcount_dec_and_test(&sp->refcnt))
-               complete(&sp->dead);
-}
-
 /*
  * Called by the TTY driver when there's room for more data.  If we have
  * more packets to send, we send them here.
  */
 static void sixpack_write_wakeup(struct tty_struct *tty)
 {
-       struct sixpack *sp = sp_get(tty);
+       struct sixpack *sp = tty->disc_data;
        int actual;
 
        if (!sp)
@@ -400,7 +369,7 @@ static void sixpack_write_wakeup(struct tty_struct *tty)
                clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
                sp->tx_enable = 0;
                netif_wake_queue(sp->dev);
-               goto out;
+               return;
        }
 
        if (sp->tx_enable) {
@@ -408,9 +377,6 @@ static void sixpack_write_wakeup(struct tty_struct *tty)
                sp->xleft -= actual;
                sp->xhead += actual;
        }
-
-out:
-       sp_put(sp);
 }
 
 /* ----------------------------------------------------------------------- */
@@ -430,7 +396,7 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
        if (!count)
                return;
 
-       sp = sp_get(tty);
+       sp = tty->disc_data;
        if (!sp)
                return;
 
@@ -446,7 +412,6 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
        }
        sixpack_decode(sp, cp, count1);
 
-       sp_put(sp);
        tty_unthrottle(tty);
 }
 
@@ -561,8 +526,6 @@ static int sixpack_open(struct tty_struct *tty)
 
        spin_lock_init(&sp->lock);
        spin_lock_init(&sp->rxlock);
-       refcount_set(&sp->refcnt, 1);
-       init_completion(&sp->dead);
 
        /* !!! length of the buffers. MTU is IP MTU, not PACLEN!  */
 
@@ -638,19 +601,11 @@ static void sixpack_close(struct tty_struct *tty)
 {
        struct sixpack *sp;
 
-       write_lock_irq(&disc_data_lock);
        sp = tty->disc_data;
-       tty->disc_data = NULL;
-       write_unlock_irq(&disc_data_lock);
        if (!sp)
                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.
-        */
-       if (!refcount_dec_and_test(&sp->refcnt))
-               wait_for_completion(&sp->dead);
+       tty->disc_data = NULL;
 
        /* We must stop the queue to avoid potentially scribbling
         * on the free buffers. The sp->dead completion is not sufficient
@@ -673,7 +628,7 @@ static void sixpack_close(struct tty_struct *tty)
 static int sixpack_ioctl(struct tty_struct *tty, unsigned int cmd,
                unsigned long arg)
 {
-       struct sixpack *sp = sp_get(tty);
+       struct sixpack *sp = tty->disc_data;
        struct net_device *dev;
        unsigned int tmp, err;
 
@@ -725,8 +680,6 @@ static int sixpack_ioctl(struct tty_struct *tty, unsigned int cmd,
                err = tty_mode_ioctl(tty, cmd, arg);
        }
 
-       sp_put(sp);
-
        return err;
 }