Re: [PATCH] of: property: Fix of_fwnode_get_reference_args() with negative index

[Krzysztof Kozlowski]

On Thu, Jun 11, 2026 at 12:28:06PM +0200, Alban Bedel wrote:
> fwnode_property_get_reference_args() should return -ENOENT when an out
> of bound index is passed. An issue arised with the OF backend because
> the OF API use signed indexes while the fwnode API use unsigned ones.
> When an index value greater the INT_MAX was passed to the OF backend
> it got casted to a negative value and it returned -EINVAL instead of

INT_MAX is not out of bound for this function. It is invalid value,
because OF code expects signed.
 
> -ENOENT. This patch add a check to of_fwnode_get_reference_args() to
> catch negative index before they are passed to the OF API and return
> -ENOENT right away.

I do not understand why are you fixing this issue that way. For this
API, the INT_MAX is correct value, but you claim that it is wrong and
should be ENOENT (even if there is entry).

Fine, if this is not a correct value, then EINVAL.

But more important I think this should be just fixed in different way -
why index in OF calls is signed in the first place? All indices are
supposed to be unsigned in general, because that is both logical and
readable when accessing arrays.

> 
> This issue appeared when the following pattern was used in the LED
> subsystem:
> 
>     index = fwnode_property_match_string(fwnode, "led-names", name)
>     led_node = fwnode_find_reference(fwnode, "leds", index);
> 
> Unlike the same pattern with the OF API, this pattern implicitly cast
> the signed return value of fwnode_property_match_string() to an
> unsigned index leading to the above issue with the OF backend. It can
> be argued that the return value of fwnode_property_match_string()
> should be checked separately, but I think there is value in supporting
> such simple and straight to the point patterns.
> 
> Link: https://lore.kernel.org/linux-leds/aimVRwJPhlGxsIUj@tom-desktop/T/#mc43cbf7e0599991b56dd0d9680714d28d145fbc8
> Cc: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> Signed-off-by: Alban Bedel <alban.bedel@xxxxxxxxxx>
> ---
>  drivers/of/property.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 136946f8b746f..eace2d1847b99 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1157,6 +1157,13 @@ of_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
>  	unsigned int i;
>  	int ret;
>  
> +	/* This function should return -ENOENT for out of bound indexes,

/*

Please use Linux coding style comments.

> +	 * but the OF API uses signed indexes and consider negative indexes
> +	 * as invalid. Catch them here to correctly implement the fwnode API.

Best regards,
Krzysztof