November 2022 - linux-morello - op-lists.linaro.org

[PATCH v3 0/3] Introduce VM_READ_CAPS and VM_WRITE_CAPS flags

by Tudor Cretu

This patch series addresses the VM_READ_CAPS/VM_WRITE_CAPS flags issue: https://git.morello-project.org/morello/kernel/linux/-/issues/36 io_uring system uses buffers shared with userspace to read the io events and report their results. The structs that populate the submission and completion queues can contain capabilities. Shared mappings don't have the Load/Store capabilities permission to avoid leaking capabilities outside their original address space, so add two new VM flags that would allow the kernel to set up such mappings. While at it, also fix pte_modify to allow setting PTE_*_CAPS flags, add new the new rc/wc smaps flags, and remove the automatic addition of PTE_*_CAPS to user mappings. To note: this wouldn't allow userspace to make arbitrary shared mappings with tag access, the new VM flags would be for internal use only for the time being. v3: - Improved documentation, comments, and commit message - Fixed condition in Patch 3, now tested properly with Morello GDB v2: - Removed Patch 1 from the series as it wasn't essential - Added docs to Documentation/filesystems/proc.rst - Removed VM_RW_CAPS - Moved definition of VM_*_CAPS just after the definition of VM_MTE - Added details for a TODO related to file-backed mappings - Introduced Patch 3 that removes an assumption about shared mappings Review branch: https://git.morello-project.org/tudcre01/linux/-/commits/vm_rw_caps_v3/ Thanks, Tudor Tudor Cretu (3): arm64: morello: Add VM_READ_CAPS and VM_WRITE_CAPS flags arm64: morello: Explicitly add VM_*_CAPS to private user mappings arm64: morello: Check against VM_WRITE_CAPS in access_remote_cap Documentation/filesystems/proc.rst | 2 ++ arch/arm64/Kconfig | 1 + arch/arm64/include/asm/mman.h | 26 ++++++++++++++++++++++++-- arch/arm64/include/asm/page.h | 3 ++- arch/arm64/include/asm/pgtable-prot.h | 12 +++++------- arch/arm64/kernel/morello.c | 7 +++---- fs/proc/task_mmu.c | 4 ++++ include/linux/mm.h | 8 ++++++++ 8 files changed, 49 insertions(+), 14 deletions(-) -- 2.25.1

2 years

2
5
0 0

[PATCH] net/compat: Don't pack structs in COMPAT64

by Teo Couprie Diaz

The __packed attribute for struct compat_group_req led to a failure of the LTP test accept02 when trying to join a multicast group. Keep it for COMPAT32 but remove it in COMPAT64, do the same for the other structs used here. Signed-off-by: Teo Couprie Diaz <teo.coupriediaz(a)arm.com> --- Not really sure this is the right way to go, nor if the others structs should be left alone. Chaning them all didn't lead to failures in setsockopt or accept LTP tests, which was where the issue was detected. include/net/compat.h | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/include/net/compat.h b/include/net/compat.h index f2d7423217d2..5cb091e2a6a6 100644 --- a/include/net/compat.h +++ b/include/net/compat.h @@ -68,7 +68,11 @@ struct compat_group_req { __u32 gr_interface; struct __kernel_sockaddr_storage gr_group __aligned(4); +#ifdef CONFIG_COMPAT32 } __packed; +#else +}; +#endif struct compat_group_source_req { __u32 gsr_interface; @@ -76,7 +80,11 @@ struct compat_group_source_req { __aligned(4); struct __kernel_sockaddr_storage gsr_source __aligned(4); +#ifdef CONFIG_COMPAT32 } __packed; +#else +}; +#endif struct compat_group_filter { union { @@ -88,7 +96,11 @@ struct compat_group_filter { __u32 gf_numsrc_aux; struct __kernel_sockaddr_storage gf_slist[1] __aligned(4); +#ifdef CONFIG_COMPAT32 } __packed; +#else + }; +#endif struct { __u32 gf_interface; struct __kernel_sockaddr_storage gf_group @@ -97,8 +109,16 @@ struct compat_group_filter { __u32 gf_numsrc; struct __kernel_sockaddr_storage gf_slist_flex[] __aligned(4); +#ifdef CONFIG_COMPAT32 } __packed; +#else + }; +#endif }; -} __packed; +#ifdef CONFIG_COMPAT32 + } __packed; +#else + }; +#endif #endif /* NET_COMPAT_H */ base-commit: 2fcc0af155a19755f68602f400fc845d2c9e6077 -- 2.25.1

2 years

3
8
0 0

[PATCH 0/3] fork: clone3: Capabilities support

by Beata Michalska

Hi All, This series adds capabilities support for clone3 syscall along with set of testcases in morello clone kselftests. Struct clone_args signature after proposed changes: __CHERI_PURE_CAPABILITY__ : struct clone_args { __u64 flags __attribute__((__aligned__(8))); /* 0 8 */ __u8 __rsvd_v0_1[8]; /* 8 8 */ __kernel_aligned_uintptr_t pidfd; /* 16 16 */ __kernel_aligned_uintptr_t child_tid; /* 32 16 */ __kernel_aligned_uintptr_t parent_tid; /* 48 16 */ /* --- cacheline 1 boundary (64 bytes) --- */ __u64 exit_signal __attribute__((__aligned__(8))); /* 64 8 */ __u8 __rsvd_v0_2[8]; /* 72 8 */ __kernel_aligned_uintptr_t stack; /* 80 16 */ __u64 stack_size __attribute__((__aligned__(8))); /* 96 8 */ __u8 __rsvd_v0_3[8]; /* 104 8 */ __kernel_aligned_uintptr_t tls; /* 112 16 */ /* --- cacheline 2 boundary (128 bytes) --- */ __kernel_aligned_uintptr_t set_tid; /* 128 16 */ __u64 set_tid_size __attribute__((__aligned__(8))); /* 144 8 */ __u64 cgroup __attribute__((__aligned__(8))); /* 152 8 */ /* size: 160, cachelines: 3, members: 14 */ /* forced alignments: 5 */ /* last cacheline: 32 bytes */ }; otherwise: struct clone_args { __u64 flags __attribute__((__aligned__(8))); /* 0 8 */ __u8 __rsvd_v0_1[0]; /* 8 0 */ __kernel_aligned_uintptr_t pidfd __attribute__((__aligned__(8))); /* 8 8 */ __kernel_aligned_uintptr_t child_tid __attribute__((__aligned__(8))); /* 16 8 */ __kernel_aligned_uintptr_t parent_tid __attribute__((__aligned__(8))); /* 24 8 */ __u64 exit_signal __attribute__((__aligned__(8))); /* 32 8 */ __u8 __rsvd_v0_2[0]; /* 40 0 */ __kernel_aligned_uintptr_t stack __attribute__((__aligned__(8))); /* 40 8 */ __u64 stack_size __attribute__((__aligned__(8))); /* 48 8 */ __u8 __rsvd_v0_3[0]; /* 56 0 */ __kernel_aligned_uintptr_t tls __attribute__((__aligned__(8))); /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ __kernel_aligned_uintptr_t set_tid __attribute__((__aligned__(8))); /* 64 8 */ __u64 set_tid_size __attribute__((__aligned__(8))); /* 72 8 */ __u64 cgroup __attribute__((__aligned__(8))); /* 80 8 */ /* size: 88, cachelines: 2, members: 14 */ /* forced alignments: 11 */ /* last cacheline: 24 bytes */ }; Changes available at: https://git.morello-project.org/Bea/linux/-/tree/morello/clone3 LTP changes: https://git.morello-project.org/Bea/morello-linux-ltp/-/tree/morello/clone3 To run clone3 tests: ./runltp -f syscalls -s clone3 Beata Michalska (3): uaccess: Preserve capability tags with copy_struct_from_user fork: clone3: Add support for architectural capabilities kselftests/arm64: morello: Add clone3 test-cases include/linux/uaccess.h | 2 +- include/uapi/linux/sched.h | 37 ++- kernel/fork.c | 123 ++++++-- tools/testing/selftests/arm64/morello/clone.c | 269 +++++++++++++++++- 4 files changed, 391 insertions(+), 40 deletions(-) -- 2.25.1

