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

Re: [PATCH] ptrace: fix PTRACE_POKEDATA write size

by Kristina Martsenko

On 15/11/2022 10:43, Teo Couprie Diaz wrote: > On 11/11/2022 17:49, Kristina Martsenko wrote: > >> 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. > > The fix makes sense by itself, maybe it's my lack of experience with kernel dev but do we always expect it to be 8 bytes of data to trace ? The ptrace man page states that it's a machine "word", so 8 on 64-bit systems and 4 on 32-bit systems. (It remains 8 on Morello according to the PCuABI spec, there is a new request type PTRACE_POKECAP for writing 16-byte capabilities.) > If so, isn't there some type that could be more specific (and ideally fixed in size) that could be used here as to prevent further confusion later and be sure that the right amount of bytes will always be written ? Yeah, that would make sense. But the C language doesn't have a type for the machine word size, and I think the kernel just uses "unsigned long" for that. Also note that in this particular case I'm just reverting part of the change made in commit 343ca6c6706e ("kernel/ptrace: Modify ptrace syscall to accept capability arguments"), not introducing a new change. > > I'm also a bit confused when looking a the function where it gets called from : > > int ptrace_request(struct task_struct *child, long request, > user_uintptr_t addr, user_uintptr_t data) > > Here `addr` is an `user_uinptr_t`, which gets passed to `generic_ptrace_pokedata()` which will use it to poke said data. But there the type of `addr` is `unsigned long`, shouldn't it be `user_uintptr_t` as well ? > The type in ptrace_request() needs to be user_uintptr_t because ptrace is a multiplexed syscall that can take both pointers and integers in the 'addr' and 'data' arguments. Based on the request type (the big switch statement in ptrace_request()), the argument gets interpreted as either a pointer or integer. In the case of PTRACE_POKEDATA, the PCuABI spec [1] states that 'addr' is interpreted as a 64-bit value, because it holds an address in the tracee's address space (for which the tracer doesn't have a capability). So the call to generic_ptrace_pokedata() implicitly converts it to unsigned long. > It also means that `data` is going through the same kind of type change, but it's probably less of an issue because it's just data to be copied around. 'data' also needs to be interpreted as a 64-bit value for the PTRACE_POKEDATA request type because it needs to be the size of a 'word', as described above. > > > I'm not sure what I am saying is really relevant, if not please ignore ! :) Thanks for having a look! Kristina [1] https://git.morello-project.org/morello/kernel/linux/-/wikis/Morello-pure-c… > Regards, > Téo > >> 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 years

2
1
0 0

