Re: [PATCH v16 12/20] unwind_user/sframe: Add .sframe validation option

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `.sframe` validation loop in `sframe_validate_section` lacks `cond_resched()`, allowing maliciously large or crafted `.sframe` sections to cause CPU soft lockups.
--

commit 134a5da326c64a7bfc6e8145805fff7efe4a4cc1
Author: Josh Poimboeuf <jpoimboe@kernel.org>
unwind_user/sframe: Add .sframe validation option

This commit introduces a debug feature to validate all .sframe sections
when first loading the file rather than on demand. The feature reads the
entire .sframe section for every loaded executable and shared library.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 2cfa274cd8dcf..e6d66ae8e7ac6 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -384,6 +384,94 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>  	goto end;
>  }
>  
> +#ifdef CONFIG_SFRAME_VALIDATION

[ ... ]

> +static int sframe_validate_section(struct sframe_section *sec)
> +{
> +	struct sframe_fde_internal fde;
> +	unsigned long prev_func_addr;
> +	unsigned int i;
> +
> +	for (i = 0; i < sec->num_fdes; i++) {
> +		struct sframe_fre_internal fre;
> +		unsigned long fre_addr;
> +		u32 prev_ip_off;
> +		unsigned int j;
> +		int ret;
> +
> +		ret = safe_read_fde(sec, i, &fde);
> +		if (ret) {
> +			dbg_sec("safe_read_fde(%u) failed\n", i);
> +			return ret;
> +		}
> +
> +		if (i && fde.func_addr <= prev_func_addr) {
> +			dbg_sec("FDE %u not sorted\n", i);
> +			return -EINVAL;
> +		}
> +		prev_func_addr = fde.func_addr;
> +
> +		fre_addr = sec->fres_start + fde.fres_off;
> +		for (j = 0; j < fde.fres_num; j++) {
> +			ret = safe_read_fre(sec, &fde, fre_addr, &fre);

[Severity: High]
Could a large or crafted .sframe section cause a CPU soft lockup here?

Since sec->num_fdes and fde.fres_num are bounded only by the mapped user
memory region size, a .sframe section could be crafted with millions of FDEs
pointing to the same valid array of FREs.

Without a cond_resched() or fatal_signal_pending() check in these nested
loops, iterating over these entries could monopolize the CPU in kernel mode
without yielding, potentially triggering the soft lockup watchdog.

> +			if (ret) {
> +				dbg_sec("FDE %u: safe_read_fre(%u) failed\n", i, j);
> +				dbg_sec("FDE: func_addr:%#lx func_size:%#x fda_off:%#x fres_off:%#x fres_num:%u info:%u info2:%u rep_size:%u\n",
> +					fde.func_addr, fde.func_size,
> +					fde.fda_off,
> +					fde.fres_off, fde.fres_num,
> +					fde.info, fde.info2,
> +					fde.rep_size);
> +				return ret;
> +			}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521142546.3908498-1-jremus@linux.ibm.com?part=12