2 years

4
27
0 0

[RFC PATCH 0/9] update bpf syscall for PCuABI/compat64

by Zachary Leaf

Hi, This RFC proposes changes to the bpf syscall to support propagating user pointers as capabilities in the pure-capability kernel-user ABI (PCuABI). It also includes an approach to supporting the existing aarch64 ABI as a COMPAT layer (compat64). Since the bpf syscall is multiplexed this RFC changes only the BPF_PROG_LOAD option. The idea is to agree on the general approach and get some feedback here before the remaining options are changed to fully support the syscall. Since the changes are incomplete, functions in this series suffixed with _fixed will eventually replace the original function it is updating. The originals remain in place for now. Patches 8 and 9 provide two potential solutions to one problem. I don't know enough about LSMs to decide there so any input there appreciated. A basic LTP test that only does a bare minimal bpf(BPF_PROG_LOAD...) syscall has been created for this RFC to verify these changes. This also tests the new CHECK_ATTR macro implementation. Kernel branch available at: https://git.morello-project.org/zdleaf/linux/-/commits/rfc/bpf LTP test at: https://git.morello-project.org/zdleaf/morello-linux-test-project/-/commits… Thanks, Zach Zachary Leaf (9): arm64: morello: enable eBPF and tracing bpf/net: copy ptrs from user with bpf/sockptr_t bpf: extend bpfptr_t to use pointers bpf: use user pointer types for bpf_attr bpf: add compat64 handling of bpf syscall bpf: make CHECK_ATTR support compat temp for RFC: add padding to BPF_PROG_LOAD bpf/security: update bpf lsm API bpf/security: move lsm hook point .../morello_transitional_pcuabi_defconfig | 15 ++- include/linux/bpfptr.h | 19 ++++ include/linux/sockptr.h | 9 ++ include/uapi/linux/bpf.h | 55 +++++++++-- kernel/bpf/syscall.c | 95 ++++++++++++++++--- kernel/bpf/verifier.c | 10 +- 6 files changed, 176 insertions(+), 27 deletions(-) -- 2.34.1

2 years

4
39
0 0

[PATCH v2 0/3] Introduce VM_READ_CAPS and VM_WRITE_CAPS flags

by Tudor Cretu

Hi, This patch series addresses the VM_READ_CAPS/VM_WRITE_CAPS flags issue: https://git.morello-project.org/morello/kernel/linux/-/issues/36 io_uring system uses buffers shared with userspace to read the io events and report their results. The structs that populate the submission and completion queues can contain capabilities. Shared mappings don't have the Load/Store capabilities permission to avoid leaking capabilities outside their original address space, so add two new VM flags that would allow the kernel to set up such mappings. While at it, also fix pte_modify to allow setting PTE_*_CAPS flags, add new the new rc/wc smaps flags, and remove the automatic addition of PTE_*_CAPS to user mappings. To note: this wouldn't allow userspace to make arbitrary shared mappings with tag access, the new VM flags would be for internal use only for the time being. v2: - Removed Patch 1 from the series as it wasn't essential - Added docs to Documentation/filesystems/proc.rst - Removed VM_RW_CAPS - Moved definition of VM_*_CAPS just after the definition of VM_MTE - Added details for a TODO related to file-backed mappings - Introduced Patch 3 that removes an assumption about shared mappings Review branch: https://git.morello-project.org/tudcre01/linux/-/commits/vm_rw_caps_v2/ Thanks, Tudor Tudor Cretu (3): arm64: morello: Add VM_READ_CAPS and VM_WRITE_CAPS flags arm64: morello: Explicitly add VM_*_CAPS to private user mappings arm64: morello: Check against VM_WRITE_CAPS in access_remote_cap Documentation/filesystems/proc.rst | 2 ++ arch/arm64/Kconfig | 1 + arch/arm64/include/asm/mman.h | 26 ++++++++++++++++++++++++-- arch/arm64/include/asm/page.h | 3 ++- arch/arm64/include/asm/pgtable-prot.h | 12 +++++------- arch/arm64/kernel/morello.c | 7 +++---- fs/proc/task_mmu.c | 4 ++++ include/linux/mm.h | 8 ++++++++ 8 files changed, 49 insertions(+), 14 deletions(-) -- 2.25.1

2 years

2
6
0 0

[PATCH v4] lib/vsprintf: Add support for architectural capabilities

by Beata Michalska