[PATCH v2] 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> --- 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 --- Documentation/core-api/printk-formats.rst | 68 +++++++ lib/test_printf.c | 133 ++++++++++++ lib/vsprintf.c | 233 +++++++++++++++++++++- scripts/checkpatch.pl | 13 +- 4 files changed, 443 insertions(+), 4 deletions(-) diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index 5e89497ba314..bd97a9750e54 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -202,6 +202,74 @@ 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 behavior). + +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 nests, so extra consciousness 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..71d5b8d1c9b5 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,134 @@ 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 __setup { + char *plain_fmt; + char *fmt; + char *expected; + int action; + } const setup[] = { + { "%lp", "%lpx", "1:da00400059ab89ab:"PTR_STR, CAP_AXN_NONE }, + { "%#lp", "%#lpx", + "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab]", + CAP_AXN_NONE + }, + { "%lp", "%lpx", "0:da00400059ab89ab:"PTR_STR, CAP_TAG_CLEAR }, + { "%#lp", "%#lpx", + "0x"PTR_STR" [rwRW,0xffff0123456789ab-0xffff0123456799ab] (invalid)", + CAP_TAG_CLEAR + }, + { "%lp", "%lpx", PTR_STR, CAP_NULL_DERIVE }, + { "%#lp", "%#lpx", PTR_STR, CAP_NULL_DERIVE } + }; + + char buf[PLAIN_BUF_SIZE]; + const char * const cap_fmt[] = {"%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 | CHERI_PERM_SEAL; + + 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 ? 4 + ARRAY_SIZE(setup) : + 4 + 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 += 4; + goto non_hashed; + } + + for (int i = 0; i < ARRAY_SIZE(cap_fmt); ++i) { + + nchars = snprintf(buf, PLAIN_BUF_SIZE, cap_fmt[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 otherwise nchars would 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 = cap; + + switch (setup[i].action) { + case CAP_TAG_CLEAR: + current_cap = cheri_tag_clear(current_cap); + break; + case CAP_NULL_DERIVE: + /* Null-derived capability */ + current_cap = (void *__capability) + (__cheri_addr uintcap_t)((void *)cheri_base_get(current_cap)); + break; + default: + 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 +913,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..af08619ebd42 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 case 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, (__cheri_fromcap void *)(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, (__cheri_fromcap void *)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, (__cheri_fromcap void *)(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 need to be handled now: subject to + * LoadCap/StoreCap permission which would make this + * unsafe if 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,20 @@ 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); + + 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

2
2
0 0

[PATCH 0/4] Support io_uring for Purecap apps

by Tudor Cretu

This series makes it possible for purecap apps to use the io_uring system. With these patches, all io_uring LTP tests pass in both Purecap and compat modes. Note that the LTP tests only address the basic functionality of the io_uring system and a significant portion of the multiplexed functionality is untested. Review branch: https://git.morello-project.org/tudcre01/linux/-/commits/io_uring_v1 Tudor Cretu (4): compiler_types: Add (u)intcap_t to native_words arm64: mm: Add VM_READ_CAPS and VM_WRITE_CAPS flags io_uring: Implement compat versions of uAPI structs and handle them io_uring: Use user pointer type in the uAPI structs arch/arm64/Kconfig | 1 + arch/arm64/include/asm/mman.h | 6 + fs/io_uring.c | 734 ++++++++++++++++++++++++++------- fs/proc/task_mmu.c | 4 + include/linux/compiler_types.h | 7 + include/linux/mm.h | 10 + include/uapi/linux/io_uring.h | 32 +- 7 files changed, 623 insertions(+), 171 deletions(-) -- 2.25.1

2 years

2
10
0 0

[RFC PATCH 0/9] New CHERI API and rehauled user_ptr.h

by Kevin Brodsky

Hi, I am posting this series now to gather some opinions (notably in terms of naming) before I proceed further. The main focus is the introduction of two new user_ptr helpers: make_privileged_user_ptr() to create fine-grained user pointers (appropriate bounds and permissions), and check_user_ptr() to check user pointers. This does however require more involved CHERI operations than what we've used so far, and as a result it felt like the right time to introduce a new header with various CHERI-related definitions. This new cheri.h header should be included in new code instead of the compiler-provided cheriintrin.h, notably because it is safe to include it unconditionally. linux/cheri.h is also a great place to introduce appropriate (CHERI-generic) root capabilities, which is another focus of this series. This makes it possible to have generic implementations of uaddr_to_user_ptr*() and get rid of asm/user_ptr.h. The introduction of a root userspace capability with appropriate bounds and permissions is the only functional change from a userspace perspective: many capabilities given to userspace will now have bounds encompassing only the user address space and permissions corresponding to what is expected of an RWX capability in PCuABI. This work is to be continued by replacing most uses of morello_root_cap with cheri_root_cap_userspace (either in v2 or in a separate series). On a similar theme, compat_ptr() should be modified to derive capabilities from the current user DDC, and the new seal/CID root capabilities should be used in binfmt_elf.c. This would complete the transition to appropriate root capabilities. Back to the two new user_ptr helpers, make_privileged_user_ptr() is meant to replace uaddr_to_user_ptr_safe() and the latter should eventually disappear. This probably belongs to a different patch series, however the last patch provides an example of such a change. This work should probably wait until we start accessing user memory through capabilities in uaccess, as right now the capability metadata is not used anyway. Note that calls to uaddr_to_user_ptr() are workarounds in themselves and should all be eliminated eventually, so they are not considered here. Regarding check_user_ptr(), there is no immediate need for it - it will become relevant to implement explicit checking of user pointers (when get_user_pages() and friends are used). Finally the user_ptr.rst documentation needs to be updated to reflect the new helpers, this is to be done in v2. This series depends on Beata's handy printk patch for the warning messages. It was lightly tested and should be mostly fine, however note that compat_ptr() currently triggers warnings because it is implemented in terms of uaddr_to_user_ptr_safe() and compat_ptr() may be passed arbitrary integers. This will be fixed in v2 by appropriately deriving capabilities from DDC as mentioned above. Review branch: https://git.morello-project.org/kbrodsky-arm/linux/-/commits/cheri_ptr_api Thanks, Kevin Kevin Brodsky (9): linux/user_ptr.h: Remove kaddr_to_user_ptr() linux/user_ptr.h: Improve comment formatting arm64: uapi: Add asm/cheri.h linux/cheri.h: Introduce CHERI helpers arm64: morello: Implement cheri.h linux/user_ptr.h: Generic PCuABI impl for uaddr_to_user_ptr* arm64: Remove asm/user_ptr.h linux/user_ptr.h: Introduce fine-grained helpers mm/memory: Create fine-grained user pointer Documentation/core-api/user_ptr.rst | 8 -- arch/Kconfig | 2 +- arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/cheri.h | 14 ++++ arch/arm64/include/asm/user_ptr.h | 43 ---------- arch/arm64/include/uapi/asm/cheri.h | 7 ++ arch/arm64/kernel/morello.c | 39 +++++++-- include/linux/cheri.h | 122 ++++++++++++++++++++++++++++ include/linux/user_ptr.h | 113 +++++++++++++++++++------- lib/Makefile | 3 + lib/cheri.c | 67 +++++++++++++++ lib/user_ptr.c | 62 ++++++++++++++ mm/memory.c | 3 +- 13 files changed, 392 insertions(+), 93 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h delete mode 100644 arch/arm64/include/asm/user_ptr.h create mode 100644 arch/arm64/include/uapi/asm/cheri.h create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c create mode 100644 lib/user_ptr.c -- 2.34.1

2 years

7
54
0 0

Introduction on the "Linux on Morello Project"

by Vincenzo Frascino

Hi Arnd, I spoke to Linus (in Cc) on Friday and I thought it was a good idea to give to you an update on what we are doing as part of the linux on Morello project. We originally started with the basic enablement of the feature almost two year ago and then proceeded enabling the userspace support as part of the research project. To do so we went through the exercise of defining a Pure Capability based user Application Binary Interface (PCuABI) [1]. This ABI is still in review and we are hoping to finalize it by the end of October 2022. To get started with our implementation we identified a more stable subset of the full PCuABI which we call transitional PCuABI [2] and made sure it can work with the most commonly used C libraries (musl, glibc). The full PCuABI can be seen as an extension of the Transitional PCuABI. Recently we opened our implementation of the transitional PCuABI for external contributions [3]. We setup a mailing list as well for reviews and general discussions around Morello [4] and have a public task tracker that details what we are planning to do next [5]. Last but not least we have a public CI that verifies our implementation (currently based on kselftest and ltp but we are planning to extend it to more test suites in future) [6]. In reading our code, please consider that to enable userspace "quickly" we had to take some shortcuts of which we are aware. Because of that we feel that this is the right moment to start discussing design choices with the wider linux community especially after Matt's (in Cc) presentation at LPC ("Zettalinux: It's Not Too Late To Start") which made us realize that in the near future we will have to solve similar kind of problems. We consider in fact problems like the distinction in between an address and a pointer foundational work for a pure capability kernel. Caveat: Morello is not a committed architecture and the arm64 maintainers are not keen on supporting it upstream before that happens. I hope this email provides an helpful introduction on where we are and what we are doing. Please feel free to ask any question and to subscribe to the list if you want to follow the work we are doing. Thanks, Vincenzo [1] https://git.morello-project.org/morello/kernel/linux/-/wikis/Morello-pure-c… [2] https://git.morello-project.org/morello/kernel/linux/-/wikis/Transitional-M… [3] https://git.morello-project.org/morello/kernel/linux [4] https://op-lists.linaro.org/mailman3/lists/linux-morello.op-lists.linaro.or… [5] https://git.morello-project.org/groups/morello/kernel/-/epics?state=opened&… [6] https://git.morello-project.org/morello/kernel/linux/-/pipelines

2 years, 1 month

3
6
0 0

[PATCH v2] kernel: kcmp: Fix syscall ABI for purecap

by carsten.haitzler＠foss.arm.com

From: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguments might be pointers (capabilites) passed in. One of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. Signed-off-by: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> --- kernel/kcmp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..d59f74c68c93 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,7 +133,7 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2) + unsigned long, idx1, user_uintptr_t, idx2) { struct task_struct *task1, *task2; int ret; @@ -204,7 +204,8 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD: - ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2); + ret = kcmp_epoll_target(task1, task2, idx1, + (struct kcmp_epoll_slot __user *)idx2); break; default: ret = -EINVAL; -- 2.25.1

