From a6c2056232543bb56713ea06acf8d6b0ee2554de Mon Sep 17 00:00:00 2001 From: Michal Koziel Date: Wed, 15 Feb 2023 10:36:45 +0100 Subject: [PATCH] pogo: turn serdev_ready to atomic variable to prevent deadlock We are closing serdev on connect (if open), not on disconnect. There is a small possibility for MCU to send a message if it is physically disconnected and reconnected within a very short time. This is because there might just enough charge to keep the MCU running. At the same time the driver has initialized re-connection and tries to close serdev. In that situation the pogo lock is taken by fsm_entry_enumerate() while the tty lock is taken by pogo_onewire_receive_buf(). Then both functions try to take the other lock resulting in deadlock. At the same time serdev_ready should be 0. It is safe to check it without taking the lock and exit allowing fsm_entry_enumerate() to close and open serdev without deadlocking. Signed-off-by: Michal Koziel --- drivers/misc/rm-pogo/pogo.h | 2 +- drivers/misc/rm-pogo/pogo_fsm.c | 6 +++--- drivers/misc/rm-pogo/pogo_main.c | 23 ++++++++++++++++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/misc/rm-pogo/pogo.h b/drivers/misc/rm-pogo/pogo.h index 049695378eb8..d30a74a98d98 100644 --- a/drivers/misc/rm-pogo/pogo.h +++ b/drivers/misc/rm-pogo/pogo.h @@ -208,7 +208,7 @@ struct rm_pogo_data { int mode_requested; int uart_rx_mode; - bool serdev_ready; + atomic_t serdev_ready; bool serdev_open; bool tx_ack_timeout; bool tx_ack_required; diff --git a/drivers/misc/rm-pogo/pogo_fsm.c b/drivers/misc/rm-pogo/pogo_fsm.c index fa39c69898bd..6d51da331e25 100644 --- a/drivers/misc/rm-pogo/pogo_fsm.c +++ b/drivers/misc/rm-pogo/pogo_fsm.c @@ -206,7 +206,7 @@ static bool fsm_entry_idle(struct rm_pogo_data *pdata) pdata->onewire_rx_buf_len = 0; pdata->pogo_connected = false; - pdata->serdev_ready = false; + atomic_set(&pdata->serdev_ready, 0); pdata->tx_ack_timeout = false; pdata->tx_ack_required = false; @@ -257,7 +257,7 @@ static bool fsm_entry_enumerate(struct rm_pogo_data *pdata) if (!pogo_serdev_open(pdata)) { pdata->serdev_open = true; - pdata->serdev_ready = true; + atomic_set(&pdata->serdev_ready, 1); } else { return false; } @@ -711,7 +711,7 @@ static void fsm_routine_start_app(struct rm_pogo_data *pdata) static void clean_up_uart_keyboard(struct rm_pogo_data *pdata) { - pdata->serdev_ready = false; + atomic_set(&pdata->serdev_ready, 0); if (pdata->kb_dev == NULL) return; diff --git a/drivers/misc/rm-pogo/pogo_main.c b/drivers/misc/rm-pogo/pogo_main.c index 5e77530d8cfb..a36403829915 100644 --- a/drivers/misc/rm-pogo/pogo_main.c +++ b/drivers/misc/rm-pogo/pogo_main.c @@ -319,15 +319,28 @@ static int pogo_onewire_receive_buf(struct serdev_device *serdev, print_hex_dump_debug("pogo raw data:", DUMP_PREFIX_NONE, count, 1, buf, count, false); - mutex_lock(&data->lock); - if (!data->serdev_ready) { + + /* We are closing serdev on connect (if open), not on disconnect. + * There is a small possibility for MCU to send a message if it is + * physically disconnected and reconnected within a very short time. This is + * because there might just enough charge to keep the MCU running. + * At the same time the driver has initialized re-connection and tries to + * close serdev. In that situation the pogo lock is taken by fsm_entry_enumerate() + * while the tty lock is taken by pogo_onewire_receive_buf(). Then both + * functions try to take the other lock resulting in deadlock. + * At the same time serdev_ready should be 0. It is safe to check it + * without taking the lock and exit allowing fsm_entry_enumerate() to + * close and open serdev without deadlocking. + */ + + if (atomic_read(&data->serdev_ready) == 0) { dev_warn(data->dev, "%s: serdev is not ready, dropping data\n", __func__); - mutex_unlock(&data->lock); return count; } - dev_dbg(data->dev, "%s: %d bytes data received.\n", __func__, count); + mutex_lock(&data->lock); + dev_dbg(data->dev, "%s: %d bytes data received.\n", __func__, count); /* For debugging from user-space */ if (count <= ONE_WIRE_MAX_TX_MSG_SIZE) { @@ -533,7 +546,7 @@ static int rm_pogo_probe(struct serdev_device *serdev) pdata->dev = dev; pdata->serdev = serdev; - pdata->serdev_ready = false; + atomic_set(&pdata->serdev_ready, 0); pdata->serdev_open = false; mutex_init(&pdata->lock);