diff mbox series

pwm: lp3943: Use of_property_count_u32_elems() to get property length

Message ID 20240731201407.1838385-8-robh@kernel.org
State Accepted
Headers show
Series pwm: lp3943: Use of_property_count_u32_elems() to get property length | expand

Commit Message

Rob Herring July 31, 2024, 8:14 p.m. UTC
Replace of_get_property() with the type specific
of_property_count_u32_elems() to get the property length.

This is part of a larger effort to remove callers of of_get_property()
and similar functions. of_get_property() leaks the DT property data
pointer which is a problem for dynamically allocated nodes which may
be freed.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/pwm/pwm-lp3943.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Uwe Kleine-König Aug. 1, 2024, 8:58 a.m. UTC | #1
Hello Rob,

On Wed, Jul 31, 2024 at 02:14:03PM -0600, Rob Herring (Arm) wrote:
> Replace of_get_property() with the type specific
> of_property_count_u32_elems() to get the property length.
> 
> This is part of a larger effort to remove callers of of_get_property()
> and similar functions. of_get_property() leaks the DT property data
> pointer which is a problem for dynamically allocated nodes which may
> be freed.

To understand that right: The problem is that of_get_property() returns
pp->value, which might be freed. In this driver this isn't problematic
as the returned value is just used for a NULL check. So this isn't
urgent and queuing it for the next merge window is fine, right?

Best regards
Uwe
Rob Herring Aug. 1, 2024, 3:52 p.m. UTC | #2
On Thu, Aug 1, 2024 at 2:58 AM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello Rob,
>
> On Wed, Jul 31, 2024 at 02:14:03PM -0600, Rob Herring (Arm) wrote:
> > Replace of_get_property() with the type specific
> > of_property_count_u32_elems() to get the property length.
> >
> > This is part of a larger effort to remove callers of of_get_property()
> > and similar functions. of_get_property() leaks the DT property data
> > pointer which is a problem for dynamically allocated nodes which may
> > be freed.
>
> To understand that right: The problem is that of_get_property() returns
> pp->value, which might be freed. In this driver this isn't problematic
> as the returned value is just used for a NULL check. So this isn't
> urgent and queuing it for the next merge window is fine, right?

Yes, 6.12 is fine.

Rob
Uwe Kleine-König Aug. 2, 2024, 2:06 a.m. UTC | #3
Hello Rob,

On Thu, Aug 01, 2024 at 09:52:18AM -0600, Rob Herring wrote:
> On Thu, Aug 1, 2024 at 2:58 AM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> > On Wed, Jul 31, 2024 at 02:14:03PM -0600, Rob Herring (Arm) wrote:
> > > Replace of_get_property() with the type specific
> > > of_property_count_u32_elems() to get the property length.
> > >
> > > This is part of a larger effort to remove callers of of_get_property()
> > > and similar functions. of_get_property() leaks the DT property data
> > > pointer which is a problem for dynamically allocated nodes which may
> > > be freed.
> >
> > To understand that right: The problem is that of_get_property() returns
> > pp->value, which might be freed. In this driver this isn't problematic
> > as the returned value is just used for a NULL check. So this isn't
> > urgent and queuing it for the next merge window is fine, right?
> 
> Yes, 6.12 is fine.

Thanks for confirming, queued in
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
for 6.12-rc.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-lp3943.c b/drivers/pwm/pwm-lp3943.c
index 61189cea1046..f0e94c9e5956 100644
--- a/drivers/pwm/pwm-lp3943.c
+++ b/drivers/pwm/pwm-lp3943.c
@@ -218,7 +218,7 @@  static int lp3943_pwm_parse_dt(struct device *dev,
 	struct lp3943_platform_data *pdata;
 	struct lp3943_pwm_map *pwm_map;
 	enum lp3943_pwm_output *output;
-	int i, err, proplen, count = 0;
+	int i, err, count = 0;
 	u32 num_outputs;
 
 	if (!node)
@@ -234,11 +234,8 @@  static int lp3943_pwm_parse_dt(struct device *dev,
 	 */
 
 	for (i = 0; i < LP3943_NUM_PWMS; i++) {
-		if (!of_get_property(node, name[i], &proplen))
-			continue;
-
-		num_outputs = proplen / sizeof(u32);
-		if (num_outputs == 0)
+		num_outputs = of_property_count_u32_elems(node, name[i]);
+		if (num_outputs <= 0)
 			continue;
 
 		output = devm_kcalloc(dev, num_outputs, sizeof(*output),