Introduce new printk's pointer format extension to support capabilities, with one being denoted by specifying the 'l' length modifier alongside the 'p' type character => "%lp". In addition to the raw capability format, adding '#' flag will generate the simplified one, being slightly more verbose. In both cases, the actual result is subject to pointer hashing (address only) unless either 'x' qualifier or no_hash_pointers command line parameter is being provided. For more details see the docs, updated here as well. While at it, add some testcases to validate new format alongside checkpatchpl new warning, to raise the awareness of potential misuse. Signed-off-by: Beata Michalska <beata.michalska(a)arm.com> --- v4: - Docs: removing spurious new lines and adding missing 'dot'. - test: add incrementing total_tests and drop breaking on failure for hashed formats, plus fixing accounting for actual tests skipped - lib/vsprintf.c: - Drop the comment about potentially breaking capability into separate values corresponding to tag, metadata and value - moved length var declaration into if block as this is the right scope for it v3: - Docs: style/spelling changes - test: improved code layout (as per review comments), dropped stale SEAL permission, fixed accounting for skipped tests - lib/vsprintf.c: - switched from __cheri_fromcap to cheri_address_get - fixed indentations - vbin_printf: adjusted comment for storing caps in bin buffer - bstr_printf: added missing check for buffer end v2: - Rephrase/rearrange docs + adding link to the CHERI guidelines on printf formats (also dropping 'hybrid-mode' mentions) - Code readability/formatting improvements (as per review comments) - Adding '0x' prefix for relevant simplified format components - Removing missing permissions from the final output in simplified format - Use pointer_string directly when no hashing is expected - Added support for vbin_printf/bstr_printf (trace_printk is now able to print capabilities) Review branch available at: https://git.morello-project.org/Bea/linux/-/tree/morello/cap_printk_v4 --- Documentation/core-api/printk-formats.rst | 60 ++++++ lib/test_printf.c | 158 +++++++++++++++ lib/vsprintf.c | 231 +++++++++++++++++++++- scripts/checkpatch.pl | 13 +- 4 files changed, 458 insertions(+), 4 deletions(-) diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index 5e89497ba314..a4840ac9c001 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -202,6 +202,66 @@ Example:: printk("test: difference between pointers: %td\n", ptr2 - ptr1); +Capabilities +------------ + +:: + + %lp[x] 1:ffffc00000010005:0123456789abcdef + %#lp[x] 0x0123456789abcdef [rwxRWE,0x0000000000000000-0xffffffffffffffff] + +For printing CHERI architectural capabilities (when supported, otherwise +the result of using it is undefined). Formatting based on: +https://github.com/CTSRD-CHERI/cheri-c-programming/wiki/Displaying-Capabilities +with a small detour of dropping support for basic format in favour of the raw +one, with the assumption that the actual address can be printed with existing +printk formats (subject to appropriate casting when dealing with capabilities +and non-capability formats, which otherwise renders undefined behaviour). + +The ``l`` modifier distinguishes capability pointers from standard (integer) +pointers, giving the following capability format being printed:: + + Capability tag:High bits:Low bits + ------------------------------------ + <tag>:<127:64>:<63:0> + + +Specifying ``#`` modifier results in printing capabilities in a simplified format:: + + <address> [<permissions>,<base>-<top>] (<attr>) + +where:: + + - permissions - none or any combination of: + - 'r' : CHERI_PERM_LOAD + - 'w' : CHERI_PERM_STORE + - 'x' : CHERI_PERM_EXECUTE + - 'R' : CHERI_PERM_LOAD_CAP + - 'W' : CHERI_PERM_STORE_CAP + - 'E' : ARM_CAP_PERMISSION_EXECUTIVE + - base - lower bound + - top - upper bound + - attr - none or any combination of: + - 'invalid' : capability's tag is clear + - 'sentry' : capability is a sealed entry + - 'sealed' : capability is sealed with a type other than the + sealed entry object type + +The above applies to formats with either ``no_hash_pointers`` parameter being +enabled or ``x`` modifier being provided - otherwise subject to hashing +(address only, with remaining bits being disregarded). + +Refer to `Unmodified Addresses`_ for potential security implications. + +Currently only Morello architecture is being supported. + +Note: +An attempt to use this format with a non-capability type is undefined behaviour. +There are no safety nets, so extra caution is advised! Handle with care. +Same applies when capabilities are not supported, as the use of either the ``#`` +flag or the ``l`` length modifier is undefined behaviour when combined with +%p. + Struct Resources ---------------- diff --git a/lib/test_printf.c b/lib/test_printf.c index 07309c45f327..1b877b68a30d 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -24,6 +24,10 @@ #include <linux/property.h> +#ifdef __CHERI__ +#include <cheriintrin.h> +#endif + #include "../tools/testing/selftests/kselftest_module.h" #define BUF_SIZE 256 @@ -755,6 +759,159 @@ errptr(void) #endif } +static void __init +capability_pointer(void) +{ +#ifdef __CHERI__ + enum action { + /* No action - use as is */ + CAP_AXN_NONE, + /* Clear capability tag */ + CAP_TAG_CLEAR, + /* Derive null-capability */ + CAP_NULL_DERIVE + }; + + struct { + const char *plain_fmt; + const char *fmt; + const char *expected; + int action; + } const setup[] = { + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = "1:d800400059ab89ab:"PTR_STR, + .action = CAP_AXN_NONE, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab]", + .action = CAP_AXN_NONE, + }, + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = "0:d800400059ab89ab:"PTR_STR, + .action = CAP_TAG_CLEAR, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab] (invalid)", + .action = CAP_TAG_CLEAR, + }, + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = PTR_STR, + .action = CAP_NULL_DERIVE, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = PTR_STR, + .action = CAP_NULL_DERIVE, + } + }; + + char buf[PLAIN_BUF_SIZE]; + const char * const cap_fmt_non_hashed[] = {"%lp", "%#lp"}; + int nchars; + + void * __capability cap = cheri_ddc_get(); + + /* Expecting basic permissions to be set */ + size_t perms = CHERI_PERM_GLOBAL | CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP | + CHERI_PERM_STORE | CHERI_PERM_STORE_CAP; + + size_t bounds = 4096; + + cap = cheri_address_set(cap, (ptraddr_t)PTR); + cap = cheri_perms_and(cap, perms); + /* + * Basic checks so that the actual output can be safely compared + * to the expected one + */ + if (!cheri_tag_get(cap) || cheri_perms_get(cap) != perms + || cheri_is_sealed(cap)) { + pr_warn("Failed to create capability for testing - skipping"); + skipped_tests += no_hash_pointers ? + ARRAY_SIZE(cap_fmt_non_hashed) + ARRAY_SIZE(setup) * 2 : + ARRAY_SIZE(cap_fmt_non_hashed) + ARRAY_SIZE(setup); + return; + } + + cap = cheri_bounds_set_exact(cap, bounds); + + /* Verify hashing */ + if (no_hash_pointers) { + pr_warn("Skipping capability hashing tests\n"); + skipped_tests += ARRAY_SIZE(cap_fmt_non_hashed); + goto non_hashed; + } + + for (int i = 0; i < ARRAY_SIZE(cap_fmt_non_hashed); ++i) { + nchars = snprintf(buf, PLAIN_BUF_SIZE, cap_fmt_non_hashed[i], + cap); + + total_tests++; + /* + * Should be ADDR_WIDTH in this case , but hey, let's not be picky + * about it, at least not here ... not yet ... + */ + if (nchars != PTR_WIDTH) { + /* + * This also covers the case when the value has not been + * actually hashed, as nchars would then be greater than + * PTR_WIDTH + */ + pr_warn("Something went wrong with hashing capability value\n"); + failed_tests++; + continue; + } + + if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { + pr_warn("crng possibly not yet initialized - capability value buffer contains \"%s\"", + PTR_VAL_NO_CRNG); + } else if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0) { + pr_warn("Unexpected format for supposedly hashed capability value\n"); + failed_tests++; + continue; + } + } + +non_hashed: + for (int i = 0; i < ARRAY_SIZE(setup); ++i) { + void * __capability current_cap; + + switch (setup[i].action) { + case CAP_TAG_CLEAR: + current_cap = cheri_tag_clear(cap); + break; + case CAP_NULL_DERIVE: + /* Null-derived capability */ + current_cap = cheri_address_set(0, cheri_address_get(cap)); + break; + default: + current_cap = cap; + break; + } + + if (no_hash_pointers) + test(setup[i].expected, setup[i].plain_fmt, current_cap); + else + ++skipped_tests; + + test(setup[i].expected, setup[i].fmt, current_cap); + } + + return; + +#endif /* CONFIG_ARM64_MORELLO */ +} + static void __init test_pointer(void) { @@ -781,6 +938,7 @@ test_pointer(void) errptr(); fwnode_pointer(); fourcc_pointer(); + capability_pointer(); } static void __init selftest(void) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 40d26a07a133..0ffd72546b85 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -51,6 +51,10 @@ #include <asm/byteorder.h> /* cpu_to_le16 */ #include <asm/unaligned.h> +#ifdef __CHERI__ +#include <cheriintrin.h> +#endif + #include <linux/string_helpers.h> #include "kstrtox.h" @@ -435,7 +439,10 @@ enum format_type { FORMAT_TYPE_UINT, FORMAT_TYPE_INT, FORMAT_TYPE_SIZE_T, - FORMAT_TYPE_PTRDIFF + FORMAT_TYPE_PTRDIFF, +#ifdef __CHERI__ + FORMAT_TYPE_CAPABILITY, +#endif }; struct printf_spec { @@ -2488,6 +2495,181 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, } } +#ifdef __CHERI__ +/* + * Support for printing capabilities with %[#]lp[x] format. + * It stands slightly in contradiction to kernel extensions + * for pointer printk formats as it is using the 'l' length + * modifier instead of relying on extended format specifiers. + * Currently supported formats: + * + * lp[x] - Hex value of full capability bits preceded with capability tag: + * <tag>:<127:64>:<63:0> + * (* outcome subject to kernel pointer hashing ) + * #lp[x] - Simplified format as: + * <address> [permissions=[rwxRWE],<base>-<top>] (attr=[invalid,sentry,sealed]) + * (* outcome subject to kernel pointer hashing ) + * + * Details at: + * Documentation/core-api/printk-formats.rst + * (keep both updated when making any changes) + * + * * - only the address undergoes hashing, remaining bits are being disregarded + * + */ +static noinline_for_stack +char *capability(const char *fmt, char *buf, char *end, void * __capability cap, + struct printf_spec spec) +{ +/* + * Write to the buffer only when there is a space for it, otherwise + * just advance the buffer marker to account for the space needed to + * store full value here + */ +#define update_buf_single(buf, end, c) \ + do { if (buf < end) *buf++ = c; else ++buf; } while (0) + + /* + * For null-derived capabilities switch to basic format + * (address only). + * Same applies when hashing is active. + */ + if ((!cheri_tag_get(cap) && !__builtin_cheri_copy_from_high(cap)) || + (likely(!no_hash_pointers) && *fmt != 'x')) { + + /* Avoid adding prefix */ + spec.flags &= ~SPECIAL; + return pointer(fmt, buf, end, (void *)cheri_address_get(cap), + spec); + } + + if (spec.flags & SPECIAL) { /* Simplified format for capabilities */ + int orig_field_width = spec.field_width; + cheri_perms_t perms = cheri_perms_get(cap); + ptraddr_t base, top; + const char *start = buf; + char *attr_start; + unsigned int i; + int attrib = 0; + int orig_flags; + + /* Note: order matters to match the format expected */ + struct { + cheri_perms_t cperm; char id; + } static const __perms[] = { + { CHERI_PERM_LOAD, 'r' }, + { CHERI_PERM_STORE, 'w' }, + { CHERI_PERM_EXECUTE, 'x' }, + { CHERI_PERM_LOAD_CAP, 'R' }, + { CHERI_PERM_STORE_CAP, 'W' }, +#ifdef CONFIG_ARM64_MORELLO + { ARM_CAP_PERMISSION_EXECUTIVE, 'E' } +#endif + }; + + /* + * Things get slightly confusing here when it comes to + * precision. The standard format specification claims that + * precision might cause truncation of the actual output, + * contrary to width. + * CHERI on the other hand suggests, that precision used + * with the 'p' specifier should determine the width of + * addresses though following current formatting for pointer types, + * unless explicitly stated, the width will be enforced, leaving + * precision being ineffective. Furthermore, according to the guide, + * in theory precision should not affect the way the Raw and Simplified + * formats are being handled (the non-address related parts). + * Stick with that one. + * Note: Using precision with pointers/capabilities with + * active hashing might lead to slightly unexpected output. + * That's the result of applying the precision while + * respecting the field width. + * + * Width in this case refers to the width of final string generated for + * the simplified format + */ + spec.field_width = -1; + orig_flags = spec.flags; + spec.flags &= ~(ZEROPAD | LEFT); + + buf = pointer_string(buf, end, (void *)cheri_address_get(cap), + spec); + + update_buf_single(buf, end, ' '); + update_buf_single(buf, end, '['); + for (i = 0; i < ARRAY_SIZE(__perms); ++i) { + if (perms & __perms[i].cperm) + update_buf_single(buf, end, __perms[i].id); + } + update_buf_single(buf, end, ','); + + base = cheri_base_get(cap); + buf = pointer_string(buf, end, (void *)base, spec); + update_buf_single(buf, end, '-'); + + top = base + cheri_length_get(cap); + buf = pointer_string(buf, end, (void *)top, spec); + update_buf_single(buf, end, ']'); + + /* Attributes */ + /* Reset precision to output full attribute identifiers here */ + spec.precision = -1; + + /* + * Keep track of the attribute start section to format + * it properly in case there are attributes to be reported. + * Otherwise simply rolling on a buffer might lead to overflowing + * past the terminating character if the buffer is big enough. + */ + attr_start = buf; + buf += 2; + + if (!cheri_tag_get(cap)) { + buf = string_nocheck(buf, end, "invalid", spec); + ++attrib; + } + if (cheri_is_sentry(cap)) { + if (attrib++) + update_buf_single(buf, end, ','); + buf = string_nocheck(buf, end, "sentry", spec); + + } + if (cheri_is_sealed(cap)) { + if (attrib++) + update_buf_single(buf, end, ','); + buf = string_nocheck(buf, end, "sealed", spec); + } + + if (attrib) { + update_buf_single(attr_start, end, ' '); + update_buf_single(attr_start, end, '('); + update_buf_single(buf, end, ')'); + } else { + buf = attr_start; /* Rollback on space and opening bracket */ + } + + /* Restore the originally requested width */ + spec.field_width = orig_field_width; + spec.flags |= orig_flags; + + return widen_string(buf, buf - start, end, spec); + } + + /* + * Raw format: hex dump of full capability + */ + update_buf_single(buf, end, cheri_tag_get(cap) ? '1' : '0'); + update_buf_single(buf, end, ':'); + buf = pointer_string(buf, end, + (void *) __builtin_cheri_copy_from_high(cap), + spec); + update_buf_single(buf, end, ':'); + return pointer_string(buf, end, (void *)cheri_address_get(cap), spec); + +#undef update_buf_single +} +#endif /* __CHERI__ */ + /* * Helper function to decode printf style format. * Each call decode a token from the format and return the @@ -2624,6 +2806,10 @@ int format_decode(const char *fmt, struct printf_spec *spec) case 'p': spec->type = FORMAT_TYPE_PTR; +#ifdef __CHERI__ + if (qualifier == 'l') + spec->type = FORMAT_TYPE_CAPABILITY; +#endif return ++fmt - start; case '%': @@ -2717,6 +2903,7 @@ set_precision(struct printf_spec *spec, int prec) * * - ``%n`` is unsupported * - ``%p*`` is handled by pointer() + * - ``%lp[x]`` and ``%#lp[x]`` is handled by capability() * * See pointer() or Documentation/core-api/printk-formats.rst for more * extensive description. @@ -2818,7 +3005,15 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) *str = '%'; ++str; break; - +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: + str = capability(fmt, str, end, + va_arg(args, void * __capability), + spec); + while (isalnum(*fmt)) + fmt++; + break; +#endif case FORMAT_TYPE_INVALID: /* * Presumably the arguments passed gcc's type @@ -3138,7 +3333,25 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args) while (isalnum(*fmt)) fmt++; break; +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: + /* + * Capabilities shall be handled now: subject to + * required alignment, plus exercising extra caution + * when storing the capability in the buffer directly. + */ + str = capability(fmt, str, end, + va_arg(args, void * __capability), + spec); + if (str + 1 < end) + *str++ = '\0'; + else + end[-1] = '\0'; /* Must be null terminated */ + while (isalnum(*fmt)) + fmt++; + break; +#endif default: switch (spec.type) { @@ -3319,7 +3532,21 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf) fmt++; break; } +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: + if (str < end) { + long length = strlen(args); + + memcpy(str, args, min(length, (end - str))); + + str += length; + args += length + 1; + } + while (isalnum(*fmt)) + fmt++; + break; +#endif case FORMAT_TYPE_PERCENT_CHAR: if (str < end) *str = '%'; diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 577e02998701..867e0a01ae9f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6760,7 +6760,7 @@ sub process { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) { + while ($fmt =~ /(\%[\*\d\.#l]*p(\w)(\w*))/g) { $specifier = $1; $extension = $2; $qualifier = $3; @@ -6768,7 +6768,8 @@ sub process { ($extension eq "f" && defined $qualifier && $qualifier !~ /^w/) || ($extension eq "4" && - defined $qualifier && $qualifier !~ /^cc/)) { + defined $qualifier && $qualifier !~ /^cc/) || + ($specifier =~ /lp/ && $extension ne "x")) { $bad_specifier = $specifier; last; } @@ -6780,6 +6781,14 @@ sub process { "Using vsprintf specifier '\%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '\%p'.\n" . "$here\n$stat_real\n"); } } + + if ($fmt =~ /\%[\*\d\.#]*lp/) { + my $stat_real = get_stat_real($linenr, $lc); + + WARN("VSPRINTF_POINTER_EXTENSION", + "Using vsprintf specifier '\%[#]lp[x]' is undefined behaviour when used with non-capability types. Make sure you got it right.\n" . "$here\n$stat_real\n"); + } + if ($bad_specifier ne "") { my $stat_real = get_stat_real($linenr, $lc); my $ext_type = "Invalid"; -- 2.25.1

