This implements the elf loader hook to parse RV specific
.riscv.attributes section. This section is inserted by compilers
(gcc/llvm) with build related information such as -march organized as
tag/value attribute pairs.
It identifies the various attribute tags (and corresponding values) as
currently specified in the psABI specification.
This patch only implements the elf parsing mechanics, leaving out the
recording/usage of the attributes to subsequent patches.
Signed-off-by: Vineet Gupta <[email protected]>
---
Changes since v1 [1]
- Handling potential oob accesses due to against malformed elf contents
- Handling of multiple sub-subsections
[1]https://lore.kernel.org/linux-riscv/[email protected]
Given the current state of discussions, the intended Vector extension
support would likely not need it, still posting the reworked v2 for
logical conclusion and for posterity in case need comes up in future
for something like CFI elf annotation.
Maintainers/reviewers can decide whether to merge it.
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/elf.h | 11 ++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/elf-attr.c | 232 +++++++++++++++++++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 arch/riscv/kernel/elf-attr.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e2b656043abf..f7e0ab05a2d2 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -12,6 +12,7 @@ config 32BIT
config RISCV
def_bool y
+ select ARCH_BINFMT_ELF_STATE
select ARCH_CLOCKSOURCE_INIT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index e7acffdf21d2..7ab8bd0ec330 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -116,6 +116,17 @@ do { \
*(struct user_regs_struct *)regs; \
} while (0);
+struct arch_elf_state {
+};
+
+#define INIT_ARCH_ELF_STATE {}
+
+extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state);
+
+extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
+ struct arch_elf_state *state);
+
#ifdef CONFIG_COMPAT
#define SET_PERSONALITY(ex) \
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 4cf303a779ab..eff6d845ac9d 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
obj-y += stacktrace.o
obj-y += cacheinfo.o
obj-y += patch.o
+obj-y += elf-attr.o
obj-y += probes/
obj-$(CONFIG_MMU) += vdso.o vdso/
diff --git a/arch/riscv/kernel/elf-attr.c b/arch/riscv/kernel/elf-attr.c
new file mode 100644
index 000000000000..51bcee92dd5b
--- /dev/null
+++ b/arch/riscv/kernel/elf-attr.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+#include <linux/binfmts.h>
+#include <linux/elf.h>
+#include <asm/unaligned.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "rv-elf-attr: " fmt
+
+#define PT_RISCV_ATTRIBUTES 0x70000003
+
+#define RV_ATTR_TAG_file 1
+
+#define RV_ATTR_TAG_stack_align 4
+#define RV_ATTR_TAG_arch 5
+#define RV_ATTR_TAG_unaligned_access 6
+
+#define RV_ATTR_VENDOR_RISCV "riscv"
+
+#define RV_ATTR_SEC_SZ SZ_1K
+
+static void rv_elf_attr_int(u32 tag, u32 val)
+{
+ pr_debug("Tag %x=%d\n", tag, val);
+}
+
+static void rv_elf_attr_str(u32 tag, const unsigned char *str)
+{
+ pr_debug("Tag %x=[%s]\n", tag, str);
+}
+
+/*
+ * Decode a ule128 encoded value.
+ */
+static int
+decode_uleb128_safe(unsigned char **dpp, u32 *val, const unsigned char *p_end)
+{
+ unsigned char *bp = *dpp;
+ unsigned char byte;
+ unsigned int shift = 0;
+ u32 result = 0;
+ int ok = 0;
+
+ while (bp < p_end) {
+ byte = *bp++;
+ result |= (byte & 0x7f) << shift;
+ if ((byte & 0x80) == 0) {
+ ok = 1;
+ break;
+ }
+ shift += 7;
+ }
+ if (!ok)
+ return -1;
+ *dpp = bp;
+ *val = result;
+ return 0;
+}
+
+/*
+ * Parse a single elf attribute.
+ */
+static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
+{
+ unsigned char *p = *dpp;
+ unsigned char *str;
+ u32 tag, val, s_len;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_attr;
+
+ switch (tag) {
+ case RV_ATTR_TAG_stack_align:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ rv_elf_attr_int(tag, val);
+ break;
+
+ case RV_ATTR_TAG_unaligned_access:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ rv_elf_attr_int(tag, val);
+ break;
+
+ case RV_ATTR_TAG_arch:
+ str = p;
+ s_len = strnlen(p, p_end - p) + 1;
+ p += s_len;
+ if (p > p_end)
+ goto bad_attr;
+ rv_elf_attr_str(tag, str);
+ break;
+
+ default:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ pr_debug("skipping Unrecognized Tag [%u]=%u\n", tag, val);
+ break;
+ }
+
+ *dpp = p;
+ return 0;
+bad_attr:
+ return -ENOEXEC;
+}
+
+/*
+ * Parse .riscv.attributes elf section.
+ */
+static int rv_parse_elf_attributes(struct file *f, const struct elf_phdr *phdr,
+ struct arch_elf_state *state)
+{
+ unsigned char buf[RV_ATTR_SEC_SZ];
+ unsigned char *p, *p_end;
+ ssize_t n;
+ int ret = 0;
+ loff_t pos;
+
+ pr_debug("Section .riscv.attributes found\n");
+
+ /* Assume a reasonable size for now */
+ if (phdr->p_filesz > sizeof(buf))
+ goto bad_elf;
+
+ memset(buf, 0, RV_ATTR_SEC_SZ);
+ pos = phdr->p_offset;
+ n = kernel_read(f, &buf, phdr->p_filesz, &pos);
+
+ if (n < 0)
+ return -EIO;
+
+ p = buf;
+ p_end = p + n;
+
+ /* sanity check format-version */
+ if (*p++ != 'A')
+ goto bad_elf;
+
+ /*
+ * elf attribute section organized as Vendor sub-sections(s)
+ * {sub-section length, vendor name, vendor data}
+ * Vendor data organized as sub-subsection(s)
+ * {tag, sub-subsection length, attributes contents}
+ * Attribute contents organized as
+ * {tag, value} pair(s).
+ */
+ while ((p_end - p) >= 4) {
+ int sub_len, vname_len;
+
+ sub_len = get_unaligned_le32(p);
+ if (sub_len <= 4 || sub_len > n)
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= 4;
+
+ /* Vendor name string */
+ vname_len = strnlen(p, sub_len) + 1;
+ if (vname_len > sub_len)
+ goto bad_elf;
+
+ /* skip non-mandatory sub-section for now */
+ if (strncmp(p, RV_ATTR_VENDOR_RISCV, sub_len)) {
+ p += sub_len;
+ continue;
+ }
+
+ p += vname_len;
+ sub_len -= vname_len;
+
+ /* Vendor data: sub-subsections(s) */
+ while (sub_len > 0) {
+ u32 tag, content_len;
+ unsigned char *sub_end, *sub_start = p;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_elf;
+
+ if ((p_end - p) < 4)
+ goto bad_elf;
+
+ content_len = get_unaligned_le32(p);
+ if (content_len > sub_len)
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= content_len;
+ sub_end = sub_start + content_len;
+
+ /* For now handle attributes relating to whole file */
+ if (tag != RV_ATTR_TAG_file) {
+ p = sub_end;
+ continue;
+ }
+
+ /* Attribute(s): tag:value pairs */
+ while (p < sub_end) {
+ ret = rv_parse_elf_attr_safe(&p, p_end);
+ if (ret)
+ goto bad_elf;
+ }
+ }
+ }
+
+ return ret;
+bad_elf:
+ return -ENOEXEC;
+}
+
+/*
+ * Hook invoked by generic elf loader to parse riscv specific elf segments.
+ */
+int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state)
+{
+ struct elf_phdr *phdr = _phdr;
+ int ret = 0;
+
+ if (phdr->p_type == PT_RISCV_ATTRIBUTES && !is_interp)
+ ret = rv_parse_elf_attributes(elf, phdr, state);
+
+ return ret;
+}
+
+int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
--
2.34.1
On 12 Jan 2023, at 21:06, Vineet Gupta <[email protected]> wrote:
>
> This implements the elf loader hook to parse RV specific
> .riscv.attributes section. This section is inserted by compilers
> (gcc/llvm) with build related information such as -march organized as
> tag/value attribute pairs.
>
> It identifies the various attribute tags (and corresponding values) as
> currently specified in the psABI specification.
>
> This patch only implements the elf parsing mechanics, leaving out the
> recording/usage of the attributes to subsequent patches.
>
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
> Changes since v1 [1]
> - Handling potential oob accesses due to against malformed elf contents
> - Handling of multiple sub-subsections
>
> [1]https://lore.kernel.org/linux-riscv/[email protected]
>
> Given the current state of discussions, the intended Vector extension
> support would likely not need it, still posting the reworked v2 for
> logical conclusion and for posterity in case need comes up in future
> for something like CFI elf annotation.
> Maintainers/reviewers can decide whether to merge it.
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/elf.h | 11 ++
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/elf-attr.c | 232 +++++++++++++++++++++++++++++++++++
> 4 files changed, 245 insertions(+)
> create mode 100644 arch/riscv/kernel/elf-attr.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e2b656043abf..f7e0ab05a2d2 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -12,6 +12,7 @@ config 32BIT
>
> config RISCV
> def_bool y
> + select ARCH_BINFMT_ELF_STATE
> select ARCH_CLOCKSOURCE_INIT
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> index e7acffdf21d2..7ab8bd0ec330 100644
> --- a/arch/riscv/include/asm/elf.h
> +++ b/arch/riscv/include/asm/elf.h
> @@ -116,6 +116,17 @@ do { \
> *(struct user_regs_struct *)regs; \
> } while (0);
>
> +struct arch_elf_state {
> +};
> +
> +#define INIT_ARCH_ELF_STATE {}
> +
> +extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
> + bool is_interp, struct arch_elf_state *state);
> +
> +extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
> + struct arch_elf_state *state);
> +
> #ifdef CONFIG_COMPAT
>
> #define SET_PERSONALITY(ex) \
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 4cf303a779ab..eff6d845ac9d 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
> obj-y += stacktrace.o
> obj-y += cacheinfo.o
> obj-y += patch.o
> +obj-y += elf-attr.o
> obj-y += probes/
> obj-$(CONFIG_MMU) += vdso.o vdso/
>
> diff --git a/arch/riscv/kernel/elf-attr.c b/arch/riscv/kernel/elf-attr.c
> new file mode 100644
> index 000000000000..51bcee92dd5b
> --- /dev/null
> +++ b/arch/riscv/kernel/elf-attr.c
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Rivos Inc.
> + */
> +
> +#include <linux/binfmts.h>
> +#include <linux/elf.h>
> +#include <asm/unaligned.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "rv-elf-attr: " fmt
> +
> +#define PT_RISCV_ATTRIBUTES 0x70000003
> +
> +#define RV_ATTR_TAG_file 1
> +
> +#define RV_ATTR_TAG_stack_align 4
> +#define RV_ATTR_TAG_arch 5
> +#define RV_ATTR_TAG_unaligned_access 6
> +
> +#define RV_ATTR_VENDOR_RISCV "riscv"
> +
> +#define RV_ATTR_SEC_SZ SZ_1K
> +
> +static void rv_elf_attr_int(u32 tag, u32 val)
> +{
> + pr_debug("Tag %x=%d\n", tag, val);
> +}
> +
> +static void rv_elf_attr_str(u32 tag, const unsigned char *str)
> +{
> + pr_debug("Tag %x=[%s]\n", tag, str);
> +}
> +
> +/*
> + * Decode a ule128 encoded value.
> + */
> +static int
> +decode_uleb128_safe(unsigned char **dpp, u32 *val, const unsigned char *p_end)
> +{
> + unsigned char *bp = *dpp;
> + unsigned char byte;
> + unsigned int shift = 0;
> + u32 result = 0;
> + int ok = 0;
> +
> + while (bp < p_end) {
> + byte = *bp++;
> + result |= (byte & 0x7f) << shift;
> + if ((byte & 0x80) == 0) {
> + ok = 1;
> + break;
Why not just do the return here?
> + }
> + shift += 7;
> + }
> + if (!ok)
> + return -1;
> + *dpp = bp;
> + *val = result;
> + return 0;
> +}
> +
> +/*
> + * Parse a single elf attribute.
> + */
> +static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
> +{
> + unsigned char *p = *dpp;
> + unsigned char *str;
> + u32 tag, val, s_len;
> +
> + if (decode_uleb128_safe(&p, &tag, p_end))
> + goto bad_attr;
> +
> + switch (tag) {
> + case RV_ATTR_TAG_stack_align:
> + if (decode_uleb128_safe(&p, &val, p_end))
> + goto bad_attr;
> + rv_elf_attr_int(tag, val);
> + break;
> +
> + case RV_ATTR_TAG_unaligned_access:
> + if (decode_uleb128_safe(&p, &val, p_end))
> + goto bad_attr;
> + rv_elf_attr_int(tag, val);
> + break;
> +
> + case RV_ATTR_TAG_arch:
> + str = p;
> + s_len = strnlen(p, p_end - p) + 1;
> + p += s_len;
> + if (p > p_end)
Constructing such a p is UB, check s_len before instead.
> + goto bad_attr;
> + rv_elf_attr_str(tag, str);
> + break;
> +
> + default:
> + if (decode_uleb128_safe(&p, &val, p_end))
From the ratified spec:
"RISC-V attributes have a string value if the tag number is odd and an integer value if the tag number is even."
> + goto bad_attr;
> + pr_debug("skipping Unrecognized Tag [%u]=%u\n", tag, val);
> + break;
> + }
> +
> + *dpp = p;
> + return 0;
> +bad_attr:
> + return -ENOEXEC;
> +}
> +
> +/*
> + * Parse .riscv.attributes elf section.
> + */
> +static int rv_parse_elf_attributes(struct file *f, const struct elf_phdr *phdr,
> + struct arch_elf_state *state)
> +{
> + unsigned char buf[RV_ATTR_SEC_SZ];
> + unsigned char *p, *p_end;
> + ssize_t n;
> + int ret = 0;
> + loff_t pos;
> +
> + pr_debug("Section .riscv.attributes found\n");
> +
> + /* Assume a reasonable size for now */
> + if (phdr->p_filesz > sizeof(buf))
> + goto bad_elf;
> +
> + memset(buf, 0, RV_ATTR_SEC_SZ);
This will hide bugs from sanitisers...
> + pos = phdr->p_offset;
> + n = kernel_read(f, &buf, phdr->p_filesz, &pos);
> +
> + if (n < 0)
> + return -EIO;
> +
> + p = buf;
> + p_end = p + n;
> +
> + /* sanity check format-version */
> + if (*p++ != 'A')
What if n is 0?
> + goto bad_elf;
> +
> + /*
> + * elf attribute section organized as Vendor sub-sections(s)
> + * {sub-section length, vendor name, vendor data}
> + * Vendor data organized as sub-subsection(s)
> + * {tag, sub-subsection length, attributes contents}
> + * Attribute contents organized as
> + * {tag, value} pair(s).
> + */
> + while ((p_end - p) >= 4) {
> + int sub_len, vname_len;
u32?
> +
> + sub_len = get_unaligned_le32(p);
> + if (sub_len <= 4 || sub_len > n)
n is the total amount read in, not the remaining amount.
> + goto bad_elf;
> +
> + p += 4;
> + sub_len -= 4;
> +
> + /* Vendor name string */
> + vname_len = strnlen(p, sub_len) + 1;
> + if (vname_len > sub_len)
> + goto bad_elf;
> +
> + /* skip non-mandatory sub-section for now */
> + if (strncmp(p, RV_ATTR_VENDOR_RISCV, sub_len)) {
> + p += sub_len;
> + continue;
> + }
> +
> + p += vname_len;
> + sub_len -= vname_len;
> +
> + /* Vendor data: sub-subsections(s) */
> + while (sub_len > 0) {
> + u32 tag, content_len;
> + unsigned char *sub_end, *sub_start = p;
Confusing naming for sub-subsection variables.
> +
> + if (decode_uleb128_safe(&p, &tag, p_end))
> + goto bad_elf;
> +
> + if ((p_end - p) < 4)
> + goto bad_elf;
> +
> + content_len = get_unaligned_le32(p);
> + if (content_len > sub_len)
> + goto bad_elf;
> +
> + p += 4;
> + sub_len -= content_len;
> + sub_end = sub_start + content_len;
> +
> + /* For now handle attributes relating to whole file */
> + if (tag != RV_ATTR_TAG_file) {
> + p = sub_end;
> + continue;
> + }
> +
> + /* Attribute(s): tag:value pairs */
> + while (p < sub_end) {
> + ret = rv_parse_elf_attr_safe(&p, p_end);
> + if (ret)
> + goto bad_elf;
> + }
> + }
> + }
> +
> + return ret;
> +bad_elf:
> + return -ENOEXEC;
> +}
> +
> +/*
> + * Hook invoked by generic elf loader to parse riscv specific elf segments.
> + */
> +int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
> + bool is_interp, struct arch_elf_state *state)
> +{
> + struct elf_phdr *phdr = _phdr;
> + int ret = 0;
> +
> + if (phdr->p_type == PT_RISCV_ATTRIBUTES && !is_interp)
> + ret = rv_parse_elf_attributes(elf, phdr, state);
> +
> + return ret;
> +}
> +
> +int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
> + struct arch_elf_state *state)
> +{
> + return 0;
> +}
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On 1/12/23 14:38, Jessica Clarke wrote:
> On 12 Jan 2023, at 21:06, Vineet Gupta<[email protected]> wrote:
>> +static int
>> +decode_uleb128_safe(unsigned char **dpp, u32 *val, const unsigned char *p_end)
>> +{
>> + unsigned char *bp = *dpp;
>> + unsigned char byte;
>> + unsigned int shift = 0;
>> + u32 result = 0;
Ved commented off-list about u32 being wide enough. So I'll be making
@val u64 everywhere.
>> + int ok = 0;
>> +
>> + while (bp < p_end) {
>> + byte = *bp++;
>> + result |= (byte & 0x7f) << shift;
>> + if ((byte & 0x80) == 0) {
>> + ok = 1;
>> + break;
> Why not just do the return here?
I guess I could.
>> +
>> + case RV_ATTR_TAG_arch:
>> + str = p;
>> + s_len = strnlen(p, p_end - p) + 1;
>> + p += s_len;
>> + if (p > p_end)
> Constructing such a p is UB, check s_len before instead.
OK.
>> + goto bad_attr;
>> + rv_elf_attr_str(tag, str);
>> + break;
>> +
>> + default:
>> + if (decode_uleb128_safe(&p, &val, p_end))
> From the ratified spec:
>
> "RISC-V attributes have a string value if the tag number is odd and an integer value if the tag number is even."
OK, added sanity checks.
>
>> +
>> + memset(buf, 0, RV_ATTR_SEC_SZ);
> This will hide bugs from sanitisers...
And if kernel_read() fills it partially, leave the rest uninitialized ?
I'll keep the memset.
>> + pos = phdr->p_offset;
>> + n = kernel_read(f, &buf, phdr->p_filesz, &pos);
>> +
>> + if (n < 0)
>> + return -EIO;
>> +
>> + p = buf;
>> + p_end = p + n;
>> +
>> + /* sanity check format-version */
>> + if (*p++ != 'A')
> What if n is 0?
I can check for n <= 0 above.
>> + goto bad_elf;
>> +
>> + /*
>> + * elf attribute section organized as Vendor sub-sections(s)
>> + * {sub-section length, vendor name, vendor data}
>> + * Vendor data organized as sub-subsection(s)
>> + * {tag, sub-subsection length, attributes contents}
>> + * Attribute contents organized as
>> + * {tag, value} pair(s).
>> + */
>> + while ((p_end - p) >= 4) {
>> + int sub_len, vname_len;
> u32?
OK.
>> +
>> + sub_len = get_unaligned_le32(p);
>> + if (sub_len <= 4 || sub_len > n)
> n is the total amount read in, not the remaining amount.
Fixed to sub_len > (p_end - p)
>
>> +
>> + /* Vendor data: sub-subsections(s) */
>> + while (sub_len > 0) {
>> + u32 tag, content_len;
>> + unsigned char *sub_end, *sub_start = p;
> Confusing naming for sub-subsection variables.
Fair enough: p_ss_start, p_ss_end, ss_len
Thx.
-Vineet
This implements the elf loader hook to parse RV specific
.riscv.attributes section. This section is inserted by compilers
(gcc/llvm) with build related information such as -march organized as
tag/value attribute pairs.
It identifies the various attribute tags (and corresponding values) as
currently specified in the psABI specification.
This patch only implements the elf parsing mechanics, leaving out the
recording/usage of the attributes to subsequent patches.
Signed-off-by: Vineet Gupta <[email protected]>
---
Changes since v2 [2]
- Address Jessica's review comments.
Mostly robustify code some more, checking for end of buffer etc.
Changes since v1 [1]
- Handling potential oob accesses due to against malformed elf contents
- Handling of multiple sub-subsections
[1]https://lore.kernel.org/linux-riscv/[email protected]
[2]https://lore.kernel.org/linux-riscv/[email protected]
Given the current state of discussions, the intended Vector extension
support would likely not need it, still posting the reworked v3 for
logical conclusion and for posterity in case need comes up in future
for something like CFI elf annotation.
Maintainers/reviewers can decide whether to merge it.
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/elf.h | 11 ++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/elf-attr.c | 239 +++++++++++++++++++++++++++++++++++
4 files changed, 252 insertions(+)
create mode 100644 arch/riscv/kernel/elf-attr.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e2b656043abf..f7e0ab05a2d2 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -12,6 +12,7 @@ config 32BIT
config RISCV
def_bool y
+ select ARCH_BINFMT_ELF_STATE
select ARCH_CLOCKSOURCE_INIT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index e7acffdf21d2..7ab8bd0ec330 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -116,6 +116,17 @@ do { \
*(struct user_regs_struct *)regs; \
} while (0);
+struct arch_elf_state {
+};
+
+#define INIT_ARCH_ELF_STATE {}
+
+extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state);
+
+extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
+ struct arch_elf_state *state);
+
#ifdef CONFIG_COMPAT
#define SET_PERSONALITY(ex) \
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 4cf303a779ab..eff6d845ac9d 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
obj-y += stacktrace.o
obj-y += cacheinfo.o
obj-y += patch.o
+obj-y += elf-attr.o
obj-y += probes/
obj-$(CONFIG_MMU) += vdso.o vdso/
diff --git a/arch/riscv/kernel/elf-attr.c b/arch/riscv/kernel/elf-attr.c
new file mode 100644
index 000000000000..8df8f2eea42b
--- /dev/null
+++ b/arch/riscv/kernel/elf-attr.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+#include <linux/binfmts.h>
+#include <linux/elf.h>
+#include <asm/unaligned.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "rv-elf-attr: " fmt
+
+#define PT_RISCV_ATTRIBUTES 0x70000003
+
+#define RV_ATTR_TAG_file 1
+
+#define RV_ATTR_TAG_stack_align 4
+#define RV_ATTR_TAG_arch 5
+#define RV_ATTR_TAG_unaligned_access 6
+
+#define RV_ATTR_VENDOR_RISCV "riscv"
+
+#define RV_ATTR_SEC_SZ SZ_1K
+
+static void rv_elf_attr_int(u64 tag, u64 val)
+{
+ pr_debug("Tag %llx=%llx\n", tag, val);
+}
+
+static void rv_elf_attr_str(u64 tag, const unsigned char *str)
+{
+ pr_debug("Tag %llx=[%s]\n", tag, str);
+}
+
+/*
+ * Decode a ule128 encoded value.
+ */
+static int
+decode_uleb128_safe(unsigned char **dpp, u64 *val, const unsigned char *p_end)
+{
+ unsigned char *bp = *dpp;
+ unsigned char byte;
+ unsigned int shift = 0;
+ u64 result = 0;
+
+ while (bp < p_end) {
+ byte = *bp++;
+ result |= (byte & 0x7f) << shift;
+ if ((byte & 0x80) == 0) {
+ *dpp = bp;
+ *val = result;
+ return 0;
+ }
+ shift += 7;
+ }
+
+ return -1;
+}
+
+#define ELF_ATTR_TAG_EVEN(t) (((t) % 2) ? false : true)
+
+/*
+ * Parse a single elf attribute.
+ */
+static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
+{
+ unsigned char *p = *dpp;
+ unsigned char *str;
+ u64 tag, val;
+ u32 s_len;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_attr;
+
+ switch (tag) {
+ case RV_ATTR_TAG_stack_align:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ if (!ELF_ATTR_TAG_EVEN(tag))
+ goto bad_attr;
+ rv_elf_attr_int(tag, val);
+ break;
+
+ case RV_ATTR_TAG_unaligned_access:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ if (!ELF_ATTR_TAG_EVEN(tag))
+ goto bad_attr;
+ rv_elf_attr_int(tag, val);
+ break;
+
+ case RV_ATTR_TAG_arch:
+ if (ELF_ATTR_TAG_EVEN(tag))
+ goto bad_attr;
+ str = p;
+ s_len = strnlen(p, p_end - p) + 1;
+ if ((p + s_len) > p_end)
+ goto bad_attr;
+ p += s_len;
+ rv_elf_attr_str(tag, str);
+ break;
+
+ default:
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ pr_debug("skipping Unrecognized Tag [%llx]=%llx\n", tag, val);
+ break;
+ }
+
+ *dpp = p;
+ return 0;
+bad_attr:
+ return -ENOEXEC;
+}
+
+/*
+ * Parse .riscv.attributes elf section.
+ */
+static int rv_parse_elf_attributes(struct file *f, const struct elf_phdr *phdr,
+ struct arch_elf_state *state)
+{
+ unsigned char buf[RV_ATTR_SEC_SZ];
+ unsigned char *p, *p_end;
+ ssize_t n;
+ int ret = 0;
+ loff_t pos;
+
+ pr_debug("Section .riscv.attributes found\n");
+
+ /* Assume a reasonable size for now */
+ if (phdr->p_filesz > sizeof(buf))
+ goto bad_elf;
+
+ memset(buf, 0, RV_ATTR_SEC_SZ);
+ pos = phdr->p_offset;
+ n = kernel_read(f, &buf, phdr->p_filesz, &pos);
+
+ if (n <= 0)
+ return -EIO;
+
+ p = buf;
+ p_end = p + n;
+
+ /* sanity check format-version */
+ if (*p++ != 'A')
+ goto bad_elf;
+
+ /*
+ * elf attribute section organized as Vendor sub-sections(s)
+ * {sub-section length, vendor name, vendor data}
+ * Vendor data organized as sub-subsection(s)
+ * {tag, sub-subsection length, attributes contents}
+ * Attribute contents organized as
+ * {tag, value} pair(s).
+ */
+ while ((p_end - p) >= 4) {
+ u32 sub_len, vname_len;
+
+ sub_len = get_unaligned_le32(p);
+ if (sub_len <= 4 || sub_len > (p_end - p))
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= 4;
+
+ /* Vendor name string */
+ vname_len = strnlen(p, sub_len) + 1;
+ if (vname_len > sub_len)
+ goto bad_elf;
+
+ /* skip non-mandatory sub-section for now */
+ if (strncmp(p, RV_ATTR_VENDOR_RISCV, sub_len)) {
+ p += sub_len;
+ continue;
+ }
+
+ p += vname_len;
+ sub_len -= vname_len;
+
+ /* Vendor data: sub-subsections(s) */
+ while (sub_len > 0) {
+ unsigned char *p_ss_end, *p_ss_start = p;
+ u32 ss_len;
+ u64 tag;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_elf;
+
+ if ((p_end - p) < 4)
+ goto bad_elf;
+
+ ss_len = get_unaligned_le32(p);
+ if (ss_len > sub_len)
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= ss_len;
+ p_ss_end = p_ss_start + ss_len;
+
+ /* For now handle attributes relating to whole file */
+ if (tag != RV_ATTR_TAG_file) {
+ p = p_ss_end;
+ continue;
+ }
+
+ /* Attribute(s): tag:value pairs */
+ while (p < p_ss_end) {
+ ret = rv_parse_elf_attr_safe(&p, p_end);
+ if (ret)
+ goto bad_elf;
+ }
+ }
+ }
+
+ return ret;
+bad_elf:
+ return -ENOEXEC;
+}
+
+/*
+ * Hook invoked by generic elf loader to parse riscv specific elf segments.
+ */
+int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state)
+{
+ struct elf_phdr *phdr = _phdr;
+ int ret = 0;
+
+ if (phdr->p_type == PT_RISCV_ATTRIBUTES && !is_interp)
+ ret = rv_parse_elf_attributes(elf, phdr, state);
+
+ return ret;
+}
+
+int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
--
2.34.1
On 19 Jan 2023, at 17:43, Vineet Gupta <[email protected]> wrote:
>
> This implements the elf loader hook to parse RV specific
> .riscv.attributes section. This section is inserted by compilers
> (gcc/llvm) with build related information such as -march organized as
> tag/value attribute pairs.
>
> It identifies the various attribute tags (and corresponding values) as
> currently specified in the psABI specification.
>
> This patch only implements the elf parsing mechanics, leaving out the
> recording/usage of the attributes to subsequent patches.
>
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
> Changes since v2 [2]
> - Address Jessica's review comments.
> Mostly robustify code some more, checking for end of buffer etc.
>
> Changes since v1 [1]
> - Handling potential oob accesses due to against malformed elf contents
> - Handling of multiple sub-subsections
>
> [1]https://lore.kernel.org/linux-riscv/[email protected]
> [2]https://lore.kernel.org/linux-riscv/[email protected]
>
> Given the current state of discussions, the intended Vector extension
> support would likely not need it, still posting the reworked v3 for
> logical conclusion and for posterity in case need comes up in future
> for something like CFI elf annotation.
> Maintainers/reviewers can decide whether to merge it.
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/elf.h | 11 ++
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/elf-attr.c | 239 +++++++++++++++++++++++++++++++++++
> 4 files changed, 252 insertions(+)
> create mode 100644 arch/riscv/kernel/elf-attr.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e2b656043abf..f7e0ab05a2d2 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -12,6 +12,7 @@ config 32BIT
>
> config RISCV
> def_bool y
> + select ARCH_BINFMT_ELF_STATE
> select ARCH_CLOCKSOURCE_INIT
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> index e7acffdf21d2..7ab8bd0ec330 100644
> --- a/arch/riscv/include/asm/elf.h
> +++ b/arch/riscv/include/asm/elf.h
> @@ -116,6 +116,17 @@ do { \
> *(struct user_regs_struct *)regs; \
> } while (0);
>
> +struct arch_elf_state {
> +};
> +
> +#define INIT_ARCH_ELF_STATE {}
> +
> +extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
> + bool is_interp, struct arch_elf_state *state);
> +
> +extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
> + struct arch_elf_state *state);
> +
> #ifdef CONFIG_COMPAT
>
> #define SET_PERSONALITY(ex) \
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 4cf303a779ab..eff6d845ac9d 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
> obj-y += stacktrace.o
> obj-y += cacheinfo.o
> obj-y += patch.o
> +obj-y += elf-attr.o
> obj-y += probes/
> obj-$(CONFIG_MMU) += vdso.o vdso/
>
> diff --git a/arch/riscv/kernel/elf-attr.c b/arch/riscv/kernel/elf-attr.c
> new file mode 100644
> index 000000000000..8df8f2eea42b
> --- /dev/null
> +++ b/arch/riscv/kernel/elf-attr.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Rivos Inc.
> + */
> +
> +#include <linux/binfmts.h>
> +#include <linux/elf.h>
> +#include <asm/unaligned.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "rv-elf-attr: " fmt
> +
> +#define PT_RISCV_ATTRIBUTES 0x70000003
> +
> +#define RV_ATTR_TAG_file 1
> +
> +#define RV_ATTR_TAG_stack_align 4
> +#define RV_ATTR_TAG_arch 5
> +#define RV_ATTR_TAG_unaligned_access 6
> +
> +#define RV_ATTR_VENDOR_RISCV "riscv"
> +
> +#define RV_ATTR_SEC_SZ SZ_1K
> +
> +static void rv_elf_attr_int(u64 tag, u64 val)
> +{
> + pr_debug("Tag %llx=%llx\n", tag, val);
> +}
> +
> +static void rv_elf_attr_str(u64 tag, const unsigned char *str)
> +{
> + pr_debug("Tag %llx=[%s]\n", tag, str);
> +}
> +
> +/*
> + * Decode a ule128 encoded value.
> + */
> +static int
> +decode_uleb128_safe(unsigned char **dpp, u64 *val, const unsigned char *p_end)
> +{
> + unsigned char *bp = *dpp;
> + unsigned char byte;
> + unsigned int shift = 0;
> + u64 result = 0;
> +
> + while (bp < p_end) {
> + byte = *bp++;
> + result |= (byte & 0x7f) << shift;
> + if ((byte & 0x80) == 0) {
> + *dpp = bp;
> + *val = result;
> + return 0;
> + }
> + shift += 7;
> + }
> +
> + return -1;
> +}
> +
> +#define ELF_ATTR_TAG_EVEN(t) (((t) % 2) ? false : true)
I don’t think you need a macro for "unsigned integer is even".
> +
> +/*
> + * Parse a single elf attribute.
> + */
> +static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
> +{
> + unsigned char *p = *dpp;
> + unsigned char *str;
> + u64 tag, val;
> + u32 s_len;
> +
> + if (decode_uleb128_safe(&p, &tag, p_end))
> + goto bad_attr;
> +
> + switch (tag) {
> + case RV_ATTR_TAG_stack_align:
> + if (decode_uleb128_safe(&p, &val, p_end))
> + goto bad_attr;
> + if (!ELF_ATTR_TAG_EVEN(tag))
> + goto bad_attr;
Huh? You just checked it against a constant so you know exactly what it
is. This is just Static_assert(RV_ATTR_TAG_stack_align % 2 == 0) but at
run time. And you know that’s going to be the case, because the spec is
self-consistent by design (wouldn’t be a worthwhile spec otherwise).
> + rv_elf_attr_int(tag, val);
> + break;
> +
> + case RV_ATTR_TAG_unaligned_access:
> + if (decode_uleb128_safe(&p, &val, p_end))
> + goto bad_attr;
> + if (!ELF_ATTR_TAG_EVEN(tag))
> + goto bad_attr;
> + rv_elf_attr_int(tag, val);
> + break;
> +
> + case RV_ATTR_TAG_arch:
> + if (ELF_ATTR_TAG_EVEN(tag))
> + goto bad_attr;
> + str = p;
> + s_len = strnlen(p, p_end - p) + 1;
> + if ((p + s_len) > p_end)
> + goto bad_attr;
> + p += s_len;
> + rv_elf_attr_str(tag, str);
> + break;
> +
> + default:
The whole point of the even/odd split is so that when you *don’t* know
what the tag means you can still decode its value and thus know how to
skip past it. That is, *here* is where you need to be checking
even/odd, and deciding whether to treat it as a string or a ULEB128,
which is why I annotated *here* not one of the other case labels before.
> + if (decode_uleb128_safe(&p, &val, p_end))
> + goto bad_attr;
> + pr_debug("skipping Unrecognized Tag [%llx]=%llx\n", tag, val);
> + break;
> + }
> +
> + *dpp = p;
> + return 0;
> +bad_attr:
> + return -ENOEXEC;
> +}
> +
> +/*
> + * Parse .riscv.attributes elf section.
> + */
> +static int rv_parse_elf_attributes(struct file *f, const struct elf_phdr *phdr,
> + struct arch_elf_state *state)
> +{
> + unsigned char buf[RV_ATTR_SEC_SZ];
> + unsigned char *p, *p_end;
> + ssize_t n;
> + int ret = 0;
> + loff_t pos;
> +
> + pr_debug("Section .riscv.attributes found\n");
> +
> + /* Assume a reasonable size for now */
> + if (phdr->p_filesz > sizeof(buf))
> + goto bad_elf;
> +
> + memset(buf, 0, RV_ATTR_SEC_SZ);
> + pos = phdr->p_offset;
> + n = kernel_read(f, &buf, phdr->p_filesz, &pos);
> +
> + if (n <= 0)
> + return -EIO;
0 should be ENOEXEC not EIO? And surely in the < 0 case you want to be
forwarding on the exact error from kernel_read, not squashing it into
EIO?
> +
> + p = buf;
> + p_end = p + n;
> +
> + /* sanity check format-version */
> + if (*p++ != 'A')
> + goto bad_elf;
> +
> + /*
> + * elf attribute section organized as Vendor sub-sections(s)
> + * {sub-section length, vendor name, vendor data}
> + * Vendor data organized as sub-subsection(s)
> + * {tag, sub-subsection length, attributes contents}
> + * Attribute contents organized as
> + * {tag, value} pair(s).
> + */
> + while ((p_end - p) >= 4) {
> + u32 sub_len, vname_len;
> +
> + sub_len = get_unaligned_le32(p);
> + if (sub_len <= 4 || sub_len > (p_end - p))
> + goto bad_elf;
> +
> + p += 4;
> + sub_len -= 4;
> +
> + /* Vendor name string */
> + vname_len = strnlen(p, sub_len) + 1;
> + if (vname_len > sub_len)
> + goto bad_elf;
> +
> + /* skip non-mandatory sub-section for now */
> + if (strncmp(p, RV_ATTR_VENDOR_RISCV, sub_len)) {
> + p += sub_len;
> + continue;
> + }
> +
> + p += vname_len;
> + sub_len -= vname_len;
> +
> + /* Vendor data: sub-subsections(s) */
> + while (sub_len > 0) {
> + unsigned char *p_ss_end, *p_ss_start = p;
> + u32 ss_len;
> + u64 tag;
> +
> + if (decode_uleb128_safe(&p, &tag, p_end))
> + goto bad_elf;
> +
> + if ((p_end - p) < 4)
> + goto bad_elf;
> +
> + ss_len = get_unaligned_le32(p);
> + if (ss_len > sub_len)
> + goto bad_elf;
> +
> + p += 4;
> + sub_len -= ss_len;
> + p_ss_end = p_ss_start + ss_len;
> +
> + /* For now handle attributes relating to whole file */
> + if (tag != RV_ATTR_TAG_file) {
> + p = p_ss_end;
> + continue;
> + }
> +
> + /* Attribute(s): tag:value pairs */
> + while (p < p_ss_end) {
> + ret = rv_parse_elf_attr_safe(&p, p_end);
> + if (ret)
> + goto bad_elf;
> + }
> + }
> + }
> +
> + return ret;
> +bad_elf:
> + return -ENOEXEC;
> +}
> +
> +/*
> + * Hook invoked by generic elf loader to parse riscv specific elf segments.
> + */
> +int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
> + bool is_interp, struct arch_elf_state *state)
> +{
> + struct elf_phdr *phdr = _phdr;
> + int ret = 0;
> +
> + if (phdr->p_type == PT_RISCV_ATTRIBUTES && !is_interp)
Both the executable and its interpreter matter.
Jess
> + ret = rv_parse_elf_attributes(elf, phdr, state);
> +
> + return ret;
> +}
> +
> +int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
> + struct arch_elf_state *state)
> +{
> + return 0;
> +}
> --
> 2.34.1
>
On 1/19/23 12:33, Jessica Clarke wrote:
>> +
>> +/*
>> + * Parse a single elf attribute.
>> + */
>> +static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
>> +{
>> + unsigned char *p = *dpp;
>> + unsigned char *str;
>> + u64 tag, val;
>> + u32 s_len;
>> +
>> + if (decode_uleb128_safe(&p, &tag, p_end))
>> + goto bad_attr;
>> +
>> + switch (tag) {
>> + case RV_ATTR_TAG_stack_align:
>> + if (decode_uleb128_safe(&p, &val, p_end))
>> + goto bad_attr;
>> + if (!ELF_ATTR_TAG_EVEN(tag))
>> + goto bad_attr;
> Huh? You just checked it against a constant so you know exactly what it
> is. This is just Static_assert(RV_ATTR_TAG_stack_align % 2 == 0) but at
> run time. And you know that’s going to be the case, because the spec is
> self-consistent by design (wouldn’t be a worthwhile spec otherwise).
Makes sense.
>> + rv_elf_attr_int(tag, val);
>> + break;
>> +
>> + case RV_ATTR_TAG_unaligned_access:
>> + if (decode_uleb128_safe(&p, &val, p_end))
>> + goto bad_attr;
>> + if (!ELF_ATTR_TAG_EVEN(tag))
>> + goto bad_attr;
>> + rv_elf_attr_int(tag, val);
>> + break;
>> +
>> + case RV_ATTR_TAG_arch:
>> + if (ELF_ATTR_TAG_EVEN(tag))
>> + goto bad_attr;
>> + str = p;
>> + s_len = strnlen(p, p_end - p) + 1;
>> + if ((p + s_len) > p_end)
>> + goto bad_attr;
>> + p += s_len;
>> + rv_elf_attr_str(tag, str);
>> + break;
>> +
>> + default:
> The whole point of the even/odd split is so that when you *don’t* know
> what the tag means you can still decode its value and thus know how to
> skip past it. That is, *here* is where you need to be checking
> even/odd, and deciding whether to treat it as a string or a ULEB128,
I see the point. We can ignore the specific tags and just treat odd and
even tags as string and int respectively.
And keep a loose check of the known tags vs. unknown.
> which is why I annotated *here* not one of the other case labels before.
OK. I really need to pay attention to what and where to your comments :-)
>
>> + memset(buf, 0, RV_ATTR_SEC_SZ);
>> + pos = phdr->p_offset;
>> + n = kernel_read(f, &buf, phdr->p_filesz, &pos);
>> +
>> + if (n <= 0)
>> + return -EIO;
> 0 should be ENOEXEC not EIO? And surely in the < 0 case you want to be
> forwarding on the exact error from kernel_read, not squashing it into
> EIO?
Right.
>
>> +/*
>> + * Hook invoked by generic elf loader to parse riscv specific elf segments.
>> + */
>> +int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
>> + bool is_interp, struct arch_elf_state *state)
>> +{
>> + struct elf_phdr *phdr = _phdr;
>> + int ret = 0;
>> +
>> + if (phdr->p_type == PT_RISCV_ATTRIBUTES && !is_interp)
> Both the executable and its interpreter matter.
OK.
Thx,
-Vineet
This implements the elf loader hook to parse RV specific
.riscv.attributes section. This section is inserted by compilers
(gcc/llvm) with build related information such as -march organized as
tag/value attribute pairs.
It identifies the various attribute tags (and corresponding values) as
currently specified in the psABI specification.
This patch only implements the elf parsing mechanics, leaving out the
recording/usage of the attributes to subsequent patches.
Signed-off-by: Vineet Gupta <[email protected]>
---
Changes since v3 [3]
- Address more review comments from Jessica.
Specifically handle unknown tags better knowing they can only be
int/string.
Changes since v2 [2]
- Address Jessica's review comments.
Mostly robustify code some more, checking for end of buffer etc.
Changes since v1 [1]
- Handling potential oob accesses against malformed elf contents.
- Handling of multiple sub-subsections
[1]https://lore.kernel.org/linux-riscv/[email protected]
[2]https://lore.kernel.org/linux-riscv/[email protected]
[3]https://lore.kernel.org/linux-riscv/[email protected]
Given the current state of discussions, the intended Vector extension
support would likely not need it, still posting the reworked code for
logical conclusion and for posterity in case need comes up in future
for something like CFI elf annotation.
Maintainers/reviewers can decide whether to merge it.
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/elf.h | 11 ++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/elf-attr.c | 225 +++++++++++++++++++++++++++++++++++
4 files changed, 238 insertions(+)
create mode 100644 arch/riscv/kernel/elf-attr.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e2b656043abf..f7e0ab05a2d2 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -12,6 +12,7 @@ config 32BIT
config RISCV
def_bool y
+ select ARCH_BINFMT_ELF_STATE
select ARCH_CLOCKSOURCE_INIT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index e7acffdf21d2..7ab8bd0ec330 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -116,6 +116,17 @@ do { \
*(struct user_regs_struct *)regs; \
} while (0);
+struct arch_elf_state {
+};
+
+#define INIT_ARCH_ELF_STATE {}
+
+extern int arch_elf_pt_proc(void *ehdr, void *phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state);
+
+extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr,
+ struct arch_elf_state *state);
+
#ifdef CONFIG_COMPAT
#define SET_PERSONALITY(ex) \
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 4cf303a779ab..eff6d845ac9d 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
obj-y += stacktrace.o
obj-y += cacheinfo.o
obj-y += patch.o
+obj-y += elf-attr.o
obj-y += probes/
obj-$(CONFIG_MMU) += vdso.o vdso/
diff --git a/arch/riscv/kernel/elf-attr.c b/arch/riscv/kernel/elf-attr.c
new file mode 100644
index 000000000000..ac5df800516e
--- /dev/null
+++ b/arch/riscv/kernel/elf-attr.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+#include <linux/binfmts.h>
+#include <linux/elf.h>
+#include <asm/unaligned.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "rv-elf-attr: " fmt
+
+#define PT_RISCV_ATTRIBUTES 0x70000003
+
+#define RV_ATTR_TAG_file 1
+
+#define RV_ATTR_TAG_stack_align 4
+#define RV_ATTR_TAG_arch 5
+#define RV_ATTR_TAG_unaligned_access 6
+
+#define RV_ATTR_VENDOR_RISCV "riscv"
+
+#define RV_ATTR_SEC_SZ SZ_1K
+
+static void rv_elf_attr_int(u64 tag, u64 val)
+{
+ if (tag == RV_ATTR_TAG_stack_align ||
+ tag == RV_ATTR_TAG_unaligned_access)
+ pr_debug("Tag %llx=%llx\n", tag, val);
+ else
+ pr_debug("Unrecognized int Tag [%llx]=%llx\n", tag, val);
+}
+
+static void rv_elf_attr_str(u64 tag, const unsigned char *str)
+{
+ if (tag == RV_ATTR_TAG_arch)
+ pr_debug("Tag %llx=[%s]\n", tag, str);
+ else
+ pr_debug("Unrecognized string Tag [%llx]=[%s]\n", tag, str);
+}
+
+/*
+ * Decode a ule128 encoded value.
+ */
+static int
+decode_uleb128_safe(unsigned char **dpp, u64 *val, const unsigned char *p_end)
+{
+ unsigned char *bp = *dpp;
+ unsigned char byte;
+ unsigned int shift = 0;
+ u64 result = 0;
+
+ while (bp < p_end) {
+ byte = *bp++;
+ result |= (byte & 0x7f) << shift;
+ if ((byte & 0x80) == 0) {
+ *dpp = bp;
+ *val = result;
+ return 0;
+ }
+ shift += 7;
+ }
+
+ return -1;
+}
+
+/*
+ * Parse a single elf attribute.
+ */
+static int rv_parse_elf_attr_safe(unsigned char **dpp, unsigned char *p_end)
+{
+ unsigned char *p = *dpp;
+ unsigned char *str;
+ u64 tag, val;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_attr;
+
+ if (tag % 2) {
+ u32 s_len;
+
+ str = p;
+ s_len = strnlen(p, p_end - p) + 1;
+ if ((p + s_len) > p_end)
+ goto bad_attr;
+ p += s_len;
+ rv_elf_attr_str(tag, str);
+ } else {
+ if (decode_uleb128_safe(&p, &val, p_end))
+ goto bad_attr;
+ rv_elf_attr_int(tag, val);
+ }
+
+ *dpp = p;
+ return 0;
+bad_attr:
+ return -ENOEXEC;
+}
+
+/*
+ * Parse .riscv.attributes elf section.
+ */
+static int rv_parse_elf_attributes(struct file *f, const struct elf_phdr *phdr,
+ struct arch_elf_state *state)
+{
+ unsigned char buf[RV_ATTR_SEC_SZ];
+ unsigned char *p, *p_end;
+ ssize_t n;
+ int ret = 0;
+ loff_t pos;
+
+ pr_debug("Section .riscv.attributes found\n");
+
+ /* Assume a reasonable size for now */
+ if (phdr->p_filesz > sizeof(buf))
+ goto bad_elf;
+
+ memset(buf, 0, RV_ATTR_SEC_SZ);
+ pos = phdr->p_offset;
+ n = kernel_read(f, &buf, phdr->p_filesz, &pos);
+
+ if (n < 0)
+ return n;
+ else if (n == 0)
+ return -ENOEXEC;
+
+ p = buf;
+ p_end = p + n;
+
+ /* sanity check format-version */
+ if (*p++ != 'A')
+ goto bad_elf;
+
+ /*
+ * elf attribute section organized as Vendor sub-sections(s)
+ * {sub-section length, vendor name, vendor data}
+ * Vendor data organized as sub-subsection(s)
+ * {tag, sub-subsection length, attributes contents}
+ * Attribute contents organized as
+ * {tag, value} pair(s).
+ */
+ while ((p_end - p) >= 4) {
+ u32 sub_len, vname_len;
+
+ sub_len = get_unaligned_le32(p);
+ if (sub_len <= 4 || sub_len > (p_end - p))
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= 4;
+
+ /* Vendor name string */
+ vname_len = strnlen(p, sub_len) + 1;
+ if (vname_len > sub_len)
+ goto bad_elf;
+
+ /* skip non-mandatory sub-section for now */
+ if (strncmp(p, RV_ATTR_VENDOR_RISCV, sub_len)) {
+ p += sub_len;
+ continue;
+ }
+
+ p += vname_len;
+ sub_len -= vname_len;
+
+ /* Vendor data: sub-subsections(s) */
+ while (sub_len > 0) {
+ unsigned char *p_ss_end, *p_ss_start = p;
+ u32 ss_len;
+ u64 tag;
+
+ if (decode_uleb128_safe(&p, &tag, p_end))
+ goto bad_elf;
+
+ if ((p_end - p) < 4)
+ goto bad_elf;
+
+ ss_len = get_unaligned_le32(p);
+ if (ss_len > sub_len)
+ goto bad_elf;
+
+ p += 4;
+ sub_len -= ss_len;
+ p_ss_end = p_ss_start + ss_len;
+
+ /* For now handle attributes relating to whole file */
+ if (tag != RV_ATTR_TAG_file) {
+ p = p_ss_end;
+ continue;
+ }
+
+ /* Attribute(s): tag:value pairs */
+ while (p < p_ss_end) {
+ ret = rv_parse_elf_attr_safe(&p, p_end);
+ if (ret)
+ goto bad_elf;
+ }
+ }
+ }
+
+ return ret;
+bad_elf:
+ return -ENOEXEC;
+}
+
+/*
+ * Hook invoked by generic elf loader to parse riscv specific elf segments.
+ */
+int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
+ bool is_interp, struct arch_elf_state *state)
+{
+ struct elf_phdr *phdr = _phdr;
+ int ret = 0;
+
+ if (phdr->p_type == PT_RISCV_ATTRIBUTES)
+ ret = rv_parse_elf_attributes(elf, phdr, state);
+
+ return ret;
+}
+
+int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
--
2.34.1