Re: [PATCH] clocksource/drivers/timer-sp804: validate DT compatible before using it as a name

[Daniel Lezcano]

Hi Pengpeng,

On 4/3/26 08:55, Pengpeng Hou wrote:
> `sp804_of_init()` and `integrator_cp_of_init()` fetch raw
> `"compatible"` bytes with `of_get_property()` and then pass the returned
> pointer onward as the timer name.
>
> That name is used in later clock lookup and clocksource / clockevent
> registration paths, so malformed unterminated live-tree properties can
> trigger out-of-bounds reads before timer initialization completes.
>
> Validate the compatible string before using it as a C string.

If the function is called it is because there is a match with the 
compatible string. It is pointless to check if it is valid because the 
location of the executing code is under the condition the compatible exists.


> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
>
> ---
>   drivers/clocksource/timer-sp804.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
> index d69858427359..8164ceefee06 100644
> --- a/drivers/clocksource/timer-sp804.c
> +++ b/drivers/clocksource/timer-sp804.c
> @@ -284,7 +284,11 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
>   	int irq, ret = -EINVAL;
>   	u32 irq_num = 0;
>   	struct clk *clk1, *clk2;
> -	const char *name = of_get_property(np, "compatible", NULL);
> +	const char *name;
> +
> +	ret = of_property_read_string(np, "compatible", &name);
> +	if (ret)
> +		return ret;
>   
>   	if (initialized) {
>   		pr_debug("%pOF: skipping further SP804 timer device\n", np);
> @@ -371,9 +375,13 @@ static int __init integrator_cp_of_init(struct device_node *np)
>   	static int init_count = 0;
>   	void __iomem *base;
>   	int irq, ret = -EINVAL;
> -	const char *name = of_get_property(np, "compatible", NULL);
> +	const char *name;
>   	struct clk *clk;
>   
> +	ret = of_property_read_string(np, "compatible", &name);
> +	if (ret)
> +		return ret;
> +
>   	base = of_iomap(np, 0);
>   	if (!base) {
>   		pr_err("Failed to iomap\n");