2 years, 1 month

2
3
0 0

[PATCH] ptrace: fix PTRACE_POKEDATA write size

by Kristina Martsenko

The PTRACE_POKEDATA request writes a word of data to the tracee's memory. In PCuABI the size of the write remains 8 bytes. Currently the kernel erroneously writes 16 bytes, thereby overwriting 8 bytes of unrelated memory. Fix this by restoring the type of the data argument of generic_ptrace_pokedata() to unsigned long. Fixes: ("kernel/ptrace: Modify ptrace syscall to accept capability arguments") Signed-off-by: Kristina Martsenko <kristina.martsenko(a)arm.com> --- include/linux/ptrace.h | 2 +- kernel/ptrace.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index a4c84dbbe084..cfdd378636c7 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -108,7 +108,7 @@ static inline void ptrace_unlink(struct task_struct *child) int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr, user_uintptr_t data); int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr, - user_uintptr_t data); + unsigned long data); /** * ptrace_parent - return the task that is tracing the given task diff --git a/kernel/ptrace.c b/kernel/ptrace.c index c278ae0058a6..e5343257131f 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -1350,7 +1350,7 @@ int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr, } int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr, - user_uintptr_t data) + unsigned long data) { int copied; -- 2.30.2

2 years, 1 month

2
2
0 0

