[PATCH] platform/x86: intel_pmc_core: Fix uninitialized pmc/map in pmc_core_send_ltr_ignore

[PATCH] platform/x86: intel_pmc_core: Fix uninitialized pmc/map in pmc_core_send_ltr_ignore

[Purva Yeshi]

Fix Smatch-detected issue:

drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
error: uninitialized symbol 'pmc'.

drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
error: uninitialized symbol 'map'.

drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
error: we previously assumed 'pmc' could be null (see line 479)


Prevents uninitialized symbol warnings detected by smatch.

Ensures map is not accessed if pmc is NULL, preventing dereferencing
of uninitialized pointers

Add defensive check for pmc and map to catch any unexpected edge cases
and ensure all required pointers are valid.

Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
---
 drivers/platform/x86/intel/pmc/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 7a1d11f2914f..e674b940e29e 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -462,8 +462,8 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
 
 int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 {
-	struct pmc *pmc;
-	const struct pmc_reg_map *map;
+	struct pmc *pmc = NULL;
+	const struct pmc_reg_map *map = NULL;
 	u32 reg;
 	unsigned int pmc_index;
 	int ltr_index;
@@ -480,6 +480,9 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 			continue;
 
 		map = pmc->map;
+		if (!map)
+			continue;
+
 		if (ltr_index <= map->ltr_ignore_max)
 			break;
 
@@ -491,7 +494,7 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
 		ltr_index = ltr_index - (map->ltr_ignore_max + 2) - 1;
 	}
 
-	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0)
+	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0 || !pmc || !map)
 		return -EINVAL;
 
 	pr_debug("ltr_ignore for pmc%d: ltr_index:%d\n", pmc_index, ltr_index);
-- 
2.34.1

Re: [PATCH] platform/x86: intel_pmc_core: Fix uninitialized pmc/map in pmc_core_send_ltr_ignore

[Ilpo Järvinen]

On Thu, 17 Apr 2025, Purva Yeshi wrote:

> Fix Smatch-detected issue:
> 
> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
> error: uninitialized symbol 'pmc'.
> 
> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
> error: uninitialized symbol 'map'.
> 
> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
> error: we previously assumed 'pmc' could be null (see line 479)
> 
> 
> Prevents uninitialized symbol warnings detected by smatch.
> 
> Ensures map is not accessed if pmc is NULL, preventing dereferencing
> of uninitialized pointers
> 
> Add defensive check for pmc and map to catch any unexpected edge cases
> and ensure all required pointers are valid.
> 
> Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 7a1d11f2914f..e674b940e29e 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -462,8 +462,8 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
>  
>  int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
>  {
> -	struct pmc *pmc;
> -	const struct pmc_reg_map *map;
> +	struct pmc *pmc = NULL;
> +	const struct pmc_reg_map *map = NULL;
>  	u32 reg;
>  	unsigned int pmc_index;
>  	int ltr_index;
> @@ -480,6 +480,9 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
>  			continue;
>  
>  		map = pmc->map;
> +		if (!map)
> +			continue;

How can this happen?? If pmc is created, it should have a valid ->map 
AFAICT. Did you even read that code at all???

> +
>  		if (ltr_index <= map->ltr_ignore_max)
>  			break;
>  
> @@ -491,7 +494,7 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
>  		ltr_index = ltr_index - (map->ltr_ignore_max + 2) - 1;
>  	}
>  
> -	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0)
> +	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0 || !pmc || !map)

What are the situations pmc_index >= ARRAY_SIZE(pmcdev->pmcs) check 
didn't catch where these new checks do something useful??

Lots of noise but little real substance in this patch?

-- 
 i.

Re: [PATCH] platform/x86: intel_pmc_core: Fix uninitialized pmc/map in pmc_core_send_ltr_ignore

[Purva Yeshi]

On 17/04/25 18:43, Ilpo Järvinen wrote:
> On Thu, 17 Apr 2025, Purva Yeshi wrote:
> 
>> Fix Smatch-detected issue:
>>
>> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
>> error: uninitialized symbol 'pmc'.
>>
>> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
>> error: uninitialized symbol 'map'.
>>
>> drivers/platform/x86/intel/pmc/core.c:501 pmc_core_send_ltr_ignore()
>> error: we previously assumed 'pmc' could be null (see line 479)
>>
>>
>> Prevents uninitialized symbol warnings detected by smatch.
>>
>> Ensures map is not accessed if pmc is NULL, preventing dereferencing
>> of uninitialized pointers
>>
>> Add defensive check for pmc and map to catch any unexpected edge cases
>> and ensure all required pointers are valid.
>>
>> Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
>> ---
>>   drivers/platform/x86/intel/pmc/core.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index 7a1d11f2914f..e674b940e29e 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -462,8 +462,8 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
>>   
>>   int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
>>   {
>> -	struct pmc *pmc;
>> -	const struct pmc_reg_map *map;
>> +	struct pmc *pmc = NULL;
>> +	const struct pmc_reg_map *map = NULL;
>>   	u32 reg;
>>   	unsigned int pmc_index;
>>   	int ltr_index;
>> @@ -480,6 +480,9 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)

>>   			continue;
>>   
>>   		map = pmc->map;
>> +		if (!map)
>> +			continue;
> 
> How can this happen?? If pmc is created, it should have a valid ->map
> AFAICT. Did you even read that code at all???

Hi,

Thanks for the feedback.

Yes, I did read through the code and I understand your point.

The motivation behind the patch was a Smatch warning about possible 
uninitialized use of map and pmc, even though they are logically 
guarded. I now see that these checks may not be necessary given the 
existing control flow.

> 
>> +
>>   		if (ltr_index <= map->ltr_ignore_max)
>>   			break;
>>   
>> @@ -491,7 +494,7 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore)
>>   		ltr_index = ltr_index - (map->ltr_ignore_max + 2) - 1;
>>   	}
>>   
>> -	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0)
>> +	if (pmc_index >= ARRAY_SIZE(pmcdev->pmcs) || ltr_index < 0 || !pmc || !map)
> 
> What are the situations pmc_index >= ARRAY_SIZE(pmcdev->pmcs) check
> didn't catch where these new checks do something useful??
> 
> Lots of noise but little real substance in this patch?

You're right, if pmc is non-NULL, then map should also be valid, and the 
bounds check on pmc_index already prevents out-of-bounds access. Adding 
further checks might just add noise.

I'll drop the patch unless there's a cleaner way to restructure the 
logic to make Smatch silence without redundant checks.

Thanks again for the clarification!

Best regards,
Purva

>