[PATCH] power: pfuze100: Ensure loop index is incremented

[PATCH] power: pfuze100: Ensure loop index is incremented

[Andrew Goodbody]

The for loop in se_desc uses i as the loop index and also to cause the
loop to end if the passed in name is not found. However i is not
incremented which could cause the loop to continue indefinitely and
access out of bounds memory.
Add an increment of i to ensure that the loop terminates correctly in
the case where name is not found.

This issue found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/power/regulator/pfuze100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/regulator/pfuze100.c b/drivers/power/regulator/pfuze100.c
index bf3a7019411..eff166b368b 100644
--- a/drivers/power/regulator/pfuze100.c
+++ b/drivers/power/regulator/pfuze100.c
@@ -247,7 +247,7 @@ static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *d
 {
 	int i;
 
-	for (i = 0; i < size; desc++) {
+	for (i = 0; i < size; i++, desc++) {
 		if (!strcmp(desc->name, name))
 			return desc;
 		continue;

---
base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
change-id: 20250703-pfuze100_fix-4a0a1c3084cf

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] power: pfuze100: Ensure loop index is incremented

[Quentin Schulz]

Hi Andrew,

On 7/3/25 1:31 PM, Andrew Goodbody wrote:
> The for loop in se_desc uses i as the loop index and also to cause the
> loop to end if the passed in name is not found. However i is not
> incremented which could cause the loop to continue indefinitely and
> access out of bounds memory.
> Add an increment of i to ensure that the loop terminates correctly in
> the case where name is not found.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

Re: [PATCH] power: pfuze100: Ensure loop index is incremented

[Andrew Goodbody]

ping?

On 03/07/2025 12:31, Andrew Goodbody wrote:
> The for loop in se_desc uses i as the loop index and also to cause the
> loop to end if the passed in name is not found. However i is not
> incremented which could cause the loop to continue indefinitely and
> access out of bounds memory.
> Add an increment of i to ensure that the loop terminates correctly in
> the case where name is not found.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>   drivers/power/regulator/pfuze100.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/regulator/pfuze100.c b/drivers/power/regulator/pfuze100.c
> index bf3a7019411..eff166b368b 100644
> --- a/drivers/power/regulator/pfuze100.c
> +++ b/drivers/power/regulator/pfuze100.c
> @@ -247,7 +247,7 @@ static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *d
>   {
>   	int i;
>   
> -	for (i = 0; i < size; desc++) {
> +	for (i = 0; i < size; i++, desc++) {
>   		if (!strcmp(desc->name, name))
>   			return desc;
>   		continue;
> 
> ---
> base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
> change-id: 20250703-pfuze100_fix-4a0a1c3084cf
> 
> Best regards,

Re: [PATCH] power: pfuze100: Ensure loop index is incremented

[Andrew Goodbody]

On 14/07/2025 13:25, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 7/3/25 1:31 PM, Andrew Goodbody wrote:
>> The for loop in se_desc uses i as the loop index and also to cause the
>> loop to end if the passed in name is not found. However i is not
>> incremented which could cause the loop to continue indefinitely and
>> access out of bounds memory.
>> Add an increment of i to ensure that the loop terminates correctly in
>> the case where name is not found.
>>
>> This issue found by Smatch.
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Thanks!
> Quentin

Is there anything else needed before this can be merged please?

Thanks,
Andrew

Re: [PATCH] power: pfuze100: Ensure loop index is incremented

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Thu, Jul 03, 2025 at 12:31:50PM +0100, Andrew Goodbody wrote:

> The for loop in se_desc uses i as the loop index and also to cause the
> loop to end if the passed in name is not found. However i is not
> incremented which could cause the loop to continue indefinitely and
> access out of bounds memory.
> Add an increment of i to ensure that the loop terminates correctly in
> the case where name is not found.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>  drivers/power/regulator/pfuze100.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I size tested this as part of merging and saw unexpected shrinkage. In
turn, this got me to look harder at the code and I think the best answer
is to refactor things so that se_desc(...) follow the normal (linux
kernel) pattern of for (i = 0; i < ARRAY_SIZE(desc); i++) instead of
being passed size. That's I think the root of this confusion too. I'll
post a patch shortly.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]