Re: [PATCH v2 2/2] software node: Fix software_node_get_reference_args() with index -1

[Alban Bedel <alban.bedel@lht.dlh.de>]

On Sat, 27 Jun 2026 07:50:12 +0800
Zijun Hu <zijun.hu@oss.qualcomm.com> wrote:

> On 6/18/2026 11:20 PM, Alban Bedel wrote:
> > The bounds check for the index passed to
> > software_node_get_reference_args() was failing when passed UINT_MAX,
> > this in turn would lead to an out of bound access in the property
> > array. Fix the bound check to also cover the UINT_MAX case.
> > 
> > Fixes: 31e4e12e0e960 ("software node: Correct a OOB check in software_node_get_reference_args()")  
> 
> i think the fix tag may not be right.
> for original express before the fix tag: if (index * sizeof(*ref) > prop->length)
> for UINT_MAX, multiplication overflow?

You are right both the orignal code and the version introduced in
31e4e12e0e960 are subject to multiplication overflow issues. I added the
fix tag because 31e4e12e0e960 claim to fix an issue but fall short. If
someone backport this commit it might be good if they can find that
it has an issue.

But if that's not an appropriate use of the fix tag I can remove it.

> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/linux-devicetree/20260611103904.7CB131F00893@smtp.kernel.org/
> > Signed-off-by: Alban Bedel <alban.bedel@lht.dlh.de>
> > --
> > v2: No changes. Only submit this patch along with the patch that
> >     triggered the Sashiko report, to hopefully avoid another useless
> >     report.
> > ---
> >  drivers/base/swnode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> > index 869228a65cb36..2bc76f01eb77d 100644
> > --- a/drivers/base/swnode.c
> > +++ b/drivers/base/swnode.c
> > @@ -537,7 +537,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
> >  	if (prop->is_inline)
> >  		return -EINVAL;
> >  
> > -	if ((index + 1) * sizeof(*ref) > prop->length)
> > +	if (index >= prop->length / sizeof(*ref))
> >  		return -ENOENT;
> >    
> 
> who will use UINT_MAX ?

A likely scenario is if a caller pass a negative errno, for example the
return value of fwnode_property_match_string(). But as you pointed out
there is also the matter of the multiplication overflow, so various
values smaller than UINT_MAX can also lead to a failure.

> This function is a interface function.  the best fix should check
> input parameter @index and return -EINVAL if it is not expected?

The index is unsigned, so there is no invalid values, just out of
bounds. The documentation of fwnode_property_get_reference_args() says:

 * Return: %0 on success
 *	    %-ENOENT when the index is out of bounds, the index has an empty
 *		     reference or the property was not found
 *	    %-EINVAL on parse error
 *	    %-ENOTCONN when the remote firmware node exists but has not been
 *		       registered yet

So this should return -ENOENT.

Alban