[PATCH 0/3] Introduce VM_READ_CAPS and VM_WRITE_CAPS flags

by Tudor Cretu

Hi, This patch series addresses the VM_READ_CAPS/VM_WRITE_CAPS flags issue: https://git.morello-project.org/morello/kernel/linux/-/issues/36 io_uring system uses buffers shared with userspace to read the io events and report their results. The structs that populate the submission and completion queues can contain capabilities. Shared mappings don't have the Load/Store capabilities permission to avoid leaking capabilities outside their original address space, so add two new VM flags that would allow the kernel to set up such mappings. While at it, also fix pte_modify to allow setting PTE_*_CAPS flags, add new the new rc/wc smaps flags, and remove the automatic addition of PTE_*_CAPS to user mappings. To note: this wouldn't allow userspace to make arbitrary shared mappings with tag access, the new VM flags would be for internal use only for the time being. Review branch: https://git.morello-project.org/tudcre01/linux/-/commits/vm_rw_caps_v1/ Thanks, Tudor --- Tudor Cretu (3): arm64: pgtable: Allow PTE_*_CAPS in the pte_modify mask arm64: mm: Add VM_READ_CAPS and VM_WRITE_CAPS flags arm64: mm: Use VM_*_CAPS instead of enabling PTE_*_CAPS to default user mappings arch/arm64/Kconfig | 1 + arch/arm64/include/asm/mman.h | 19 +++++++++++++++++-- arch/arm64/include/asm/page.h | 2 +- arch/arm64/include/asm/pgtable-prot.h | 12 +++++------- arch/arm64/include/asm/pgtable.h | 2 +- fs/proc/task_mmu.c | 4 ++++ include/linux/mm.h | 9 +++++++++ 7 files changed, 38 insertions(+), 11 deletions(-) -- 2.25.1

