Besides more predictable behaviour, this allows for several hardened behaviour changes:
Return -EALREADY in `serdev_device_open` if the device is already open instead of causing undefined behaviour.
Allow calling `serdev_device_close`, even if the device is already closed instead of causing a null pointer dereference.
If the device is left open by the driver after remove, close it and warn instead of leaving it in a invalid state.
Signed-off-by: Markus Probst markus.probst@posteo.de --- drivers/tty/serdev/core.c | 10 +++++++--- drivers/tty/serdev/serdev-ttyport.c | 35 +++++++++++++++++++++++++++++------ include/linux/serdev.h | 2 +- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index 7500efcdfc21..77e8e1d4d2a6 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -142,6 +142,11 @@ void serdev_device_remove(struct serdev_device *serdev) struct serdev_controller *ctrl = serdev->ctrl;
device_unregister(&serdev->dev); + + /* Warn if driver did not close the serial device. */ + if (ctrl->ops->close && WARN_ON(ctrl->ops->close(ctrl))) + pm_runtime_put(&ctrl->dev); + ctrl->serdev = NULL; } EXPORT_SYMBOL_GPL(serdev_device_remove); @@ -181,9 +186,8 @@ void serdev_device_close(struct serdev_device *serdev) if (!ctrl || !ctrl->ops->close) return;
- pm_runtime_put(&ctrl->dev); - - ctrl->ops->close(ctrl); + if (ctrl->ops->close(ctrl)) + pm_runtime_put(&ctrl->dev); } EXPORT_SYMBOL_GPL(serdev_device_close);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c index bab1b143b8a6..c11908f5e1ce 100644 --- a/drivers/tty/serdev/serdev-ttyport.c +++ b/drivers/tty/serdev/serdev-ttyport.c @@ -16,6 +16,7 @@ struct serport { struct tty_driver *tty_drv; int tty_idx; unsigned long flags; + struct mutex lock; /* lock preventing modification of flags */ };
/* @@ -29,6 +30,8 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp, struct serport *serport = serdev_controller_get_drvdata(ctrl); size_t ret;
+ guard(mutex)(&serport->lock); + if (!test_bit(SERPORT_ACTIVE, &serport->flags)) return 0;
@@ -99,14 +102,23 @@ static int ttyport_open(struct serdev_controller *ctrl) struct ktermios ktermios; int ret;
+ mutex_lock(&serport->lock); + + if (test_bit(SERPORT_ACTIVE, &serport->flags)) { + ret = -EALREADY; + goto err_flags_unlock; + } + tty = tty_init_dev(serport->tty_drv, serport->tty_idx); - if (IS_ERR(tty)) - return PTR_ERR(tty); + if (IS_ERR(tty)) { + ret = PTR_ERR(tty); + goto err_flags_unlock; + } serport->tty = tty;
if (!tty->ops->open || !tty->ops->close) { ret = -ENODEV; - goto err_unlock; + goto err_tty_unlock; }
ret = tty->ops->open(serport->tty, NULL); @@ -130,23 +142,30 @@ static int ttyport_open(struct serdev_controller *ctrl)
set_bit(SERPORT_ACTIVE, &serport->flags);
+ mutex_unlock(&serport->lock); + return 0;
err_close: tty->ops->close(tty, NULL); -err_unlock: +err_tty_unlock: tty_unlock(tty); tty_release_struct(tty, serport->tty_idx); +err_flags_unlock: + mutex_unlock(&serport->lock);
return ret; }
-static void ttyport_close(struct serdev_controller *ctrl) +static bool ttyport_close(struct serdev_controller *ctrl) { struct serport *serport = serdev_controller_get_drvdata(ctrl); struct tty_struct *tty = serport->tty;
- clear_bit(SERPORT_ACTIVE, &serport->flags); + guard(mutex)(&serport->lock); + + if (!__test_and_clear_bit(SERPORT_ACTIVE, &serport->flags)) + return false;
tty_lock(tty); if (tty->ops->close) @@ -154,6 +173,8 @@ static void ttyport_close(struct serdev_controller *ctrl) tty_unlock(tty);
tty_release_struct(tty, serport->tty_idx); + + return true; }
static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigned int speed) @@ -288,6 +309,8 @@ struct device *serdev_tty_port_register(struct tty_port *port, port->client_ops = &client_ops; port->client_data = ctrl;
+ mutex_init(&serport->lock); + ret = serdev_controller_add(ctrl); if (ret) goto err_reset_data; diff --git a/include/linux/serdev.h b/include/linux/serdev.h index b6c3d957ec15..0f4e81c0950d 100644 --- a/include/linux/serdev.h +++ b/include/linux/serdev.h @@ -81,7 +81,7 @@ struct serdev_controller_ops { ssize_t (*write_buf)(struct serdev_controller *, const u8 *, size_t); void (*write_flush)(struct serdev_controller *); int (*open)(struct serdev_controller *); - void (*close)(struct serdev_controller *); + bool (*close)(struct serdev_controller *); void (*set_flow_control)(struct serdev_controller *, bool); int (*set_parity)(struct serdev_controller *, enum serdev_parity); unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);