All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] leds: lp55xx: avoid dereferencing a stale platform_data pointer
@ 2015-08-04  0:47 Toshi Kikuchi
  2015-08-04  1:07 ` Kim, Milo
  0 siblings, 1 reply; 3+ messages in thread
From: Toshi Kikuchi @ 2015-08-04  0:47 UTC (permalink / raw)
  To: linux-leds; +Cc: milo.kim

lp55xx_of_populate_pdata() allocates platform_data dynamically if DT
is used. The memory area is allocated with devm_kzalloc() so it is
automatically freed if the driver is unloaded. The driver should clear
the pointer to platform_data before it is unloaded, otherwise it will
use the stale pointer accidentally if the driver is reloaded.

Change-Id: Ia097ed83b92cec5a52534a71dcb5322869c2a7b1
Signed-off-by: Toshi Kikuchi <toshik@chromium.org>
---
 drivers/leds/leds-lp5521.c | 3 +++
 drivers/leds/leds-lp5523.c | 3 +++
 drivers/leds/leds-lp5562.c | 3 +++
 drivers/leds/leds-lp8501.c | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 8ca197a..1f39922 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -582,6 +582,9 @@ static int lp5521_remove(struct i2c_client *client)
 	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
+	if (client->dev.of_node)
+		client->dev.platform_data = NULL;
+
 	return 0;
 }
 
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index 584dbbc..15e10da 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -948,6 +948,9 @@ static int lp5523_remove(struct i2c_client *client)
 	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
+	if (client->dev.of_node)
+		client->dev.platform_data = NULL;
+
 	return 0;
 }
 
diff --git a/drivers/leds/leds-lp5562.c b/drivers/leds/leds-lp5562.c
index ca85724..32c8392 100644
--- a/drivers/leds/leds-lp5562.c
+++ b/drivers/leds/leds-lp5562.c
@@ -582,6 +582,9 @@ static int lp5562_remove(struct i2c_client *client)
 	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
+	if (client->dev.of_node)
+		client->dev.platform_data = NULL;
+
 	return 0;
 }
 
diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
index d3098e3..e8542be 100644
--- a/drivers/leds/leds-lp8501.c
+++ b/drivers/leds/leds-lp8501.c
@@ -376,6 +376,9 @@ static int lp8501_remove(struct i2c_client *client)
 	lp55xx_unregister_leds(led, chip);
 	lp55xx_deinit_device(chip);
 
+	if (client->dev.of_node)
+		client->dev.platform_data = NULL;
+
 	return 0;
 }
 
-- 
2.2.1.62.g3f15098

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] leds: lp55xx: avoid dereferencing a stale platform_data pointer
  2015-08-04  0:47 [PATCH] leds: lp55xx: avoid dereferencing a stale platform_data pointer Toshi Kikuchi
@ 2015-08-04  1:07 ` Kim, Milo
  2015-08-04 18:07   ` Toshi Kikuchi
  0 siblings, 1 reply; 3+ messages in thread
From: Kim, Milo @ 2015-08-04  1:07 UTC (permalink / raw)
  To: Toshi Kikuchi; +Cc: linux-leds

Hi Toshi,

On 8/4/2015 9:47 AM, Toshi Kikuchi wrote:
> lp55xx_of_populate_pdata() allocates platform_data dynamically if DT
> is used. The memory area is allocated with devm_kzalloc() so it is
> automatically freed if the driver is unloaded. The driver should clear
> the pointer to platform_data before it is unloaded, otherwise it will
> use the stale pointer accidentally if the driver is reloaded.
>
> Change-Id: Ia097ed83b92cec5a52534a71dcb5322869c2a7b1
> Signed-off-by: Toshi Kikuchi <toshik@chromium.org>
> ---
>   drivers/leds/leds-lp5521.c | 3 +++
>   drivers/leds/leds-lp5523.c | 3 +++
>   drivers/leds/leds-lp5562.c | 3 +++
>   drivers/leds/leds-lp8501.c | 3 +++
>   4 files changed, 12 insertions(+)
>
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index 8ca197a..1f39922 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -582,6 +582,9 @@ static int lp5521_remove(struct i2c_client *client)
>   	lp55xx_unregister_leds(led, chip);
>   	lp55xx_deinit_device(chip);
>
> +	if (client->dev.of_node)
> +		client->dev.platform_data = NULL;
> +
>   	return 0;
>   }
>
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index 584dbbc..15e10da 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -948,6 +948,9 @@ static int lp5523_remove(struct i2c_client *client)
>   	lp55xx_unregister_leds(led, chip);
>   	lp55xx_deinit_device(chip);
>
> +	if (client->dev.of_node)
> +		client->dev.platform_data = NULL;
> +
>   	return 0;
>   }
>
> diff --git a/drivers/leds/leds-lp5562.c b/drivers/leds/leds-lp5562.c
> index ca85724..32c8392 100644
> --- a/drivers/leds/leds-lp5562.c
> +++ b/drivers/leds/leds-lp5562.c
> @@ -582,6 +582,9 @@ static int lp5562_remove(struct i2c_client *client)
>   	lp55xx_unregister_leds(led, chip);
>   	lp55xx_deinit_device(chip);
>
> +	if (client->dev.of_node)
> +		client->dev.platform_data = NULL;
> +
>   	return 0;
>   }
>
> diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
> index d3098e3..e8542be 100644
> --- a/drivers/leds/leds-lp8501.c
> +++ b/drivers/leds/leds-lp8501.c
> @@ -376,6 +376,9 @@ static int lp8501_remove(struct i2c_client *client)
>   	lp55xx_unregister_leds(led, chip);
>   	lp55xx_deinit_device(chip);
>
> +	if (client->dev.of_node)
> +		client->dev.platform_data = NULL;
> +
>   	return 0;
>   }
>
>

Thanks a lot for catching this. I think it would be better to move these 
code to common part - 'leds-lp55xx-common.c' and call exported function 
in each driver.

Best regards,
Milo

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] leds: lp55xx: avoid dereferencing a stale platform_data pointer
  2015-08-04  1:07 ` Kim, Milo
