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
>
