diff mbox series

[v3,13/23] platform/chrome: cros_ec: don't allocate `din` and `dout` in cros_ec_register()

Message ID 20220608110734.2928245-14-tzungbi@kernel.org (mailing list archive)
State Superseded
Headers show
Series platform/chrome: Kunit tests and refactor for cros_ec_query_all() | expand

Commit Message

Tzung-Bi Shih June 8, 2022, 11:07 a.m. UTC
Don't allocate `din` and `dout` in cros_ec_register() as they will be
allocated soon in cros_ec_query_all().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
No change from v2.

 drivers/platform/chrome/cros_ec.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Guenter Roeck June 8, 2022, 4:15 p.m. UTC | #1
On Wed, Jun 8, 2022 at 4:08 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Don't allocate `din` and `dout` in cros_ec_register() as they will be
> allocated soon in cros_ec_query_all().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
> No change from v2.
>
>  drivers/platform/chrome/cros_ec.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index e51a3f2176c7..29d3b544dafb 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -188,14 +188,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>         ec_dev->max_passthru = 0;
>         ec_dev->ec = NULL;
>         ec_dev->pd = NULL;
> -
> -       ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> -       if (!ec_dev->din)
> -               return -ENOMEM;
> -
> -       ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
> -       if (!ec_dev->dout)
> -               return -ENOMEM;
> +       ec_dev->din = NULL;
> +       ec_dev->dout = NULL;
>
>         mutex_init(&ec_dev->lock);
>
> --
> 2.36.1.255.ge46751e96f-goog
>
Tzung-Bi Shih June 9, 2022, 5:09 a.m. UTC | #2
On Wed, Jun 08, 2022 at 09:15:56AM -0700, Guenter Roeck wrote:
> On Wed, Jun 8, 2022 at 4:08 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > Don't allocate `din` and `dout` in cros_ec_register() as they will be
> > allocated soon in cros_ec_query_all().

Pardon me, I should test them earlier.

I misunderstood.  The patch will cause kernel crash (NULL dereference)
because cros_ec_query_all() relies on `din` and `dout` for getting protocol
info and then it reallocates the buffers according to the info later.

I think we should just leave them as they are.  Will drop this patch and next
patch ([v3,14/23] platform/chrome: don't use devm variants for `din` and
`dout`).
diff mbox series

Patch

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index e51a3f2176c7..29d3b544dafb 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -188,14 +188,8 @@  int cros_ec_register(struct cros_ec_device *ec_dev)
 	ec_dev->max_passthru = 0;
 	ec_dev->ec = NULL;
 	ec_dev->pd = NULL;
-
-	ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
-	if (!ec_dev->din)
-		return -ENOMEM;
-
-	ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
-	if (!ec_dev->dout)
-		return -ENOMEM;
+	ec_dev->din = NULL;
+	ec_dev->dout = NULL;
 
 	mutex_init(&ec_dev->lock);