2 years, 1 month

2
2
0 0

[PATCH v2] mm/mmap: disable brk in purecap

by Teo Couprie Diaz

brk is not implemented in purecap, return -ENOSYS when not in compat. Signed-off-by: Teo Couprie Diaz <teo.coupriediaz(a)arm.com> --- Thanks Tudor for providing the code snippet, making it much more clear than my original ideas. v2: Fix style and format issues. mm/mmap.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mm/mmap.c b/mm/mmap.c index ce282f9d9f8e..5de8e48b66b7 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -202,6 +202,11 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) bool downgraded = false; LIST_HEAD(uf); +#ifdef CONFIG_CHERI_PURECAP_UABI + if (!in_compat_syscall()) + return -ENOSYS; +#endif + if (mmap_write_lock_killable(mm)) return -EINTR; base-commit: 3deb26714719d5068f5ef5d0fa9bc457c3cef6c1 -- 2.25.1

2 years, 1 month

2
1
0 0

[PATCH] kernel: kcmp: Fix syscall ABI for morello purecap

by carsten.haitzler＠foss.arm.com

From: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguments might be pointers (capabilites) passed in. One of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. Signed-off-by: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> --- kernel/kcmp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..da164266d647 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,7 +133,7 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2) + unsigned long, idx1, user_uintptr_t, idx2) { struct task_struct *task1, *task2; int ret; @@ -204,7 +204,8 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD: - ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2); + ret = kcmp_epoll_target(task1, task2, idx1, + (struct kcmp_epoll_slot __user *)idx2); break; default: ret = -EINVAL; -- 2.25.1