2 years, 1 month

2
11
0 0

[PATCH v3] lib/vsprintf: Add support for architectural capabilities

by Beata Michalska

Introduce new printk's pointer format extension to support capabilities, with one being denoted by specifying the 'l' length modifier alongside the 'p' type character => "%lp". In addition to the raw capability format, adding '#' flag will generate the simplified one, being slightly more verbose. In both cases, the actual result is subject to pointer hashing (address only) unless either 'x' qualifier or no_hash_pointers command line parameter is being provided. For more details see the docs, updated here as well. While at it, add some testcases to validate new format alongside checkpatchpl new warning, to raise the awareness of potential misuse. Signed-off-by: Beata Michalska <beata.michalska(a)arm.com> --- v3: - Docs: style/spelling changes - test: improved code layout (as per review comments), dropped stale SEAL permission, fixed accounting for skipped tests - lib/vsprintf.c: - switched from __cheri_fromcap to cheri_address_get - fixed indentations - vbin_printf: adjusted comment for storing caps in bin buffer - bstr_printf: added missing check for buffer end v2: - Rephrase/rearrange docs + adding link to the CHERI guidelines on printf formats (also dropping 'hybrid-mode' mentions) - Code readability/formatting improvements (as per review comments) - Adding '0x' prefix for relevant simplified format components - Removing missing permissions from the final output in simplified format - Use pointer_string directly when no hashing is expected - Added support for vbin_printf/bstr_printf (trace_printk is now able to print capabilities) Review branch available at: https://git.morello-project.org/Bea/linux/-/tree/morello/cap_printk_v3 --- Documentation/core-api/printk-formats.rst | 65 ++++++ lib/test_printf.c | 158 +++++++++++++++ lib/vsprintf.c | 235 +++++++++++++++++++++- scripts/checkpatch.pl | 13 +- 4 files changed, 467 insertions(+), 4 deletions(-) diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index 5e89497ba314..8f85dca293b1 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -202,6 +202,71 @@ Example:: printk("test: difference between pointers: %td\n", ptr2 - ptr1); +Capabilities +------------ + +:: + + %lp[x] 1:ffffc00000010005:0123456789abcdef + %#lp[x] 0x0123456789abcdef [rwxRWE,0x0000000000000000-0xffffffffffffffff] + + +For printing CHERI architectural capabilities (when supported, otherwise +the result of using it is undefined). Formatting based on: +https://github.com/CTSRD-CHERI/cheri-c-programming/wiki/Displaying-Capabilities +with a small detour of dropping support for basic format in favour of the raw +one, with the assumption that the actual address can be printed with existing +printk formats (subject to appropriate casting when dealing with capabilities +and non-capability formats, which otherwise renders undefined behaviour). + +The ``l`` modifier distinguishes capability pointers from standard (integer) +pointers, giving the following capability format being printed:: + + + Capability tag:High bits:Low bits + ------------------------------------ + <tag>:<127:64>:<63:0> + + +Specifying ``#`` modifier results in printing capabilities in a simplified format:: + + + <address> [<permissions>,<base>-<top>] (<attr>) + +where:: + + + - permissions - none or any combination of: + - 'r' : CHERI_PERM_LOAD + - 'w' : CHERI_PERM_STORE + - 'x' : CHERI_PERM_EXECUTE + - 'R' : CHERI_PERM_LOAD_CAP + - 'W' : CHERI_PERM_STORE_CAP + - 'E' : ARM_CAP_PERMISSION_EXECUTIVE + - base - lower bound + - top - upper bound + - attr - none or any combination of: + - 'invalid' : capability's tag is clear + - 'sentry' : capability is a sealed entry + - 'sealed' : capability is sealed with a type other than the + sealed entry object type + + +The above applies to formats with either ``no_hash_pointers`` parameter being +enabled or ``x`` modifier being provided - otherwise subject to hashing +(address only, with remaining bits being disregarded) + +Refer to `Unmodified Addresses`_ for potential security implications. + +Currently only Morello architecture is being supported. + +Note: +An attempt to use this format with a non-capability type is undefined behaviour. +There are no safety nets, so extra caution is advised! Handle with care. +Same applies when capabilities are not supported, as the use of either the ``#`` +flag or the ``l`` length modifier is undefined behaviour when combined with +%p. + Struct Resources ---------------- diff --git a/lib/test_printf.c b/lib/test_printf.c index 07309c45f327..858d4ce6d8b4 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -24,6 +24,10 @@ #include <linux/property.h> +#ifdef __CHERI__ +#include <cheriintrin.h> +#endif + #include "../tools/testing/selftests/kselftest_module.h" #define BUF_SIZE 256 @@ -755,6 +759,159 @@ errptr(void) #endif } +static void __init +capability_pointer(void) +{ +#ifdef __CHERI__ + enum action { + /* No action - use as is */ + CAP_AXN_NONE, + /* Clear capability tag */ + CAP_TAG_CLEAR, + /* Derive null-capability */ + CAP_NULL_DERIVE + }; + + struct { + const char *plain_fmt; + const char *fmt; + const char *expected; + int action; + } const setup[] = { + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = "1:d800400059ab89ab:"PTR_STR, + .action = CAP_AXN_NONE, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab]", + .action = CAP_AXN_NONE, + }, + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = "0:d800400059ab89ab:"PTR_STR, + .action = CAP_TAG_CLEAR, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab] (invalid)", + .action = CAP_TAG_CLEAR, + }, + { + .plain_fmt = "%lp", + .fmt = "%lpx", + .expected = PTR_STR, + .action = CAP_NULL_DERIVE, + }, + { + .plain_fmt = "%#lp", + .fmt = "%#lpx", + .expected = PTR_STR, + .action = CAP_NULL_DERIVE, + } + }; + + char buf[PLAIN_BUF_SIZE]; + const char * const cap_fmt_non_hashed[] = {"%lp", "%#lp"}; + int nchars; + + void * __capability cap = cheri_ddc_get(); + + /* Expecting basic permissions to be set */ + size_t perms = CHERI_PERM_GLOBAL | CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP | + CHERI_PERM_STORE | CHERI_PERM_STORE_CAP; + + size_t bounds = 4096; + + cap = cheri_address_set(cap, (ptraddr_t)PTR); + cap = cheri_perms_and(cap, perms); + /* + * Basic checks so that the actual output can be safely compared + * to the expected one + */ + if (!cheri_tag_get(cap) || cheri_perms_get(cap) != perms + || cheri_is_sealed(cap)) { + pr_warn("Failed to create capability for testing - skipping"); + skipped_tests += no_hash_pointers ? + ARRAY_SIZE(cap_fmt_non_hashed) + ARRAY_SIZE(setup) : + ARRAY_SIZE(cap_fmt_non_hashed) + ARRAY_SIZE(setup)/2; + return; + } + + cap = cheri_bounds_set_exact(cap, bounds); + + /* Verify hashing */ + if (no_hash_pointers) { + pr_warn("Skipping capability hashing tests\n"); + skipped_tests += ARRAY_SIZE(cap_fmt_non_hashed); + goto non_hashed; + } + + for (int i = 0; i < ARRAY_SIZE(cap_fmt_non_hashed); ++i) { + nchars = snprintf(buf, PLAIN_BUF_SIZE, cap_fmt_non_hashed[i], + cap); + + /* + * Should be ADDR_WIDTH in this case , but hey, let's not be picky + * about it, at least not here ... not yet ... + */ + if (nchars != PTR_WIDTH) { + /* + * This also covers the case when the value has not been + * actually hashed, as nchars would then be greater than + * PTR_WIDTH + */ + pr_warn("Something went wrong with hashing capability value\n"); + goto failed; + } + + if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { + pr_warn("crng possibly not yet initialized - capability value buffer contains \"%s\"", + PTR_VAL_NO_CRNG); + } else if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0) { + pr_warn("Unexpected format for supposedly hashed capability value\n"); + goto failed; + } + } + +non_hashed: + for (int i = 0; i < ARRAY_SIZE(setup); ++i) { + void * __capability current_cap; + + switch (setup[i].action) { + case CAP_TAG_CLEAR: + current_cap = cheri_tag_clear(cap); + break; + case CAP_NULL_DERIVE: + /* Null-derived capability */ + current_cap = cheri_address_set(0, + cheri_address_get(cap)); + break; + default: + current_cap = cap; + break; + } + + if (no_hash_pointers) + test(setup[i].expected, setup[i].plain_fmt, current_cap); + else + ++skipped_tests; + + test(setup[i].expected, setup[i].fmt, current_cap); + } + + return; + +failed: + failed_tests++; +#endif /* CONFIG_ARM64_MORELLO */ +} + static void __init test_pointer(void) { @@ -781,6 +938,7 @@ test_pointer(void) errptr(); fwnode_pointer(); fourcc_pointer(); + capability_pointer(); } static void __init selftest(void) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 40d26a07a133..708fe114bbf7 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -51,6 +51,10 @@ #include <asm/byteorder.h> /* cpu_to_le16 */ #include <asm/unaligned.h> +#ifdef __CHERI__ +#include <cheriintrin.h> +#endif + #include <linux/string_helpers.h> #include "kstrtox.h" @@ -435,7 +439,10 @@ enum format_type { FORMAT_TYPE_UINT, FORMAT_TYPE_INT, FORMAT_TYPE_SIZE_T, - FORMAT_TYPE_PTRDIFF + FORMAT_TYPE_PTRDIFF, +#ifdef __CHERI__ + FORMAT_TYPE_CAPABILITY, +#endif }; struct printf_spec { @@ -2488,6 +2495,181 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, } } +#ifdef __CHERI__ +/* + * Support for printing capabilities with %[#]lp[x] format. + * It stands slightly in contradiction to kernel extensions + * for pointer printk formats as it is using the 'l' length + * modifier instead of relying on extended format specifiers. + * Currently supported formats: + * + * lp[x] - Hex value of full capability bits preceded with capability tag: + * <tag>:<127:64>:<63:0> + * (* outcome subject to kernel pointer hashing ) + * #lp[x] - Simplified format as: + * <address> [permissions=[rwxRWE],<base>-<top>] (attr=[invalid,sentry,sealed]) + * (* outcome subject to kernel pointer hashing ) + * + * Details at: + * Documentation/core-api/printk-formats.rst + * (keep both updated when making any changes) + * + * * - only the address undergoes hashing, remaining bits are being disregarded + * + */ +static noinline_for_stack +char *capability(const char *fmt, char *buf, char *end, void * __capability cap, + struct printf_spec spec) +{ +/* + * Write to the buffer only when there is a space for it, otherwise + * just advance the buffer marker to account for the space needed to + * store full value here + */ +#define update_buf_single(buf, end, c) \ + do { if (buf < end) *buf++ = c; else ++buf; } while (0) + + /* + * For null-derived capabilities switch to basic format + * (address only). + * Same applies when hashing is active. + */ + if ((!cheri_tag_get(cap) && !__builtin_cheri_copy_from_high(cap)) || + (likely(!no_hash_pointers) && *fmt != 'x')) { + + /* Avoid adding prefix */ + spec.flags &= ~SPECIAL; + return pointer(fmt, buf, end, (void *)cheri_address_get(cap), + spec); + } + + if (spec.flags & SPECIAL) { /* Simplified format for capabilities */ + int orig_field_width = spec.field_width; + cheri_perms_t perms = cheri_perms_get(cap); + ptraddr_t base, top; + const char *start = buf; + char *attr_start; + unsigned int i; + int attrib = 0; + int orig_flags; + + /* Note: order matters to match the format expected */ + struct { + cheri_perms_t cperm; char id; + } static const __perms[] = { + { CHERI_PERM_LOAD, 'r' }, + { CHERI_PERM_STORE, 'w' }, + { CHERI_PERM_EXECUTE, 'x' }, + { CHERI_PERM_LOAD_CAP, 'R' }, + { CHERI_PERM_STORE_CAP, 'W' }, +#ifdef CONFIG_ARM64_MORELLO + { ARM_CAP_PERMISSION_EXECUTIVE, 'E' } +#endif + }; + + /* + * Things get slightly confusing here when it comes to + * precision. The standard format specification claims that + * precision might cause truncation of the actual output, + * contrary to width. + * CHERI on the other hand suggests, that precision used + * with the 'p' specifier should determine the width of + * addresses though following current formatting for pointer types, + * unless explicitly stated, the width will be enforced, leaving + * precision being ineffective. Furthermore, according to the guide, + * in theory precision should not affect the way the Raw and Simplified + * formats are being handled (the non-address related parts). + * Stick with that one. + * Note: Using precision with pointers/capabilities with + * active hashing might lead to slightly unexpected output. + * That's the result of applying the precision while + * respecting the field width. + * + * Width in this case refers to the width of final string generated for + * the simplified format + */ + spec.field_width = -1; + orig_flags = spec.flags; + spec.flags &= ~(ZEROPAD | LEFT); + + buf = pointer_string(buf, end, (void *)cheri_address_get(cap), + spec); + + update_buf_single(buf, end, ' '); + update_buf_single(buf, end, '['); + for (i = 0; i < ARRAY_SIZE(__perms); ++i) { + if (perms & __perms[i].cperm) + update_buf_single(buf, end, __perms[i].id); + } + update_buf_single(buf, end, ','); + + base = cheri_base_get(cap); + buf = pointer_string(buf, end, (void *)base, spec); + update_buf_single(buf, end, '-'); + + top = base + cheri_length_get(cap); + buf = pointer_string(buf, end, (void *)top, spec); + update_buf_single(buf, end, ']'); + + /* Attributes */ + /* Reset precision to output full attribute identifiers here */ + spec.precision = -1; + + /* + * Keep track of the attribute start section to format + * it properly in case there are attributes to be reported. + * Otherwise simply rolling on a buffer might lead to overflowing + * past the terminating character if the buffer is big enough. + */ + attr_start = buf; + buf += 2; + + if (!cheri_tag_get(cap)) { + buf = string_nocheck(buf, end, "invalid", spec); + ++attrib; + } + if (cheri_is_sentry(cap)) { + if (attrib++) + update_buf_single(buf, end, ','); + buf = string_nocheck(buf, end, "sentry", spec); + + } + if (cheri_is_sealed(cap)) { + if (attrib++) + update_buf_single(buf, end, ','); + buf = string_nocheck(buf, end, "sealed", spec); + } + + if (attrib) { + update_buf_single(attr_start, end, ' '); + update_buf_single(attr_start, end, '('); + update_buf_single(buf, end, ')'); + } else { + buf = attr_start; /* Rollback on space and opening bracket */ + } + + /* Restore the originally requested width */ + spec.field_width = orig_field_width; + spec.flags |= orig_flags; + + return widen_string(buf, buf - start, end, spec); + } + + /* + * Raw format: hex dump of full capability + */ + update_buf_single(buf, end, cheri_tag_get(cap) ? '1' : '0'); + update_buf_single(buf, end, ':'); + buf = pointer_string(buf, end, + (void *) __builtin_cheri_copy_from_high(cap), + spec); + update_buf_single(buf, end, ':'); + return pointer_string(buf, end, (void *)cheri_address_get(cap), spec); + +#undef update_buf_single +} +#endif /* __CHERI__ */ + /* * Helper function to decode printf style format. * Each call decode a token from the format and return the @@ -2624,6 +2806,10 @@ int format_decode(const char *fmt, struct printf_spec *spec) case 'p': spec->type = FORMAT_TYPE_PTR; +#ifdef __CHERI__ + if (qualifier == 'l') + spec->type = FORMAT_TYPE_CAPABILITY; +#endif return ++fmt - start; case '%': @@ -2717,6 +2903,7 @@ set_precision(struct printf_spec *spec, int prec) * * - ``%n`` is unsupported * - ``%p*`` is handled by pointer() + * - ``%lp[x]`` and ``%#lp[x]`` is handled by capability() * * See pointer() or Documentation/core-api/printk-formats.rst for more * extensive description. @@ -2818,7 +3005,15 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) *str = '%'; ++str; break; - +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: + str = capability(fmt, str, end, + va_arg(args, void * __capability), + spec); + while (isalnum(*fmt)) + fmt++; + break; +#endif case FORMAT_TYPE_INVALID: /* * Presumably the arguments passed gcc's type @@ -3138,7 +3333,28 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args) while (isalnum(*fmt)) fmt++; break; +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: + /* + * Capabilities shall be handled now: subject to + * required alignment, plus exercising extra caution + * when storing the capability in the buffer directly. + * Otherwise breaking down the capability + * to extract the information requested (format) + * would be rather counter-productive. + */ + str = capability(fmt, str, end, + va_arg(args, void * __capability), + spec); + if (str + 1 < end) + *str++ = '\0'; + else + end[-1] = '\0'; /* Must be null terminated */ + while (isalnum(*fmt)) + fmt++; + break; +#endif default: switch (spec.type) { @@ -3319,7 +3535,22 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf) fmt++; break; } +#ifdef __CHERI__ + case FORMAT_TYPE_CAPABILITY: { + long length = strlen(args); + + if (str < end) { + memcpy(str, args, min(length, (end - str))); + + str += length; + args += length + 1; + } + while (isalnum(*fmt)) + fmt++; + break; + } +#endif case FORMAT_TYPE_PERCENT_CHAR: if (str < end) *str = '%'; diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 577e02998701..867e0a01ae9f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6760,7 +6760,7 @@ sub process { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) { + while ($fmt =~ /(\%[\*\d\.#l]*p(\w)(\w*))/g) { $specifier = $1; $extension = $2; $qualifier = $3; @@ -6768,7 +6768,8 @@ sub process { ($extension eq "f" && defined $qualifier && $qualifier !~ /^w/) || ($extension eq "4" && - defined $qualifier && $qualifier !~ /^cc/)) { + defined $qualifier && $qualifier !~ /^cc/) || + ($specifier =~ /lp/ && $extension ne "x")) { $bad_specifier = $specifier; last; } @@ -6780,6 +6781,14 @@ sub process { "Using vsprintf specifier '\%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '\%p'.\n" . "$here\n$stat_real\n"); } } + + if ($fmt =~ /\%[\*\d\.#]*lp/) { + my $stat_real = get_stat_real($linenr, $lc); + + WARN("VSPRINTF_POINTER_EXTENSION", + "Using vsprintf specifier '\%[#]lp[x]' is undefined behaviour when used with non-capability types. Make sure you got it right.\n" . "$here\n$stat_real\n"); + } + if ($bad_specifier ne "") { my $stat_real = get_stat_real($linenr, $lc); my $ext_type = "Invalid"; -- 2.25.1

