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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75Vde1RAKTCTzmt0eHjNGrKUyi7r1rtNo934WW6wqi9T=ng@mail.gmail.com>
Date:   Tue, 3 Aug 2021 21:26:50 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Evgeny Novikov <novikov@...ras.ru>
Cc:     Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
        David E Box <david.e.box@...el.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <mgross@...ux.intel.com>,
        "David E. Box" <david.e.box@...ux.intel.com>,
        Gayatri Kammela <gayatri.kammela@...el.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        ldv-project@...uxtesting.org
Subject: Re: [PATCH] platform/x86: intel_pmc_core: Fix potential buffer overflows

On Tue, Aug 3, 2021 at 9:21 PM Evgeny Novikov <novikov@...ras.ru> 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@...il.com>

> Fixes: 005125bfd70e ("platform/x86: intel_pmc_core: Handle sub-states generically")
> Signed-off-by: Evgeny Novikov <novikov@...ras.ru>
> ---
>  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