[PATCH] leds: leds-wm831x-status: init chip_pdata before access

[PATCH] leds: leds-wm831x-status: init chip_pdata before access

[Shuah Khan]

wm831x_status_probe() accesses status from chip_pdata before
initializing it. Fix it.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 drivers/leds/leds-wm831x-status.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index c48b80574f02..5060c83f3b25 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -212,7 +212,7 @@ static int wm831x_status_probe(struct platform_device *pdev)
 	struct wm831x_status_pdata pdata;
 	struct wm831x_status *drvdata;
 	struct resource *res;
-	int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
+	int id;
 	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_REG, 0);
@@ -229,9 +229,10 @@ static int wm831x_status_probe(struct platform_device *pdev)
 	drvdata->wm831x = wm831x;
 	drvdata->reg = res->start;
 
-	if (dev_get_platdata(wm831x->dev))
+	if (dev_get_platdata(wm831x->dev)) {
 		chip_pdata = dev_get_platdata(wm831x->dev);
-	else
+		id = pdev->id % ARRAY_SIZE(chip_pdata->status);
+	} else
 		chip_pdata = NULL;
 
 	memset(&pdata, 0, sizeof(pdata));
-- 
2.34.1

Re: [PATCH] leds: leds-wm831x-status: init chip_pdata before access

[Pavel Machek]

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

Hi!

> wm831x_status_probe() accesses status from chip_pdata before
> initializing it. Fix it.
> 
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

Does it? ARRAY_SIZE() will be compile-time constant, no?

What is the bug? Did you test the code?

Best regards,
								Pavel


> +++ b/drivers/leds/leds-wm831x-status.c
> @@ -212,7 +212,7 @@ static int wm831x_status_probe(struct platform_device *pdev)
>  	struct wm831x_status_pdata pdata;
>  	struct wm831x_status *drvdata;
>  	struct resource *res;
> -	int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
> +	int id;
>  	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> @@ -229,9 +229,10 @@ static int wm831x_status_probe(struct platform_device *pdev)
>  	drvdata->wm831x = wm831x;
>  	drvdata->reg = res->start;
>  
> -	if (dev_get_platdata(wm831x->dev))
> +	if (dev_get_platdata(wm831x->dev)) {
>  		chip_pdata = dev_get_platdata(wm831x->dev);
> -	else
> +		id = pdev->id % ARRAY_SIZE(chip_pdata->status);
> +	} else
>  		chip_pdata = NULL;
>  
>  	memset(&pdata, 0, sizeof(pdata));

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

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

Re: [PATCH] leds: leds-wm831x-status: init chip_pdata before access

[Shuah Khan]

On 11/22/22 14:05, Pavel Machek wrote:
> Hi!
> 
>> wm831x_status_probe() accesses status from chip_pdata before
>> initializing it. Fix it.
>>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> Does it? ARRAY_SIZE() will be compile-time constant, no?
> 
> What is the bug? Did you test the code?
> 

Is ARRAY_SIZE() safe when accessing the status chip_pdata->status?
I wasn't sure. If so, this change isn't necessary.

thanks,
-- Shuah

Re: [PATCH] leds: leds-wm831x-status: init chip_pdata before access

[Pavel Machek]

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

On Tue 2022-11-22 15:39:35, Shuah Khan wrote:
> On 11/22/22 14:05, Pavel Machek wrote:
> > Hi!
> > 
> > > wm831x_status_probe() accesses status from chip_pdata before
> > > initializing it. Fix it.
> > > 
> > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> > 
> > Does it? ARRAY_SIZE() will be compile-time constant, no?
> > 
> > What is the bug? Did you test the code?
> > 
> 
> Is ARRAY_SIZE() safe when accessing the status chip_pdata->status?
> I wasn't sure. If so, this change isn't necessary.

I think so. Feel free to quite C standard to prove me wrong :-).

Best regards,
								Pavel

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

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