diff mbox series

[v3] iio: cros_ec: Fix the allocation size for cros_ec_command

Message ID 20230630083112.2344234-1-yguoaz@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v3] iio: cros_ec: Fix the allocation size for cros_ec_command | expand

Commit Message

yguoaz June 30, 2023, 8:31 a.m. UTC 
The struct cros_ec_command contains several integer fields and a
trailing array. An allocation size neglecting the integer fields can
lead to buffer overrun.

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Yiyuan Guo <yguoaz@gmail.com>
---
v2->v3: 
 * Added R-b tag from Tzung-Bi Shih
 * Aligned the code by adding an extra tab before "max"
 * Added a patch changelog
v1->v2: Prefixed the commit title with "iio: cros_ec:"

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Guenter Roeck June 30, 2023, 2:06 p.m. UTC | #1 
On Fri, Jun 30, 2023 at 1:31 AM Yiyuan Guo <yguoaz@gmail.com> wrote:
>
> The struct cros_ec_command contains several integer fields and a
> trailing array. An allocation size neglecting the integer fields can
> lead to buffer overrun.
>
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> Signed-off-by: Yiyuan Guo <yguoaz@gmail.com>

Please _never_ send a patch as reply to a previous one, much less with
a Re: subject.

Guenter

> ---
> v2->v3:
>  * Added R-b tag from Tzung-Bi Shih
>  * Aligned the code by adding an extra tab before "max"
>  * Added a patch changelog
> v1->v2: Prefixed the commit title with "iio: cros_ec:"
>
>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 943e9e14d1e9..b72d39fc2434 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -253,7 +253,7 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>         platform_set_drvdata(pdev, indio_dev);
>
>         state->ec = ec->ec_dev;
> -       state->msg = devm_kzalloc(&pdev->dev,
> +       state->msg = devm_kzalloc(&pdev->dev, sizeof(*state->msg) +
>                                 max((u16)sizeof(struct ec_params_motion_sense),
>                                 state->ec->max_response), GFP_KERNEL);
>         if (!state->msg)
> --
> 2.25.1
>
yguoaz June 30, 2023, 2:42 p.m. UTC | #2 
Got it. I have resent the patch in a separate thread.

Thanks,
Yiyuan

On Fri, Jun 30, 2023 at 10:07 PM Guenter Roeck <groeck@google.com> wrote:
>
> On Fri, Jun 30, 2023 at 1:31 AM Yiyuan Guo <yguoaz@gmail.com> wrote:
> >
> > The struct cros_ec_command contains several integer fields and a
> > trailing array. An allocation size neglecting the integer fields can
> > lead to buffer overrun.
> >
> > Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > Signed-off-by: Yiyuan Guo <yguoaz@gmail.com>
>
> Please _never_ send a patch as reply to a previous one, much less with
> a Re: subject.
>
> Guenter
>
> > ---
> > v2->v3:
> >  * Added R-b tag from Tzung-Bi Shih
> >  * Aligned the code by adding an extra tab before "max"
> >  * Added a patch changelog
> > v1->v2: Prefixed the commit title with "iio: cros_ec:"
> >
> >  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > index 943e9e14d1e9..b72d39fc2434 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > @@ -253,7 +253,7 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> >         platform_set_drvdata(pdev, indio_dev);
> >
> >         state->ec = ec->ec_dev;
> > -       state->msg = devm_kzalloc(&pdev->dev,
> > +       state->msg = devm_kzalloc(&pdev->dev, sizeof(*state->msg) +
> >                                 max((u16)sizeof(struct ec_params_motion_sense),
> >                                 state->ec->max_response), GFP_KERNEL);
> >         if (!state->msg)
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 943e9e14d1e9..b72d39fc2434 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -253,7 +253,7 @@  int cros_ec_sensors_core_init(struct platform_device *pdev,
 	platform_set_drvdata(pdev, indio_dev);
 
 	state->ec = ec->ec_dev;
-	state->msg = devm_kzalloc(&pdev->dev,
+	state->msg = devm_kzalloc(&pdev->dev, sizeof(*state->msg) +
 				max((u16)sizeof(struct ec_params_motion_sense),
 				state->ec->max_response), GFP_KERNEL);
 	if (!state->msg)