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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.