Re: [PATCH] libfdt: fdt_path_offset_namelen: Reject empty path

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Oct 06, 2023 at 01:48:39PM +0100, Pierre-Clément Tosi wrote:
> Make empty paths result in FDT_ERR_BADPATH.
> 
> Per the specification (v0.4-rc4):
> 
> > The convention for specifying a device path is:
> >     /node-name-1/node-name-2/node-name-N
> >
> > The path to the root node is /.
> >
> > A unit address may be omitted if the full path to the
> > node is unambiguous.

As Rob noted, I don't really see how this quote is relevant to the
change at hand.

The change itself looks like a good idea, though.  Without this, we
will at the very least do a one byte bad access in the next line.  If
someone does path a negative value it will do... something bad,
probably.

> Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
> ---
>  libfdt/fdt_ro.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index c4c520c..46b4ef5 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -255,6 +255,9 @@ int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
>  
>  	FDT_RO_PROBE(fdt);
>  
> +	if (namelen < 1)
> +		return -FDT_ERR_BADPATH;

It would be better to make this:
	if (!can_assume(VALID_INPUT) && namelen <= 0)

This allows the test to be optimised out in builds where we can assume
always valid parameters.

>  	/* see if we have an alias */
>  	if (*path != '/') {
>  		const char *q = memchr(path, '/', end - p);

It would also be really nice to add a testcase for behaviour in the
namelen == 0 and namelen < 0 cases.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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