* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 9:47 [PATCH] platform/chrome: cros_ec_uart: fix race condition Tzung-Bi Shih
@ 2022-12-29 16:57 ` Mark Hasemeyer
2022-12-29 20:09 ` Guenter Roeck
2022-12-29 20:07 ` Guenter Roeck
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Mark Hasemeyer @ 2022-12-29 16:57 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, robertzieba, bhanumaiya
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> index 0cef2888dffd..6916069f1599 100644
> --- a/drivers/platform/chrome/cros_ec_uart.c
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -270,7 +270,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> }
>
> serdev_device_set_drvdata(serdev, ec_dev);
> - serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> init_waitqueue_head(&ec_uart->response.wait_queue);
>
> ec_uart->serdev = serdev;
> @@ -300,6 +299,8 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> sizeof(struct ec_response_get_protocol_info);
> ec_dev->dout_size = sizeof(struct ec_host_request);
>
> + serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> +
> return cros_ec_register(ec_dev);
> }
Thanks for catching this. As a sanity check, I compiled and loaded the driver on
my local setup with these changes. LGTM.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 16:57 ` Mark Hasemeyer
@ 2022-12-29 20:09 ` Guenter Roeck
2023-01-03 18:33 ` Mark Hasemeyer
0 siblings, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2022-12-29 20:09 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: Tzung-Bi Shih, bleung, groeck, chrome-platform, robertzieba,
bhanumaiya
Hi Mark,
On Thu, Dec 29, 2022 at 8:58 AM Mark Hasemeyer <markhas@chromium.org> wrote:
>
> > diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> > index 0cef2888dffd..6916069f1599 100644
> > --- a/drivers/platform/chrome/cros_ec_uart.c
> > +++ b/drivers/platform/chrome/cros_ec_uart.c
> > @@ -270,7 +270,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> > }
> >
> > serdev_device_set_drvdata(serdev, ec_dev);
> > - serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> > init_waitqueue_head(&ec_uart->response.wait_queue);
> >
> > ec_uart->serdev = serdev;
> > @@ -300,6 +299,8 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> > sizeof(struct ec_response_get_protocol_info);
> > ec_dev->dout_size = sizeof(struct ec_host_request);
> >
> > + serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> > +
> > return cros_ec_register(ec_dev);
> > }
> Thanks for catching this. As a sanity check, I compiled and loaded the driver on
> my local setup with these changes. LGTM.
Please consider replying with a Reviewed-by: and/or Tested-by: tag.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 20:09 ` Guenter Roeck
@ 2023-01-03 18:33 ` Mark Hasemeyer
0 siblings, 0 replies; 7+ messages in thread
From: Mark Hasemeyer @ 2023-01-03 18:33 UTC (permalink / raw)
To: Guenter Roeck
Cc: Tzung-Bi Shih, bleung, groeck, chrome-platform, robertzieba,
bhanumaiya
> > > diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> > > index 0cef2888dffd..6916069f1599 100644
> > > --- a/drivers/platform/chrome/cros_ec_uart.c
> > > +++ b/drivers/platform/chrome/cros_ec_uart.c
> > > @@ -270,7 +270,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> > > }
> > >
> > > serdev_device_set_drvdata(serdev, ec_dev);
> > > - serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> > > init_waitqueue_head(&ec_uart->response.wait_queue);
> > >
> > > ec_uart->serdev = serdev;
> > > @@ -300,6 +299,8 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> > > sizeof(struct ec_response_get_protocol_info);
> > > ec_dev->dout_size = sizeof(struct ec_host_request);
> > >
> > > + serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> > > +
> > > return cros_ec_register(ec_dev);
> > > }
> > Thanks for catching this. As a sanity check, I compiled and loaded the driver on
> > my local setup with these changes. LGTM.
>
> Please consider replying with a Reviewed-by: and/or Tested-by: tag.
>
FWIW -
Tested-by: Mark Hasemeyer <markhas@chromium.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 9:47 [PATCH] platform/chrome: cros_ec_uart: fix race condition Tzung-Bi Shih
2022-12-29 16:57 ` Mark Hasemeyer
@ 2022-12-29 20:07 ` Guenter Roeck
2023-01-03 5:40 ` patchwork-bot+chrome-platform
2023-01-03 8:10 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-12-29 20:07 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, robertzieba, markhas, bhanumaiya
On Thu, Dec 29, 2022 at 1:47 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> From: Robert Zieba <robertzieba@google.com>
>
> serdev_device_set_client_ops() is called before `ec_dev` is fully
> initialized. This can result in cros_ec_uart_rx_bytes() being called
> while `ec_dev` is still not initialized, resulting in a kernel panic.
>
> Call serdev_device_set_client_ops() after `ec_dev` is initialized.
>
> Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> Signed-off-by: Robert Zieba <robertzieba@google.com>
> [tzungbi: modified commit message and fixed context conflict.]
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> This is from https://crrev.com/c/3490635. I just found the patch when
> reviewing some downstream patches.
>
> drivers/platform/chrome/cros_ec_uart.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> index 0cef2888dffd..6916069f1599 100644
> --- a/drivers/platform/chrome/cros_ec_uart.c
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -270,7 +270,6 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> }
>
> serdev_device_set_drvdata(serdev, ec_dev);
> - serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> init_waitqueue_head(&ec_uart->response.wait_queue);
>
> ec_uart->serdev = serdev;
> @@ -300,6 +299,8 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
> sizeof(struct ec_response_get_protocol_info);
> ec_dev->dout_size = sizeof(struct ec_host_request);
>
> + serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> +
> return cros_ec_register(ec_dev);
> }
>
> --
> 2.39.0.314.g84b9a713c41-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 9:47 [PATCH] platform/chrome: cros_ec_uart: fix race condition Tzung-Bi Shih
2022-12-29 16:57 ` Mark Hasemeyer
2022-12-29 20:07 ` Guenter Roeck
@ 2023-01-03 5:40 ` patchwork-bot+chrome-platform
2023-01-03 8:10 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+chrome-platform @ 2023-01-03 5:40 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, robertzieba, markhas, bhanumaiya
Hello:
This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:
On Thu, 29 Dec 2022 17:47:38 +0800 you wrote:
> From: Robert Zieba <robertzieba@google.com>
>
> serdev_device_set_client_ops() is called before `ec_dev` is fully
> initialized. This can result in cros_ec_uart_rx_bytes() being called
> while `ec_dev` is still not initialized, resulting in a kernel panic.
>
> Call serdev_device_set_client_ops() after `ec_dev` is initialized.
>
> [...]
Here is the summary with links:
- platform/chrome: cros_ec_uart: fix race condition
https://git.kernel.org/chrome-platform/c/56d4b33d45e3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] platform/chrome: cros_ec_uart: fix race condition
2022-12-29 9:47 [PATCH] platform/chrome: cros_ec_uart: fix race condition Tzung-Bi Shih
` (2 preceding siblings ...)
2023-01-03 5:40 ` patchwork-bot+chrome-platform
@ 2023-01-03 8:10 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+chrome-platform @ 2023-01-03 8:10 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, robertzieba, markhas, bhanumaiya
Hello:
This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:
On Thu, 29 Dec 2022 17:47:38 +0800 you wrote:
> From: Robert Zieba <robertzieba@google.com>
>
> serdev_device_set_client_ops() is called before `ec_dev` is fully
> initialized. This can result in cros_ec_uart_rx_bytes() being called
> while `ec_dev` is still not initialized, resulting in a kernel panic.
>
> Call serdev_device_set_client_ops() after `ec_dev` is initialized.
>
> [...]
Here is the summary with links:
- platform/chrome: cros_ec_uart: fix race condition
https://git.kernel.org/chrome-platform/c/56d4b33d45e3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread