[PATCH] arm64: pi: validate bootargs before parsing them

Pengpeng Hou posted 1 patch 2 months, 1 week ago
arch/arm64/kernel/pi/idreg-override.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] arm64: pi: validate bootargs before parsing them
Posted by Pengpeng Hou 2 months, 1 week ago
get_bootargs_cmdline() fetches the raw bootargs property from the FDT
and immediately calls strlen() on it before later passing the same
pointer into the early command-line parser. Flat DT properties are
external boot input, and this path does not prove that bootargs is
NUL-terminated within its declared bounds.

Use fdt_stringlist_get() so malformed unterminated bootargs are
rejected before the local parser treats them as C strings.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 arch/arm64/kernel/pi/idreg-override.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
index bc57b290e5e7..310ed279ef26 100644
--- a/arch/arm64/kernel/pi/idreg-override.c
+++ b/arch/arm64/kernel/pi/idreg-override.c
@@ -373,11 +373,11 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt, int node)
 	if (node < 0)
 		return NULL;
 
-	prop = fdt_getprop(fdt, node, bootargs, NULL);
+	prop = fdt_stringlist_get(fdt, node, bootargs, 0, NULL);
 	if (!prop)
 		return NULL;
 
-	return strlen(prop) ? prop : NULL;
+	return *prop ? prop : NULL;
 }
 
 static __init void parse_cmdline(const void *fdt, int chosen)
-- 
2.50.1 (Apple Git-155)
Re: [PATCH] arm64: pi: validate bootargs before parsing them
Posted by Pengpeng Hou 2 months, 1 week ago
Hi Will,

Thanks, that's a fair question.

The reason I cared here is not that we can make every malformed FDT survive
cleanly, but that this particular caller turns a raw property into an
unbounded C string immediately:

  fdt_getprop() -> strlen() -> __parse_cmdline()

If `bootargs` is not NUL-terminated within the property bounds, `strlen()`
can walk past the property before the parser even starts. By contrast, a
NUL-terminated but semantically bogus string stays within the property bounds
and is then just handled as bad/ignored command-line content.

So the issue I was trying to harden is specifically “raw FDT property becomes
a C string without a local bound check”, not malformed DT handling in the
broadest sense.

You're also right to ask about scope. I do not think every early
`fdt_getprop()` caller should be converted mechanically; the ones that matter
here are the callers that immediately feed raw properties into unbounded
string helpers or parsers. This arm64 PI bootargs path is one of those.

I will rework this around that boundary instead of pushing this one caller in
isolation. In other words, I will audit the same early-bootargs family of
callers and only convert the ones that have this direct
`raw property -> unbounded C-string helper/parser` shape.

Thanks,
Pengpeng


Re: [PATCH] arm64: pi: validate bootargs before parsing them
Posted by Will Deacon 2 months, 1 week ago
On Fri, Apr 03, 2026 at 11:56:05AM +0800, Pengpeng Hou wrote:
> get_bootargs_cmdline() fetches the raw bootargs property from the FDT
> and immediately calls strlen() on it before later passing the same
> pointer into the early command-line parser. Flat DT properties are
> external boot input, and this path does not prove that bootargs is
> NUL-terminated within its declared bounds.
> 
> Use fdt_stringlist_get() so malformed unterminated bootargs are
> rejected before the local parser treats them as C strings.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  arch/arm64/kernel/pi/idreg-override.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
> index bc57b290e5e7..310ed279ef26 100644
> --- a/arch/arm64/kernel/pi/idreg-override.c
> +++ b/arch/arm64/kernel/pi/idreg-override.c
> @@ -373,11 +373,11 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt, int node)
>  	if (node < 0)
>  		return NULL;
>  
> -	prop = fdt_getprop(fdt, node, bootargs, NULL);
> +	prop = fdt_stringlist_get(fdt, node, bootargs, 0, NULL);
>  	if (!prop)
>  		return NULL;
>  
> -	return strlen(prop) ? prop : NULL;
> +	return *prop ? prop : NULL;
>  }

I'm not exactly sure why we need to go out of our way to handle a
malformed DT at this stage, tbh. If it's corrupted in other ways (e.g.
random ASCII replacement) then we're probably still going to crash and
burn. There's nothing particularly special about NUL terminators.

If we _do_ decide that this is worth fixing, what about the other
early callers of fdt_getprop (kaslr, EFI stub)?

Will