Skip to content

Commit 5e700b3

Browse files
nloomansTzung-Bi Shih
authored andcommitted
platform/chrome: cros_ec_uart: properly fix race condition
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: [email protected] Signed-off-by: Noah Loomans <[email protected]> Reviewed-by: Guenter Roeck <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Tzung-Bi Shih <[email protected]>
1 parent 6613476 commit 5e700b3

File tree

1 file changed

+14
-14
lines changed

1 file changed

+14
-14
lines changed

drivers/platform/chrome/cros_ec_uart.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
263263
if (!ec_dev)
264264
return -ENOMEM;
265265

266-
ret = devm_serdev_device_open(dev, serdev);
267-
if (ret) {
268-
dev_err(dev, "Unable to open UART device");
269-
return ret;
270-
}
271-
272266
serdev_device_set_drvdata(serdev, ec_dev);
273267
init_waitqueue_head(&ec_uart->response.wait_queue);
274268

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

283-
ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
284-
if (ret < 0) {
285-
dev_err(dev, "Failed to set up host baud rate (%d)", ret);
286-
return ret;
287-
}
288-
289-
serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
290-
291277
/* Initialize ec_dev for cros_ec */
292278
ec_dev->phys_name = dev_name(dev);
293279
ec_dev->dev = dev;
@@ -301,6 +287,20 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
301287

302288
serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
303289

290+
ret = devm_serdev_device_open(dev, serdev);
291+
if (ret) {
292+
dev_err(dev, "Unable to open UART device");
293+
return ret;
294+
}
295+
296+
ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
297+
if (ret < 0) {
298+
dev_err(dev, "Failed to set up host baud rate (%d)", ret);
299+
return ret;
300+
}
301+
302+
serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
303+
304304
return cros_ec_register(ec_dev);
305305
}
306306

0 commit comments

Comments
 (0)