Re: [PATCH] platform/x86/intel/pmc: validate LPM mode without stale index

From: Yousef Alhouseen

Date: Sat Jul 18 2026 - 08:21:18 EST


Agreed. I'll remove that blank line in v2 and carry your Reviewed-by.

Thanks,
Yousef

On Fri, 10 Jul 2026 09:09:28 -0700, David Box
<david.e.box@xxxxxxxxxxxxxxx> wrote:
> On Tue, Jun 30, 2026 at 12:51:25PM +0200, Yousef Alhouseen wrote:
> > pmc_lpm_mode_write() compares the requested mode with the loop variable
> > after pmc_for_each_mode(). If no low-power modes are enabled,
> > the loop never assigns that variable and the comparison reads an
> > uninitialized value.
> >
> > Track a successful match explicitly, including the empty-mode case.
> >
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> > ---
> > drivers/platform/x86/intel/pmc/core.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> > index 9f77c0716e59..9833df172d4e 100644
> > --- a/drivers/platform/x86/intel/pmc/core.c
> > +++ b/drivers/platform/x86/intel/pmc/core.c
> > @@ -1147,6 +1147,7 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
> > struct pmc_dev *pmcdev = s->private;
> > struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
> > bool clear = false, c10 = false;
> > + bool mode_enabled = false;
> > unsigned char buf[8];
> > int mode;
> > u32 reg;
> > @@ -1167,11 +1168,16 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
> > mode = sysfs_match_string(pmc_lpm_modes, buf);
> >
>
> Since you're now checking mode right after assignment remove this empty line to
> tie it to the test.
>
> > /* Check string matches enabled mode */
> > - pmc_for_each_mode(m, pmc)
> > - if (mode == m)
> > - break;
> > + if (mode >= 0) {
> > + pmc_for_each_mode(m, pmc) {
> > + if (mode == m) {
> > + mode_enabled = true;
> > + break;
> > + }
> > + }
> > + }
> >
> > - if (mode != m || mode < 0) {
> > + if (!mode_enabled) {
> > if (sysfs_streq(buf, "clear"))
> > clear = true;
> > else if (sysfs_streq(buf, "c10"))
> > --
> > 2.54.0
> >
>
> With the above change,
>
> Reviewed-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
>
> Thanks