@ 2015-08-04 18:07   ` Toshi Kikuchi
  0 siblings, 0 replies; 3+ messages in thread
From: Toshi Kikuchi @ 2015-08-04 18:07 UTC (permalink / raw)
  To: Kim, Milo; +Cc: linux-leds

Hi Milo,

I posted another patch (v2). Let me know what you think.

Thanks,
Toshi

On Mon, Aug 3, 2015 at 6:07 PM, Kim, Milo <milo.kim@ti.com> wrote:
> Hi Toshi,
>
>
> On 8/4/2015 9:47 AM, Toshi Kikuchi wrote:
>>
>> lp55xx_of_populate_pdata() allocates platform_data dynamically if DT
>> is used. The memory area is allocated with devm_kzalloc() so it is
>> automatically freed if the driver is unloaded. The driver should clear
>> the pointer to platform_data before it is unloaded, otherwise it will
>> use the stale pointer accidentally if the driver is reloaded.
>>
>> Change-Id: Ia097ed83b92cec5a52534a71dcb5322869c2a7b1
>> Signed-off-by: Toshi Kikuchi <toshik@chromium.org>
>> ---
>>   drivers/leds/leds-lp5521.c | 3 +++
>>   drivers/leds/leds-lp5523.c | 3 +++
>>   drivers/leds/leds-lp5562.c | 3 +++
>>   drivers/leds/leds-lp8501.c | 3 +++
>>   4 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
>> index 8ca197a..1f39922 100644
>> --- a/drivers/leds/leds-lp5521.c
>> +++ b/drivers/leds/leds-lp5521.c
>> @@ -582,6 +582,9 @@ static int lp5521_remove(struct i2c_client *client)
>>         lp55xx_unregister_leds(led, chip);
>>         lp55xx_deinit_device(chip);
>>
>> +       if (client->dev.of_node)
>> +               client->dev.platform_data = NULL;
>> +
>>         return 0;
>>   }
>>
>> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
>> index 584dbbc..15e10da 100644
>> --- a/drivers/leds/leds-lp5523.c
>> +++ b/drivers/leds/leds-lp5523.c
>> @@ -948,6 +948,9 @@ static int lp5523_remove(struct i2c_client *client)
>>         lp55xx_unregister_leds(led, chip);
>>         lp55xx_deinit_device(chip);
>>
>> +       if (client->dev.of_node)
>> +               client->dev.platform_data = NULL;
>> +
>>         return 0;
>>   }
>>
>> diff --git a/drivers/leds/leds-lp5562.c b/drivers/leds/leds-lp5562.c
>> index ca85724..32c8392 100644
>> --- a/drivers/leds/leds-lp5562.c
>> +++ b/drivers/leds/leds-lp5562.c
>> @@ -582,6 +582,9 @@ static int lp5562_remove(struct i2c_client *client)
>>         lp55xx_unregister_leds(led, chip);
>>         lp55xx_deinit_device(chip);
>>
>> +       if (client->dev.of_node)
>> +               client->dev.platform_data = NULL;
>> +
>>         return 0;
>>   }
>>
>> diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
>> index d3098e3..e8542be 100644
>> --- a/drivers/leds/leds-lp8501.c
>> +++ b/drivers/leds/leds-lp8501.c
>> @@ -376,6 +376,9 @@ static int lp8501_remove(struct i2c_client *client)
>>         lp55xx_unregister_leds(led, chip);
>>         lp55xx_deinit_device(chip);
>>
>> +       if (client->dev.of_node)
>> +               client->dev.platform_data = NULL;
>> +
>>         return 0;
>>   }
>>
>>
>
> Thanks a lot for catching this. I think it would be better to move these
> code to common part - 'leds-lp55xx-common.c' and call exported function in
> each driver.
>
> Best regards,
> Milo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-08-04 18:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-04  0:47 [PATCH] leds: lp55xx: avoid dereferencing a stale platform_data pointer Toshi Kikuchi
2015-08-04  1:07 ` Kim, Milo
2015-08-04 18:07   ` Toshi Kikuchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.