On Thu, 21 May 2026 16:25:28 +0200
Jens Remus <[email protected]> wrote:
> +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;
> +
> + if (copy_from_user(&shdr, (void __user *)sec->sframe_start,
> sizeof(shdr))) {
> + dbg("header usercopy failed\n");
> + return -EFAULT;
> + }
> +
> + if (shdr.preamble.magic != SFRAME_MAGIC ||
> + shdr.preamble.version != SFRAME_VERSION_3 ||
> + !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) ||
> + !(shdr.preamble.flags & SFRAME_F_FDE_FUNC_START_PCREL) ||
> + shdr.auxhdr_len) {
> + dbg("bad/unsupported sframe header\n");
> + return -EINVAL;
> + }
> +
> + if (!shdr.num_fdes || !shdr.num_fres) {
> + dbg("no fde/fre entries\n");
> + return -EINVAL;
> + }
> +
> + 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;
> +
> + if (fres_start < fdes_end || fres_end > sec->sframe_end) {
> + dbg("inconsistent fde/fre offsets\n");
> + return -EINVAL;
> + }
I really think we should also test if fres_start >= fres_end. Because
if fres_start is near sec->sframe_end and on 32bit archs fre_len makes
fres_end overflow, this doesn't catch it.
Even if it doesn't cause issues later, I rather have it caught here.
-- Steve
> +
> + sec->num_fdes = num_fdes;
> + sec->fdes_start = fdes_start;
> + sec->fres_start = fres_start;
> + sec->fres_end = fres_end;
> +
> + sec->ra_off = shdr.cfa_fixed_ra_offset;
> + sec->fp_off = shdr.cfa_fixed_fp_offset;
> +
> + return 0;
> +}
