Re: [PATCH] platform/x86/intel/pmc: validate LPM mode without stale index
Yousef Alhouseen <[email protected]>
| Newsgroups | org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAMuQ4bUs3hpQ50+qd2nXvqhRij6zLM_jWLTgh0ZDjWQFYGuMog@mail.gmail.com> |
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 <[email protected]> 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 <[email protected]> > > --- > > 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 <[email protected]> > > Thanks