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

[David Box]

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