can: slcan: fix freed work crash

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2071839
Tested: This is one of a series of patch sets to enable Arm SystemReady IR
 support.  This set updates the CAN driver subsystem.  This set has
 been tested on NXP imx8mm and NV Jetson Xavier NX via simple boot
 tests, using cansend/candump from can-utils to pass messages through
 a virtual CAN interface, and of course the CI loop.

commit fb855e9f3b6b42c72af3f1eb0b288998fe0d5ebb
Author: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Date:   Thu Dec 1 08:34:26 2022 +0100

    can: slcan: fix freed work crash

    The LTP test pty03 is causing a crash in slcan:
      BUG: kernel NULL pointer dereference, address: 0000000000000008
      #PF: supervisor read access in kernel mode
      #PF: error_code(0x0000) - not-present page
      PGD 0 P4D 0
      Oops: 0000 [#1] PREEMPT SMP NOPTI
      CPU: 0 PID: 348 Comm: kworker/0:3 Not tainted 6.0.8-1-default #1 openSUSE Tumbleweed 9d20364b934f5aab0a9bdf84e8f45cfdfae39dab
      Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
      Workqueue:  0x0 (events)
      RIP: 0010:process_one_work (/home/rich/kernel/linux/kernel/workqueue.c:706 /home/rich/kernel/linux/kernel/workqueue.c:2185)
      Code: 49 89 ff 41 56 41 55 41 54 55 53 48 89 f3 48 83 ec 10 48 8b 06 48 8b 6f 48 49 89 c4 45 30 e4 a8 04 b8 00 00 00 00 4c 0f 44 e0 <49> 8b 44 24 08 44 8b a8 00 01 00 00 41 83 e5 20 f6 45 10 04 75 0e
      RSP: 0018:ffffaf7b40f47e98 EFLAGS: 00010046
      RAX: 0000000000000000 RBX: ffff9d644e1b8b48 RCX: ffff9d649e439968
      RDX: 00000000ffff8455 RSI: ffff9d644e1b8b48 RDI: ffff9d64764aa6c0
      RBP: ffff9d649e4335c0 R08: 0000000000000c00 R09: ffff9d64764aa734
      R10: 0000000000000007 R11: 0000000000000001 R12: 0000000000000000
      R13: ffff9d649e4335e8 R14: ffff9d64490da780 R15: ffff9d64764aa6c0
      FS:  0000000000000000(0000) GS:ffff9d649e400000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 0000000000000008 CR3: 0000000036424000 CR4: 00000000000006f0
      Call Trace:
       <TASK>
      worker_thread (/home/rich/kernel/linux/kernel/workqueue.c:2436)
      kthread (/home/rich/kernel/linux/kernel/kthread.c:376)
      ret_from_fork (/home/rich/kernel/linux/arch/x86/entry/entry_64.S:312)

    Apparently, the slcan's tx_work is freed while being scheduled. While
    slcan_netdev_close() (netdev side) calls flush_work(&sl->tx_work),
    slcan_close() (tty side) does not. So when the netdev is never set UP,
    but the tty is stuffed with bytes and forced to wakeup write, the work
    is scheduled, but never flushed.

    So add an additional flush_work() to slcan_close() to be sure the work
    is flushed under all circumstances.

    The Fixes commit below moved flush_work() from slcan_close() to
    slcan_netdev_close(). What was the rationale behind it? Maybe we can
    drop the one in slcan_netdev_close()?

    I see the same pattern in can327. So it perhaps needs the very same fix.

    Fixes: cfcb4465e992 ("can: slcan: remove legacy infrastructure")
    Link: https://bugzilla.suse.com/show_bug.cgi?id=1205597
    Reported-by: Richard Palethorpe <richard.palethorpe@suse.com>
    Tested-by: Petr Vorel <petr.vorel@suse.com>
    Cc: Dario Binacchi <dario.binacchi@amarulasolutions.com>
    Cc: Wolfgang Grandegger <wg@grandegger.com>
    Cc: Marc Kleine-Budde <mkl@pengutronix.de>
    Cc: "David S. Miller" <davem@davemloft.net>
    Cc: Eric Dumazet <edumazet@google.com>
    Cc: Jakub Kicinski <kuba@kernel.org>
    Cc: Paolo Abeni <pabeni@redhat.com>
    Cc: linux-can@vger.kernel.org
    Cc: netdev@vger.kernel.org
    Cc: stable@vger.kernel.org
    Cc: Max Staudt <max@enpas.org>
    Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
    Reviewed-by: Max Staudt <max@enpas.org>
    Link: https://lore.kernel.org/all/20221201073426.17328-1-jirislaby@kernel.org
    Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
    (cherry picked from commit fb855e9f3b6b42c72af3f1eb0b288998fe0d5ebb)

Signed-off-by: Al Stone <ahs3@redhat.com>
This commit is contained in:
Al Stone 2023-01-10 17:01:17 -07:00
parent 8ade3ef8b7
commit e07b949a98
1 changed files with 6 additions and 4 deletions

View File

@ -864,12 +864,14 @@ static void slcan_close(struct tty_struct *tty)
{
struct slcan *sl = (struct slcan *)tty->disc_data;
/* unregister_netdev() calls .ndo_stop() so we don't have to.
* Our .ndo_stop() also flushes the TTY write wakeup handler,
* so we can safely set sl->tty = NULL after this.
*/
unregister_candev(sl->dev);
/*
* The netdev needn't be UP (so .ndo_stop() is not called). Hence make
* sure this is not running before freeing it up.
*/
flush_work(&sl->tx_work);
/* Mark channel as dead */
spin_lock_bh(&sl->lock);
tty->disc_data = NULL;