| From: Richard Palethorpe <rpalethorpe@suse.com> |
| Date: Tue, 21 Jan 2020 14:42:58 +0100 |
| Subject: can, slip: Protect tty->disc_data in write_wakeup and close with RCU |
| |
| commit 0ace17d56824165c7f4c68785d6b58971db954dd upstream. |
| |
| write_wakeup can happen in parallel with close/hangup where tty->disc_data |
| is set to NULL and the netdevice is freed thus also freeing |
| disc_data. write_wakeup accesses disc_data so we must prevent close from |
| freeing the netdev while write_wakeup has a non-NULL view of |
| tty->disc_data. |
| |
| We also need to make sure that accesses to disc_data are atomic. Which can |
| all be done with RCU. |
| |
| This problem was found by Syzkaller on SLCAN, but the same issue is |
| reproducible with the SLIP line discipline using an LTP test based on the |
| Syzkaller reproducer. |
| |
| A fix which didn't use RCU was posted by Hillf Danton. |
| |
| Fixes: 661f7fda21b1 ("slip: Fix deadlock in write_wakeup") |
| Fixes: a8e83b17536a ("slcan: Port write_wakeup deadlock fix from slip") |
| Reported-by: syzbot+017e491ae13c0068598a@syzkaller.appspotmail.com |
| Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> |
| Cc: Wolfgang Grandegger <wg@grandegger.com> |
| Cc: Marc Kleine-Budde <mkl@pengutronix.de> |
| Cc: "David S. Miller" <davem@davemloft.net> |
| Cc: Tyler Hall <tylerwhall@gmail.com> |
| Cc: linux-can@vger.kernel.org |
| Cc: netdev@vger.kernel.org |
| Cc: linux-kernel@vger.kernel.org |
| Cc: syzkaller@googlegroups.com |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| drivers/net/can/slcan.c | 12 ++++++++++-- |
| drivers/net/slip/slip.c | 12 ++++++++++-- |
| 2 files changed, 20 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/net/can/slcan.c |
| +++ b/drivers/net/can/slcan.c |
| @@ -346,9 +346,16 @@ static void slcan_transmit(struct work_s |
| */ |
| static void slcan_write_wakeup(struct tty_struct *tty) |
| { |
| - struct slcan *sl = tty->disc_data; |
| + struct slcan *sl; |
| + |
| + rcu_read_lock(); |
| + sl = rcu_dereference(tty->disc_data); |
| + if (!sl) |
| + goto out; |
| |
| schedule_work(&sl->tx_work); |
| +out: |
| + rcu_read_unlock(); |
| } |
| |
| /* Send a can_frame to a TTY queue. */ |
| @@ -640,10 +647,11 @@ static void slcan_close(struct tty_struc |
| return; |
| |
| spin_lock_bh(&sl->lock); |
| - tty->disc_data = NULL; |
| + rcu_assign_pointer(tty->disc_data, NULL); |
| sl->tty = NULL; |
| spin_unlock_bh(&sl->lock); |
| |
| + synchronize_rcu(); |
| flush_work(&sl->tx_work); |
| |
| /* Flush network side */ |
| --- a/drivers/net/slip/slip.c |
| +++ b/drivers/net/slip/slip.c |
| @@ -452,9 +452,16 @@ static void slip_transmit(struct work_st |
| */ |
| static void slip_write_wakeup(struct tty_struct *tty) |
| { |
| - struct slip *sl = tty->disc_data; |
| + struct slip *sl; |
| + |
| + rcu_read_lock(); |
| + sl = rcu_dereference(tty->disc_data); |
| + if (!sl) |
| + goto out; |
| |
| schedule_work(&sl->tx_work); |
| +out: |
| + rcu_read_unlock(); |
| } |
| |
| static void sl_tx_timeout(struct net_device *dev) |
| @@ -885,10 +892,11 @@ static void slip_close(struct tty_struct |
| return; |
| |
| spin_lock_bh(&sl->lock); |
| - tty->disc_data = NULL; |
| + rcu_assign_pointer(tty->disc_data, NULL); |
| sl->tty = NULL; |
| spin_unlock_bh(&sl->lock); |
| |
| + synchronize_rcu(); |
| flush_work(&sl->tx_work); |
| |
| /* VSV = very important to remove timers */ |