2 years, 1 month

3
2
0 0

[PATCH] kernel: kcmp: Fix syscall ABI for morello purecap

by carsten.haitzler＠foss.arm.com

From: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguemnts might be pointers (capabilites) passed in. One of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. This allows for idx2 and idx1 to potentially do this now. Signed-off-by: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> --- kernel/kcmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..6f29a94b07d4 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,13 +133,12 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2) + user_uintptr_t, idx1, user_uintptr_t, idx2) { struct task_struct *task1, *task2; int ret; rcu_read_lock(); - /* * Tasks are looked up in caller's PID namespace only. */ @@ -204,7 +203,7 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD: - ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2); + ret = kcmp_epoll_target(task1, task2, idx1, (struct kcmp_epoll_slot __user *)idx2); break; default: ret = -EINVAL; -- 2.25.1

2 years, 1 month

3
2
0 0

[PATCH] fs/timerfd: Modify last arg to timerfd_ioctl to be user_uintptr_t

by carsten.haitzler＠foss.arm.com

From: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> Block device ioctl's now take a user_uintptr_t as last argument for morello, so adapt to this to keep compiling if CONFIG_CHECKPOINT_RESTORE is enabled. Signed-off-by: Carsten Haitzler <carsten.haitzler(a)foss.arm.com> --- fs/timerfd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/timerfd.c b/fs/timerfd.c index e9c96a0c79f1..5a362ef4eed1 100644 --- a/fs/timerfd.c +++ b/fs/timerfd.c @@ -347,7 +347,7 @@ static void timerfd_show(struct seq_file *m, struct file *file) #endif #ifdef CONFIG_CHECKPOINT_RESTORE -static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +static long timerfd_ioctl(struct file *file, unsigned int cmd, user_uintptr_t arg) { struct timerfd_ctx *ctx = file->private_data; int ret = 0; -- 2.25.1

2 years, 1 month

3
2
0 0

2024

2023

2022

linux-morello