Re: [PATCH] platform/x86: intel_pmc_core: Fix potential buffer overflows

[Andy Shevchenko]

On Tue, Aug 3, 2021 at 9:21 PM Evgeny Novikov <novikov@xxxxxxxxx> wrote:
>
> It looks like pmc_core_get_low_power_modes() mixes up modes and
> priorities. In addition to invalid behavior, potentially this can
> cause buffer overflows since the driver reads priorities from the
> register and then it uses them as indexes for array lpm_priority
> that can contain 8 elements at most. The patch swaps modes and
> priorities.
>
> Found by Linux Driver Verification project (linuxtesting.org).

Seems legit.
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

> Fixes: 005125bfd70e ("platform/x86: intel_pmc_core: Handle sub-states generically")
> Signed-off-by: Evgeny Novikov <novikov@xxxxxxxxx>
> ---
>  drivers/platform/x86/intel_pmc_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index b0e486a6bdfb..667b3df03764 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -1469,8 +1469,8 @@ static void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
>                 int pri0 = GENMASK(3, 0) & priority;
>                 int pri1 = (GENMASK(7, 4) & priority) >> 4;
>
> -               lpm_priority[pri0] = mode;
> -               lpm_priority[pri1] = mode + 1;
> +               lpm_priority[mode] = pri0;

I would write it as + 0, but up to you and maintainers.

> +               lpm_priority[mode + 1] = pri1;
>         }
>
>         /*
> --
> 2.26.2
>


-- 
With Best Regards,
Andy Shevchenko