On Tue, Jan 21, 2025 at 6:32 PM Josh Poimboeuf <jpoim...@kernel.org> wrote: > > In preparation for unwinding user space stacks with sframe, add basic > sframe compile infrastructure and support for reading the .sframe > section header. > > sframe_add_section() reads the header and unconditionally returns an > error, so it's not very useful yet. A subsequent patch will improve > that. > > Signed-off-by: Josh Poimboeuf <jpoim...@kernel.org> > --- > arch/Kconfig | 3 + > include/linux/sframe.h | 36 +++++++++++ > kernel/unwind/Makefile | 3 +- > kernel/unwind/sframe.c | 136 +++++++++++++++++++++++++++++++++++++++++ > kernel/unwind/sframe.h | 71 +++++++++++++++++++++ > 5 files changed, 248 insertions(+), 1 deletion(-) > create mode 100644 include/linux/sframe.h > create mode 100644 kernel/unwind/sframe.c > create mode 100644 kernel/unwind/sframe.h >
[...] > + > +extern int sframe_add_section(unsigned long sframe_start, unsigned long > sframe_end, > + unsigned long text_start, unsigned long > text_end); > +extern int sframe_remove_section(unsigned long sframe_addr); > + > +#else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */ > + > +static inline int sframe_add_section(unsigned long sframe_start, unsigned > long sframe_end, unsigned long text_start, unsigned long text_end) { return > -ENOSYS; } nit: very-very long, wrap it? > +static inline int sframe_remove_section(unsigned long sframe_addr) { return > -ENOSYS; } > + > +#endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */ > + > +#endif /* _LINUX_SFRAME_H */ [...] > +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_2 || > + !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) || probably more a question to Indu, but why is this sorting not mandatory and part of SFrame "standard"? How realistically non-sorted FDEs would work in practice? Ain't nobody got time to sort them just to unwind the stack... > + shdr.auxhdr_len) { > + dbg("bad/unsupported sframe header\n"); > + return -EINVAL; > + } > + > + if (!shdr.num_fdes || !shdr.num_fres) { given SFRAME_F_FRAME_POINTER in the header, is it really that nonsensical and illegal to have zero FDEs/FREs? Maybe we should allow that? > + dbg("no fde/fre entries\n"); > + return -EINVAL; > + } > + > + header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr); > + if (header_end >= sec->sframe_end) { if we allow zero FDEs/FREs, header_end == sec->sframe_end is legal, right? > + 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)); > + > + fres_start = header_end + shdr.fres_off; > + fres_end = fres_start + shdr.fre_len; > + maybe use check_add_overflow() in all the above calculation, at least on 32-bit arches this all can overflow and it's not clear if below sanity check detects all possible overflows > + if (fres_start < fdes_end || fres_end > sec->sframe_end) { > + dbg("inconsistent fde/fre offsets\n"); > + return -EINVAL; > + } > + > + 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; > +} > + [...] > diff --git a/kernel/unwind/sframe.h b/kernel/unwind/sframe.h > new file mode 100644 > index 000000000000..e9bfccfaf5b4 > --- /dev/null > +++ b/kernel/unwind/sframe.h > @@ -0,0 +1,71 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * From https://www.sourceware.org/binutils/docs/sframe-spec.html > + */ > +#ifndef _SFRAME_H > +#define _SFRAME_H > + > +#include <linux/types.h> > + > +#define SFRAME_VERSION_1 1 > +#define SFRAME_VERSION_2 2 > +#define SFRAME_MAGIC 0xdee2 > + > +#define SFRAME_F_FDE_SORTED 0x1 > +#define SFRAME_F_FRAME_POINTER 0x2 > + > +#define SFRAME_ABI_AARCH64_ENDIAN_BIG 1 > +#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE 2 > +#define SFRAME_ABI_AMD64_ENDIAN_LITTLE 3 > + > +#define SFRAME_FDE_TYPE_PCINC 0 > +#define SFRAME_FDE_TYPE_PCMASK 1 > + > +struct sframe_preamble { > + u16 magic; > + u8 version; > + u8 flags; > +} __packed; > + > +struct sframe_header { > + struct sframe_preamble preamble; > + u8 abi_arch; > + s8 cfa_fixed_fp_offset; > + s8 cfa_fixed_ra_offset; > + u8 auxhdr_len; > + u32 num_fdes; > + u32 num_fres; > + u32 fre_len; > + u32 fdes_off; > + u32 fres_off; > +} __packed; > + > +#define SFRAME_HEADER_SIZE(header) \ > + ((sizeof(struct sframe_header) + header.auxhdr_len)) > + > +#define SFRAME_AARCH64_PAUTH_KEY_A 0 > +#define SFRAME_AARCH64_PAUTH_KEY_B 1 > + > +struct sframe_fde { > + s32 start_addr; > + u32 func_size; > + u32 fres_off; > + u32 fres_num; > + u8 info; > + u8 rep_size; > + u16 padding; > +} __packed; I couldn't understand from SFrame itself, but why do sframe_header, sframe_preamble, and sframe_fde have to be marked __packed, if it's all naturally aligned (intentionally and by design)?.. > + > +#define SFRAME_FUNC_FRE_TYPE(data) (data & 0xf) > +#define SFRAME_FUNC_FDE_TYPE(data) ((data >> 4) & 0x1) > +#define SFRAME_FUNC_PAUTH_KEY(data) ((data >> 5) & 0x1) > + > +#define SFRAME_BASE_REG_FP 0 > +#define SFRAME_BASE_REG_SP 1 > + > +#define SFRAME_FRE_CFA_BASE_REG_ID(data) (data & 0x1) > +#define SFRAME_FRE_OFFSET_COUNT(data) ((data >> 1) & 0xf) > +#define SFRAME_FRE_OFFSET_SIZE(data) ((data >> 5) & 0x3) > +#define SFRAME_FRE_MANGLED_RA_P(data) ((data >> 7) & 0x1) > + > +#endif /* _SFRAME_H */ > -- > 2.48.1 >