2 years, 1 month

2
3
0 0

[PATCH] uapi/linux/const.h: Prefer ISO-friendly __typeof__

by Kevin Brodsky

typeof is (still) a GNU extension, which means that it cannot be used when building ISO C (e.g. -std=c99). It should therefore be avoided in uapi headers in favour of the ISO-friendly __typeof__. Reported-by: Ruben Ayrapetyan <ruben.ayrapetyan(a)arm.com> Signed-off-by: Kevin Brodsky <kevin.brodsky(a)arm.com> --- This is really an upstream issue that has gone unnoticed so far, probably because __ALIGN_KERNEL is not widely used. Our recent commit a5cf170fd954 ("uapi: siginfo.h: Modify struct sigevent for PCuABI") revealed it, as it uses __ALIGN_KERNEL in siginfo.h in such a way that the macro is always expanded, so any userspace ISO C/C++ code including siginfo.h gets a build failure. Thinking about posting this on LKML, any opinion for/against or suggestion? Thanks, Kevin include/uapi/linux/const.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index af2a44c08683..a429381e7ca5 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -28,7 +28,7 @@ #define _BITUL(x) (_UL(1) << (x)) #define _BITULL(x) (_ULL(1) << (x)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) -- 2.34.1

2 years, 1 month

4
7
0 0

2024

2023

2022

linux-morello November 2022