Skip to content

Commit

Permalink
pogo: turn serdev_ready to atomic variable to prevent deadlock
Browse files Browse the repository at this point in the history
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 <michal.koziel@emlogic.no>
  • Loading branch information
mkemlogic committed Feb 15, 2023
1 parent ed6cf38 commit a6c2056
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
2 changes: 1 addition & 1 deletion drivers/misc/rm-pogo/pogo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions drivers/misc/rm-pogo/pogo_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 18 additions & 5 deletions drivers/misc/rm-pogo/pogo_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit a6c2056

Please sign in to comment.