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
