Skip to content

Commit

Permalink
platform/chrome: cros_ec_uart: properly fix race condition
Browse files Browse the repository at this point in the history
commit 5e700b3 upstream.

The cros_ec_uart_probe() function calls devm_serdev_device_open() before
it calls serdev_device_set_client_ops(). This can trigger a NULL pointer
dereference:

    BUG: kernel NULL pointer dereference, address: 0000000000000000
    ...
    Call Trace:
     <TASK>
     ...
     ? ttyport_receive_buf

A simplified version of crashing code is as follows:

    static inline size_t serdev_controller_receive_buf(struct serdev_controller *ctrl,
                                                      const u8 *data,
                                                      size_t count)
    {
            struct serdev_device *serdev = ctrl->serdev;

            if (!serdev || !serdev->ops->receive_buf) // CRASH!
                return 0;

            return serdev->ops->receive_buf(serdev, data, count);
    }

It assumes that if SERPORT_ACTIVE is set and serdev exists, serdev->ops
will also exist. This conflicts with the existing cros_ec_uart_probe()
logic, as it first calls devm_serdev_device_open() (which sets
SERPORT_ACTIVE), and only later sets serdev->ops via
serdev_device_set_client_ops().

Commit 01f95d4 ("platform/chrome: cros_ec_uart: fix race
condition") attempted to fix a similar race condition, but while doing
so, made the window of error for this race condition to happen much
wider.

Attempt to fix the race condition again, making sure we fully setup
before calling devm_serdev_device_open().

Fixes: 01f95d4 ("platform/chrome: cros_ec_uart: fix race condition")
Cc: stable@vger.kernel.org
Signed-off-by: Noah Loomans <noah@noahloomans.com>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Link: https://lore.kernel.org/r/20240410182618.169042-2-noah@noahloomans.com
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
nloomans authored and gregkh committed Apr 17, 2024
1 parent 7521329 commit 9e9bb74
Showing 1 changed file with 14 additions and 14 deletions.
28 changes: 14 additions & 14 deletions drivers/platform/chrome/cros_ec_uart.c
Expand Up @@ -263,12 +263,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
if (!ec_dev)
return -ENOMEM;

ret = devm_serdev_device_open(dev, serdev);
if (ret) {
dev_err(dev, "Unable to open UART device");
return ret;
}

serdev_device_set_drvdata(serdev, ec_dev);
init_waitqueue_head(&ec_uart->response.wait_queue);

Expand All @@ -280,14 +274,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
return ret;
}

ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
if (ret < 0) {
dev_err(dev, "Failed to set up host baud rate (%d)", ret);
return ret;
}

serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);

/* Initialize ec_dev for cros_ec */
ec_dev->phys_name = dev_name(dev);
ec_dev->dev = dev;
Expand All @@ -301,6 +287,20 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)

serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);

ret = devm_serdev_device_open(dev, serdev);
if (ret) {
dev_err(dev, "Unable to open UART device");
return ret;
}

ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
if (ret < 0) {
dev_err(dev, "Failed to set up host baud rate (%d)", ret);
return ret;
}

serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);

return cros_ec_register(ec_dev);
}

Expand Down

0 comments on commit 9e9bb74

Please sign in to comment.