Skip to content

Commit

Permalink
can: can327: flush TX_work on ldisc .close()
Browse files Browse the repository at this point in the history
Additionally, remove it from .ndo_stop().

This ensures that the worker is not called after being freed, and that
the UART TX queue remains active to send final commands when the
netdev is stopped.

Thanks to Jiri Slaby for finding this in slcan:

  https://lore.kernel.org/linux-can/20221201073426.17328-1-jirislaby@kernel.org/

A variant of this patch for slcan, with the flush in .ndo_stop() still
present, has been tested successfully on physical hardware:

  https://bugzilla.suse.com/show_bug.cgi?id=1205597

Fixes: 43da2f0 ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters")
Cc: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Cc: Max Staudt <max@enpas.org>
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
Signed-off-by: Max Staudt <max@enpas.org>
Link: https://lore.kernel.org/all/20221202160148.282564-1-max@enpas.org
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
  • Loading branch information
Max Staudt authored and Marc Kleine-Budde committed Dec 7, 2022
1 parent fb855e9 commit f4a4d12
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions drivers/net/can/can327.c
Original file line number Diff line number Diff line change
Expand Up @@ -796,9 +796,9 @@ static int can327_netdev_close(struct net_device *dev)

netif_stop_queue(dev);

/* Give UART one final chance to flush. */
clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
flush_work(&elm->tx_work);
/* We don't flush the UART TX queue here, as we want final stop
* commands (like the above dummy char) to be flushed out.
*/

can_rx_offload_disable(&elm->offload);
elm->can.state = CAN_STATE_STOPPED;
Expand Down Expand Up @@ -1069,12 +1069,15 @@ static void can327_ldisc_close(struct tty_struct *tty)
{
struct can327 *elm = (struct can327 *)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 elm->tty = NULL after this.
*/
/* unregister_netdev() calls .ndo_stop() so we don't have to. */
unregister_candev(elm->dev);

/* Give UART one final chance to flush.
* No need to clear TTY_DO_WRITE_WAKEUP since .write_wakeup() is
* serialised against .close() and will not be called once we return.
*/
flush_work(&elm->tx_work);

/* Mark channel as dead */
spin_lock_bh(&elm->lock);
tty->disc_data = NULL;
Expand Down

0 comments on commit f4a4d12

Please sign in to comment.