Re: [PATCH v15 02/20] unwind_user/sframe: Add support for reading .sframe headers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in SFrame section offset and size calculations allows malformed headers to bypass bounds checks on 32-bit architectures.
--

commit 2144f643835a311bd7c4b9df269057ea5516231a
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Add support for reading .sframe headers

This commit adds basic compile infrastructure and support for reading
the .sframe section header in preparation for unwinding user space stacks.
The sframe_add_section() function currently reads the header and
unconditionally returns an error until subsequent patches improve it.

Link: https://lore.kernel.org/all/f27e8463783febfa0dabb0432a3dd6be8ad98412.1737511963.git.jpoimboe@kernel.org/

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> new file mode 100644
> index 0000000000000..d24e9d4f8bef7
> --- /dev/null
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -0,0 +1,136 @@
> +static int sframe_read_header(struct sframe_section *sec)
> +{
> +	unsigned long header_end, fdes_start, fdes_end, fres_start, fres_end;
> +	struct sframe_header shdr;
> +	unsigned int num_fdes;

[ ... ]

> +	header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr);
> +	if (header_end >= sec->sframe_end) {
> +		dbg("header doesn't fit in section\n");
> +		return -EINVAL;
> +	}
> +
> +	num_fdes   = shdr.num_fdes;
> +	fdes_start = header_end + shdr.fdes_off;
> +	fdes_end   = fdes_start + (num_fdes * sizeof(struct sframe_fde_v3));
> +
> +	fres_start = header_end + shdr.fres_off;
> +	fres_end   = fres_start + shdr.fre_len;

[Severity: Medium]
On 32-bit architectures, is it possible for these offset and length
calculations to wrap around?

User-supplied 32-bit fields from the SFrame header (fdes_off, num_fdes,
fres_off, fre_len) are used to compute section boundaries using raw
arithmetic. Since unsigned long is 32 bits on those architectures,
operations like header_end + shdr.fdes_off or num_fdes * sizeof(...)
could overflow.

If a malformed header contains values that wrap the 32-bit address space,
fres_end might appear smaller than sec->sframe_end and fres_start might
appear greater than fdes_end, completely bypassing the validation check
that follows.

Would it be safer to use the <linux/overflow.h> helpers such as
size_mul() and check_add_overflow() to protect these arithmetic
operations?

> +
> +	if (fres_start < fdes_end || fres_end > sec->sframe_end) {
> +		dbg("inconsistent fde/fre offsets\n");
> +		return -EINVAL;
> +	}

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