Hi,
This series switches all uaccess routines to operate directly on the user-provided capability pointer in PCuABI, instead of making the access via the extracted 64-bit address. This means that all forms of uaccess now enforce the capability metadata, through the use of capability-based loads and stores, and will fail with -EFAULT following a failed (hardware) capability check. The impacted routines are:
- *get_user* (including get_user_ptr) - *put_user* (including put_user_ptr) - *copy_from_user* (including copy_from_user_with_ptr) - *copy_to_user* (including copy_to_user_with_ptr)
(Note that futex operations are already checked thanks to Luca's work [1].)
Enabling capability checks in uaccess exposed a variety of issues, both in the kernel and in userspace. This series addresses the fallout in the kernel before making the switch:
- Patch 1-3 overhaul strnlen_user() to prevent it from reading beyond the bounds of the user pointer, while leaving its implementation unchanged in !PCuABI. - Patch 4-6 fixes various capability propagation issues (surprisingly few of those!). - Patch 7 fixes a bug in a Morello kselftest, detected through capability checking in copy_from_user().
Finally, the switch to capability-based accesses is made:
- Patch 8-10 switch {get,put}_user and variants. - Patch 11-12 switch copy_{from,to}_user and variants.
After this series, the Morello kselftests and Musl tests still pass, and so do the liburing tests, however the LTP syscalls suite doesn't because of various issues in LTP itself. I have addressed a number of them upstream [2] (series already merged), and will post the remaining fixes to linux-morello-ltp.
Because this precise uaccess checking cannot be achieved with most existing sanitisers or hardware memory safety mechanisms, further bugs are likely to be identified in low-level libraries and test suites running in purecap (I have spotted a couple in the Bionic tests).
Issue:
https://git.morello-project.org/morello/kernel/linux/-/issues/5
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/checked...
Cheers, Kevin
[1] https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... [2] https://lore.kernel.org/ltp/20231023135647.2157030-1-kevin.brodsky@arm.com/
Kevin Brodsky (12): linux/user_ptr.h: Introduce user_ptr_{base,limit} lib: Refactor do_strnlen_user() lib: Make strnlen_user() PCuABI-friendly block: Eliminate invalid user pointer casts tty: Convert another ioctl helper to take arg as user_uintptr_t seccomp: Preserve pointers when reading sock_fprog from userspace kselftests/arm64: morello: Fix message size in mmap test arm64: morello: Implement user capability accessors inline arm64: uaccess: Move macros from futex.h to uaccess.h arm64: uaccess: Switch to capability-based {get,put}_user in PCuABI arm64: lib: Simplify copy_*_user register allocation arm64: lib: Switch to capability-based copy_*_user in PCuABI
Documentation/core-api/user_ptr.rst | 13 ++ arch/arm64/include/asm/assembler.h | 8 + arch/arm64/include/asm/futex.h | 20 +-- arch/arm64/include/asm/gpr-num.h | 6 +- arch/arm64/include/asm/morello.h | 4 - arch/arm64/include/asm/uaccess.h | 49 +++++- arch/arm64/lib/copy_from_user.S | 33 +++- arch/arm64/lib/copy_template.S | 14 +- arch/arm64/lib/copy_to_user.S | 17 +- arch/arm64/lib/morello.S | 23 --- block/blk-zoned.c | 6 +- block/ioctl.c | 22 +-- drivers/tty/tty_ioctl.c | 2 +- include/linux/blkdev.h | 8 +- include/linux/tty.h | 2 +- include/linux/user_ptr.h | 44 +++++ kernel/seccomp.c | 2 +- lib/strnlen_user.c | 163 +++++++++++++++---- tools/testing/selftests/arm64/morello/mmap.c | 2 +- 19 files changed, 319 insertions(+), 119 deletions(-)
Add generic helpers to obtain the bounds (base and limit) of a user pointer. These are useful when an algorithm wants to know whether it is possible to access memory beyond the bounds of a given object. There should be no need for them for checking in-bound accesses, as those checks are already carried out during uaccess, or explicitly via check_user_ptr_*() (which perform a full validity check, not just a bounds check).
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- Documentation/core-api/user_ptr.rst | 13 +++++++++ include/linux/user_ptr.h | 44 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 8ce09bf306b0..9db5e9271578 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -250,6 +250,19 @@ This can be done using the ``check_user_ptr_*()`` functions, see Note that these functions are no-ops (always succeed) when PCuABI is not selected, as there is no user pointer metadata to check in that case.
+Bounds +------ + +In very specific cases, it may be useful to query the lower and upper +bounds (base and limit) of a given user pointer. This is done using +``user_ptr_base(p)`` and ``user_ptr_limit(p)`` (``<linux/user_ptr.h>``). +When user pointers do not carry such information, the start and end of +the address space are returned, respectively. + +Note that the address of a user pointer should not be directly compared +with these bounds, as it may be tagged. ``untagged_addr(p)`` should be +used instead of ``user_ptr_addr(p)`` to obtain a comparable value. + Other functions handling user pointers --------------------------------------
diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 2c2180f0f0c3..685586bc0d89 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -2,6 +2,7 @@ #ifndef _LINUX_USER_PTR_H #define _LINUX_USER_PTR_H
+#include <linux/limits.h> #include <linux/typecheck.h>
/** @@ -157,6 +158,49 @@ static inline ptraddr_t user_ptr_addr(const void __user *ptr) return (ptraddr_t)(user_uintptr_t)ptr; }
+/** + * user_ptr_base() - Extract the lower bound (base) of a user pointer. + * @ptr: The user pointer to extract the base from. + * + * The base of @ptr represents the lowest address than can be accessed + * through @ptr. If @ptr does not carry any bound information, the start of the + * address space is returned. + * + * Return: The base of @ptr. + */ +static inline ptraddr_t user_ptr_base(const void __user *ptr) +{ +#ifdef CONFIG_CHERI_PURECAP_UABI + return __builtin_cheri_base_get(ptr); +#else + return 0; +#endif +} + +/** + * user_ptr_limit() - Extract the upper bound (limit) of a user pointer. + * @ptr: The user pointer to extract the limit from. + * + * The limit of @ptr represents the end of the region than can be accessed + * through @ptr (that is one byte past the highest accessible address). If @ptr + * does not carry any bound information, the end of the address space is + * returned. + * + * Return: The limit of @ptr. + */ +static inline ptraddr_t user_ptr_limit(const void __user *ptr) +{ +#ifdef CONFIG_CHERI_PURECAP_UABI + return __builtin_cheri_base_get(ptr) + __builtin_cheri_length_get(ptr); +#else + /* + * Ideally TASK_SIZE_MAX, unfortunately we cannot safely include + * <linux/uaccess.h> in this header. + */ + return ULONG_MAX; +#endif +} + /** * user_ptr_is_same() - Checks where two user pointers are exactly the same. * @p1: The first user pointer to check.
The logic to process aligned words in do_strnlen_user() is fairly complicated. This patch simplifies it a little bit, notably by moving it to its own function.
This refactoring is mainly done in preparation to a further patch that will prevent under- or over-reading the input string, when doing so is invalid.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- lib/strnlen_user.c | 73 ++++++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index e4d0f9adf5ff..60942cdfd3d2 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -7,6 +7,41 @@
#include <asm/word-at-a-time.h>
+/* + * Returns the *index* of '\0' in src, or >= max if not found. + */ +static __always_inline long find_zero_aligned(const unsigned long __user *src, + unsigned long max, + unsigned long align) +{ + const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; + long res = 0; + unsigned long c; + + unsafe_get_user(c, src++, efault); + c |= aligned_byte_mask(align); + + for (;;) { + unsigned long data; + if (has_zero(c, &data, &constants)) { + data = prep_zero_mask(c, data, &constants); + data = create_zero_mask(data); + res += find_zero(data); + break; + } + res += sizeof(unsigned long); + /* We already handled 'unsigned long' bytes. Did we do it all ? */ + if (unlikely(max <= sizeof(unsigned long))) + break; + max -= sizeof(unsigned long); + unsafe_get_user(c, src++, efault); + } + + return res; +efault: + return -EFAULT; +} + /* * Do a strnlen, return length of string *with* final '\0'. * 'count' is the user-supplied count, while 'max' is the @@ -20,11 +55,10 @@ * if it fits in a aligned 'long'. The caller needs to check * the return value against "> max". */ -static __always_inline long do_strnlen_user(const char __user *src, unsigned long count, unsigned long max) +static __always_inline long do_strnlen_user(const char __user *src, long count, unsigned long max) { - const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - unsigned long align, res = 0; - unsigned long c; + unsigned long align; + long res;
/* * Do everything aligned. But that means that we @@ -34,38 +68,21 @@ static __always_inline long do_strnlen_user(const char __user *src, unsigned lon src -= align; max += align;
- unsafe_get_user(c, (unsigned long __user *)src, efault); - c |= aligned_byte_mask(align); + res = find_zero_aligned((unsigned long __user *)src, max, align); + + if (res < 0) + return 0;
- for (;;) { - unsigned long data; - if (has_zero(c, &data, &constants)) { - data = prep_zero_mask(c, data, &constants); - data = create_zero_mask(data); - return res + find_zero(data) + 1 - align; - } - res += sizeof(unsigned long); - /* We already handled 'unsigned long' bytes. Did we do it all ? */ - if (unlikely(max <= sizeof(unsigned long))) - break; - max -= sizeof(unsigned long); - unsafe_get_user(c, (unsigned long __user *)(src+res), efault); - } res -= align;
/* - * Uhhuh. We hit 'max'. But was that the user-specified maximum - * too? If so, return the marker for "too long". + * find_zero_aligned() may end up reading more than count bytes. + * Make sure to return the marker for "too long" in that case. */ if (res >= count) return count+1;
- /* - * Nope: we hit the address space limit, and we still had more - * characters the caller would have wanted. That's 0. - */ -efault: - return 0; + return res+1; }
/**
Hi,
On 10/26/23 09:44, Kevin Brodsky wrote:
The logic to process aligned words in do_strnlen_user() is fairly complicated. This patch simplifies it a little bit, notably by moving it to its own function.
This refactoring is mainly done in preparation to a further patch that will prevent under- or over-reading the input string, when doing so is invalid.
Signed-off-by: Kevin Brodskykevin.brodsky@arm.com
lib/strnlen_user.c | 73 ++++++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index e4d0f9adf5ff..60942cdfd3d2 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -7,6 +7,41 @@ #include <asm/word-at-a-time.h> +/*
- Returns the *index* of '\0' in src, or >= max if not found.
- */
I know that there are comments on the reason for aligned vs unaligned in the function do_strnlen_user(), but maybe some info in the comment on what exactly the 'align' parameter means/does may be helpful.
+static __always_inline long find_zero_aligned(const unsigned long __user *src,
unsigned long max,unsigned long align)
On 05/01/2024 19:05, Aditya Deshpande wrote:
#include <asm/word-at-a-time.h> +/*
- Returns the *index* of '\0' in src, or >= max if not found.
- */
I know that there are comments on the reason for aligned vs unaligned in the function do_strnlen_user(), but maybe some info in the comment on what exactly the 'align' parameter means/does may be helpful.
Good point, will do!
Kevin
+static __always_inline long find_zero_aligned(const unsigned long __user *src, + unsigned long max, + unsigned long align)
The logic to process aligned words in do_strnlen_user() is fairly complicated. This patch simplifies it a little bit, notably by moving it to its own function.
This refactoring is mainly done in preparation to a further patch that will prevent under- or over-reading the input string, when doing so is invalid.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com ---
v1..v2: - Clarified what the align parameter of find_zero_aligned() means (also made it clear we return the index of a byte, not an index into src as an array of unsigned long).
Aditya, does it look better now?
lib/strnlen_user.c | 76 +++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 28 deletions(-)
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index e4d0f9adf5ff..73b7bee2480a 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -7,6 +7,44 @@
#include <asm/word-at-a-time.h>
+/* + * Returns the byte *index* of '\0' in src, or >= max if not found. + * + * align specifies the offset of the string (in bytes) in src; characters + * in the range [src, src+align) are ignored. + */ +static __always_inline long find_zero_aligned(const unsigned long __user *src, + unsigned long max, + unsigned long align) +{ + const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; + long res = 0; + unsigned long c; + + unsafe_get_user(c, src++, efault); + c |= aligned_byte_mask(align); + + for (;;) { + unsigned long data; + if (has_zero(c, &data, &constants)) { + data = prep_zero_mask(c, data, &constants); + data = create_zero_mask(data); + res += find_zero(data); + break; + } + res += sizeof(unsigned long); + /* We already handled 'unsigned long' bytes. Did we do it all ? */ + if (unlikely(max <= sizeof(unsigned long))) + break; + max -= sizeof(unsigned long); + unsafe_get_user(c, src++, efault); + } + + return res; +efault: + return -EFAULT; +} + /* * Do a strnlen, return length of string *with* final '\0'. * 'count' is the user-supplied count, while 'max' is the @@ -20,11 +58,10 @@ * if it fits in a aligned 'long'. The caller needs to check * the return value against "> max". */ -static __always_inline long do_strnlen_user(const char __user *src, unsigned long count, unsigned long max) +static __always_inline long do_strnlen_user(const char __user *src, long count, unsigned long max) { - const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; - unsigned long align, res = 0; - unsigned long c; + unsigned long align; + long res;
/* * Do everything aligned. But that means that we @@ -34,38 +71,21 @@ static __always_inline long do_strnlen_user(const char __user *src, unsigned lon src -= align; max += align;
- unsafe_get_user(c, (unsigned long __user *)src, efault); - c |= aligned_byte_mask(align); + res = find_zero_aligned((unsigned long __user *)src, max, align); + + if (res < 0) + return 0;
- for (;;) { - unsigned long data; - if (has_zero(c, &data, &constants)) { - data = prep_zero_mask(c, data, &constants); - data = create_zero_mask(data); - return res + find_zero(data) + 1 - align; - } - res += sizeof(unsigned long); - /* We already handled 'unsigned long' bytes. Did we do it all ? */ - if (unlikely(max <= sizeof(unsigned long))) - break; - max -= sizeof(unsigned long); - unsafe_get_user(c, (unsigned long __user *)(src+res), efault); - } res -= align;
/* - * Uhhuh. We hit 'max'. But was that the user-specified maximum - * too? If so, return the marker for "too long". + * find_zero_aligned() may end up reading more than count bytes. + * Make sure to return the marker for "too long" in that case. */ if (res >= count) return count+1;
- /* - * Nope: we hit the address space limit, and we still had more - * characters the caller would have wanted. That's 0. - */ -efault: - return 0; + return res+1; }
/**
strnlen_user() currently assumes that it is able to read the entire user string word by word, potentially under- or over-reading.
Unfortunately such assumption is invalid in PCuABI, as the bounds of the input user (capability) pointer may exactly match that of the string, preventing any under- or over-read, even by a few bytes.
do_strnlen_user() is therefore amended to check the bounds (base and limit) of the input pointer. If the bounds do not include the entire first or last word, the head and/or tail characters are read byte by byte (find_zero_unaligned()). The rest of the string is still read using the faster, word-by-word find_zero_aligned().
This change should be a no-op for standard ABIs (retaining the unconditional word-by-word loop), as the bounds of non-capability pointers are considered unlimited.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com ---
This patch turned out to be a lot more complex than I expected, I hope it remains mostly readable. I did some exhaustive testing of all combinations of string bounds and capability bounds for short strings (which revealed many issues along the way); this however requires adding a new syscall directly exposing strnlen_user(), which is straightforward but cannot be merged. There is nothing particularly smart about the test itself, it simply consists in a bunch of nested loops.
lib/strnlen_user.c | 104 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 11 deletions(-)
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 60942cdfd3d2..8f12259d8431 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -18,6 +18,9 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, long res = 0; unsigned long c;
+ if (max == 0) + return 0; + unsafe_get_user(c, src++, efault); c |= aligned_byte_mask(align);
@@ -42,6 +45,27 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, return -EFAULT; }
+/* + * Returns the *index* of '\0' in src, or max if not found. + */ +static __always_inline long find_zero_unaligned(const char __user *src, + unsigned long max) +{ + long res; + + for (res = 0; res < max; res++) { + char c; + + unsafe_get_user(c, src++, efault); + if (c == '\0') + break; + } + + return res; +efault: + return -EFAULT; +} + /* * Do a strnlen, return length of string *with* final '\0'. * 'count' is the user-supplied count, while 'max' is the @@ -57,23 +81,81 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, */ static __always_inline long do_strnlen_user(const char __user *src, long count, unsigned long max) { - unsigned long align; - long res; + unsigned long align, tail = 0; + long ret, res = 0; + ptraddr_t src_addr = untagged_addr(src); + ptraddr_t src_base = user_ptr_base(src); + ptraddr_t src_limit = user_ptr_limit(src);
/* - * Do everything aligned. But that means that we - * need to also expand the maximum.. + * First check that the pointer's address is within its bounds. + * If not, uaccess would fail, so return 0. Checking this now + * ensures that further calculations are valid. */ - align = (sizeof(unsigned long) - 1) & user_ptr_addr(src); - src -= align; - max += align; + if (src_base > src_addr || src_limit <= src_addr) + return 0;
- res = find_zero_aligned((unsigned long __user *)src, max, align); + align = (sizeof(unsigned long) - 1) & src_addr;
- if (res < 0) - return 0; + if (src_limit < ALIGN(src_addr + max, sizeof(unsigned long))) { + /* + * We cannot read all the words until src + max. Reduce max + * accordingly and calculate how many tail characters will need + * to be read byte by byte. + */ + max = src_limit - src_addr; + tail = (sizeof(unsigned long) - 1) & (src_addr + max); + } + + if (src_base > src_addr - align || max + align == tail) { + /* + * We cannot read the entire first aligned word, as part of it + * cannot be accessed. + */ + unsigned long head; + + if (max + align == tail) { + /* + * Less than a word can be read (see limit check above) + * - read everything byte by byte. + */ + head = max; + } else { + /* + * Read byte by byte until the next word (or return + * right away if we have already reached max). + */ + head = min(sizeof(unsigned long) - align, max); + } + + ret = find_zero_unaligned(src, head); + res += ret; + if (ret < head || max == head) + goto out; + + align = 0; + src += head; + max -= head; + } else { + /* Read the entire first aligned word, adjust max accordingly. */ + src -= align; + max += align; + }
- res -= align; + max -= tail; + ret = find_zero_aligned((unsigned long __user *)src, max, align); + res += ret - align; + if (ret < max) + goto out; + + if (tail) { + ret = find_zero_unaligned(src + max, tail); + res += ret; + } + +out: + if (ret < 0) + return 0;
/* * find_zero_aligned() may end up reading more than count bytes.
Hi,
On 10/26/23 14:14, Kevin Brodsky wrote:
strnlen_user() currently assumes that it is able to read the entire user string word by word, potentially under- or over-reading.
Unfortunately such assumption is invalid in PCuABI, as the bounds of the input user (capability) pointer may exactly match that of the string, preventing any under- or over-read, even by a few bytes.
do_strnlen_user() is therefore amended to check the bounds (base and limit) of the input pointer. If the bounds do not include the entire first or last word, the head and/or tail characters are read byte by byte (find_zero_unaligned()). The rest of the string is still read using the faster, word-by-word find_zero_aligned().
This change should be a no-op for standard ABIs (retaining the unconditional word-by-word loop), as the bounds of non-capability pointers are considered unlimited.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This patch turned out to be a lot more complex than I expected, I hope it remains mostly readable. I did some exhaustive testing of all combinations of string bounds and capability bounds for short strings (which revealed many issues along the way); this however requires adding a new syscall directly exposing strnlen_user(), which is straightforward but cannot be merged. There is nothing particularly smart about the test itself, it simply consists in a bunch of nested loops.
lib/strnlen_user.c | 104 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 11 deletions(-)
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 60942cdfd3d2..8f12259d8431 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -18,6 +18,9 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, long res = 0; unsigned long c;
- if (max == 0)
return 0;- unsafe_get_user(c, src++, efault); c |= aligned_byte_mask(align);
@@ -42,6 +45,27 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, return -EFAULT;
I suppose instead of returning negative(-EFAULT) error, returning 0 might be sufficient here. This will make the code slightly simpler as the error value is again later used for not so useful calculation.
} +/*
- Returns the *index* of '\0' in src, or max if not found.
- */
+static __always_inline long find_zero_unaligned(const char __user *src,
unsigned long max)+{
- long res;
- for (res = 0; res < max; res++) {
char c;unsafe_get_user(c, src++, efault);if (c == '\0')break;- }
- return res;
+efault:
- return -EFAULT;
Same as above.
+}
- /*
- Do a strnlen, return length of string *with* final '\0'.
- 'count' is the user-supplied count, while 'max' is the
@@ -57,23 +81,81 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, */ static __always_inline long do_strnlen_user(const char __user *src, long count, unsigned long max) {
- unsigned long align;
- long res;
- unsigned long align, tail = 0;
- long ret, res = 0;
- ptraddr_t src_addr = untagged_addr(src);
- ptraddr_t src_base = user_ptr_base(src);
- ptraddr_t src_limit = user_ptr_limit(src);
/*
* Do everything aligned. But that means that we* need to also expand the maximum..
* First check that the pointer's address is within its bounds.* If not, uaccess would fail, so return 0. Checking this now* ensures that further calculations are valid.
- align = (sizeof(unsigned long) - 1) & user_ptr_addr(src);
- src -= align;
- max += align;
- if (src_base > src_addr || src_limit <= src_addr)
return 0;
- res = find_zero_aligned((unsigned long __user *)src, max, align);
- align = (sizeof(unsigned long) - 1) & src_addr;
- if (res < 0)
return 0;
- if (src_limit < ALIGN(src_addr + max, sizeof(unsigned long))) {
/** We cannot read all the words until src + max. Reduce max* accordingly and calculate how many tail characters will need* to be read byte by byte.*/max = src_limit - src_addr;tail = (sizeof(unsigned long) - 1) & (src_addr + max);- }
- if (src_base > src_addr - align || max + align == tail) {
/** We cannot read the entire first aligned word, as part of it* cannot be accessed.*/unsigned long head;if (max + align == tail) {/** Less than a word can be read (see limit check above)* - read everything byte by byte.*/head = max;} else {/** Read byte by byte until the next word (or return* right away if we have already reached max).*/head = min(sizeof(unsigned long) - align, max);}ret = find_zero_unaligned(src, head);res += ret;if (ret < head || max == head)goto out;align = 0;src += head;max -= head;- } else {
/* Read the entire first aligned word, adjust max accordingly. */src -= align;max += align;- }
- res -= align;
- max -= tail;
- ret = find_zero_aligned((unsigned long __user *)src, max, align);
- res += ret - align;
- if (ret < max)
goto out;- if (tail) {
ret = find_zero_unaligned(src + max, tail);res += ret;- }
+out:
- if (ret < 0)
return 0;
May be:
if (!ret) return 0;
Thanks, Amit
/* * find_zero_aligned() may end up reading more than count bytes.
On 10/01/2024 10:45, Amit Daniel Kachhap wrote:
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 60942cdfd3d2..8f12259d8431 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -18,6 +18,9 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, long res = 0; unsigned long c; + if (max == 0) + return 0;
unsafe_get_user(c, src++, efault); c |= aligned_byte_mask(align); @@ -42,6 +45,27 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, return -EFAULT;
I suppose instead of returning negative(-EFAULT) error, returning 0 might be sufficient here. This will make the code slightly simpler as the error value is again later used for not so useful calculation.
There is a difference between what these helpers return and what do_strnlen_user() returns: the former return the *index* of '\0' if found (as per the comment above), while the latter returns the *length* of the string including '\0', in other words index + 1. The helpers return 0 if '\0' is found at index 0, and -EFAULT if uaccess fails.
I initially had the helpers return the length (index + 1), but I realised that it didn't work so well as this value cannot be added directly (do_strnlen_user() may call find_zero_unaligned() first, then add the return value of find_zero_aligned(), then find_zero_unaligned() again). It is possible to subtract 1 every time, but I figured it was clearer to separate out the error case and just add indices.
Hopefully that makes sense!
Kevin
On 1/10/24 18:42, Kevin Brodsky wrote:
On 10/01/2024 10:45, Amit Daniel Kachhap wrote:
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 60942cdfd3d2..8f12259d8431 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -18,6 +18,9 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, long res = 0; unsigned long c; + if (max == 0) + return 0;
unsafe_get_user(c, src++, efault); c |= aligned_byte_mask(align); @@ -42,6 +45,27 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, return -EFAULT;
I suppose instead of returning negative(-EFAULT) error, returning 0 might be sufficient here. This will make the code slightly simpler as the error value is again later used for not so useful calculation.
There is a difference between what these helpers return and what do_strnlen_user() returns: the former return the *index* of '\0' if found (as per the comment above), while the latter returns the *length* of the string including '\0', in other words index + 1. The helpers return 0 if '\0' is found at index 0, and -EFAULT if uaccess fails.
I initially had the helpers return the length (index + 1), but I realised that it didn't work so well as this value cannot be added directly (do_strnlen_user() may call find_zero_unaligned() first, then add the return value of find_zero_aligned(), then find_zero_unaligned() again). It is possible to subtract 1 every time, but I figured it was clearer to separate out the error case and just add indices.
Hopefully that makes sense!
ok. If returning 0 will not work out for error handling then your approach is fine.
Thanks, Amit
Kevin
blkdev_common_ioctl() takes both arg (unsigned long) and argp (void __user *). The latter should always be used when a pointer is expected by a command, as it has already been converted as appropriate (compat_ptr() when called from compat_blkdev_ioctl()).
A number of helpers still take arg and cast it to a void __user *, which is generally invalid. Amend them to take argp instead, saving a cast along the way.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- block/blk-zoned.c | 6 ++---- block/ioctl.c | 22 +++++++++++----------- include/linux/blkdev.h | 8 ++++---- 3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 89194cf697c6..9844a02edd52 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -330,9 +330,8 @@ static int blkdev_copy_zone_to_user(struct blk_zone *zone, unsigned int idx, * Called from blkdev_ioctl. */ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, - unsigned int cmd, user_uintptr_t arg) + unsigned int cmd, void __user *argp) { - void __user *argp = (void __user *)arg; struct zone_report_args args; struct blk_zone_report rep; int ret; @@ -383,9 +382,8 @@ static int blkdev_truncate_zone_range(struct block_device *bdev, fmode_t mode, * Called from blkdev_ioctl. */ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, - unsigned int cmd, user_uintptr_t arg) + unsigned int cmd, void __user *argp) { - void __user *argp = (void __user *)arg; struct blk_zone_range zrange; enum req_op op; int ret; diff --git a/block/ioctl.c b/block/ioctl.c index 5534aeb470c7..9fa13a9dddaf 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -83,7 +83,7 @@ static int compat_blkpg_ioctl(struct block_device *bdev, #endif
static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode, - user_uintptr_t arg) + void __user *argp) { uint64_t range[2]; uint64_t start, len; @@ -96,7 +96,7 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode, if (!bdev_max_discard_sectors(bdev)) return -EOPNOTSUPP;
- if (copy_from_user(range, (void __user *)arg, sizeof(range))) + if (copy_from_user(range, argp, sizeof(range))) return -EFAULT;
start = range[0]; @@ -152,7 +152,7 @@ static int blk_ioctl_secure_erase(struct block_device *bdev, fmode_t mode,
static int blk_ioctl_zeroout(struct block_device *bdev, fmode_t mode, - user_uintptr_t arg) + void __user *argp) { uint64_t range[2]; uint64_t start, end, len; @@ -162,7 +162,7 @@ static int blk_ioctl_zeroout(struct block_device *bdev, fmode_t mode, if (!(mode & FMODE_WRITE)) return -EBADF;
- if (copy_from_user(range, (void __user *)arg, sizeof(range))) + if (copy_from_user(range, argp, sizeof(range))) return -EFAULT;
start = range[0]; @@ -355,14 +355,14 @@ static int blkdev_flushbuf(struct block_device *bdev, fmode_t mode, }
static int blkdev_roset(struct block_device *bdev, fmode_t mode, - unsigned cmd, user_uintptr_t arg) + unsigned cmd, int __user *argp) { int ret, n;
if (!capable(CAP_SYS_ADMIN)) return -EACCES;
- if (get_user(n, (int __user *)arg)) + if (get_user(n, argp)) return -EFAULT; if (bdev->bd_disk->fops->set_read_only) { ret = bdev->bd_disk->fops->set_read_only(bdev, n); @@ -477,22 +477,22 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode, case BLKFLSBUF: return blkdev_flushbuf(bdev, mode, cmd, arg); case BLKROSET: - return blkdev_roset(bdev, mode, cmd, arg); + return blkdev_roset(bdev, mode, cmd, argp); case BLKDISCARD: - return blk_ioctl_discard(bdev, mode, arg); + return blk_ioctl_discard(bdev, mode, argp); case BLKSECDISCARD: return blk_ioctl_secure_erase(bdev, mode, argp); case BLKZEROOUT: - return blk_ioctl_zeroout(bdev, mode, arg); + return blk_ioctl_zeroout(bdev, mode, argp); case BLKGETDISKSEQ: return put_u64(argp, bdev->bd_disk->diskseq); case BLKREPORTZONE: - return blkdev_report_zones_ioctl(bdev, mode, cmd, arg); + return blkdev_report_zones_ioctl(bdev, mode, cmd, argp); case BLKRESETZONE: case BLKOPENZONE: case BLKCLOSEZONE: case BLKFINISHZONE: - return blkdev_zone_mgmt_ioctl(bdev, mode, cmd, arg); + return blkdev_zone_mgmt_ioctl(bdev, mode, cmd, argp); case BLKGETZONESZ: return put_uint(argp, bdev_zone_sectors(bdev)); case BLKGETNRZONES: diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 7630916c1471..128fdac6e9a8 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -330,9 +330,9 @@ int blk_revalidate_disk_zones(struct gendisk *disk, void (*update_driver_data)(struct gendisk *disk));
extern int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, - unsigned int cmd, unsigned long arg); + unsigned int cmd, void __user *argp); extern int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, - unsigned int cmd, unsigned long arg); + unsigned int cmd, void __user *argp);
#else /* CONFIG_BLK_DEV_ZONED */
@@ -343,14 +343,14 @@ static inline unsigned int bdev_nr_zones(struct block_device *bdev)
static inline int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, - unsigned long arg) + void __user *argp) { return -ENOTTY; }
static inline int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, - unsigned long arg) + void __user *argp) { return -ENOTTY; }
Commit "tty: Modify ioctl to use user_uintptr_t" converted most of the tty ioctl handlers and helpers to propagate the argument as user_uintptr_t instead of unsigned long. n_tty_ioctl_helper() was however missed, and as a result of number of commands receive a truncated pointer in PCuABI.
Switch to representing arg as user_uintptr_t in n_tty_ioctl_helper() as well, so that a pointer argument is fully propagated in PCuABI.
Fixes: ("tty: Modify ioctl to use user_uintptr_t") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- drivers/tty/tty_ioctl.c | 2 +- include/linux/tty.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 79fa230994f2..7a7705ee0cb2 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -954,7 +954,7 @@ int tty_perform_flush(struct tty_struct *tty, unsigned long arg) EXPORT_SYMBOL_GPL(tty_perform_flush);
int n_tty_ioctl_helper(struct tty_struct *tty, unsigned int cmd, - unsigned long arg) + user_uintptr_t arg) { int retval;
diff --git a/include/linux/tty.h b/include/linux/tty.h index a2752f8f1cdd..86797ca61512 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -497,7 +497,7 @@ static inline int tty_audit_push(void)
/* tty_ioctl.c */ int n_tty_ioctl_helper(struct tty_struct *tty, unsigned int cmd, - unsigned long arg); + user_uintptr_t arg);
/* vt.c */
struct sock_fprog includes a pointer to the user filter. In order to preserve such user pointers in PCuABI, copy_from_user_with_ptr() must be used.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- kernel/seccomp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 2f52ea7fa34a..12faa617c8da 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -703,7 +703,7 @@ seccomp_prepare_user_filter(const char __user *user_filter) fprog.filter = compat_ptr(fprog32.filter); } else /* falls through to the if below. */ #endif - if (copy_from_user(&fprog, user_filter, sizeof(fprog))) + if (copy_from_user_with_ptr(&fprog, user_filter, sizeof(fprog))) goto out; filter = seccomp_prepare_filter(&fprog); out:
sizeof(msg) only gives the expected answer if msg is an array, not if it is a pointer. Since msg happens to be smaller than a pointer (4 vs 16 in PCuABI), the write() syscall below ends up reading out of bounds (unless prevented by the capability bounds being too narrow).
Fixes: ("kselftests/arm64: morello: Add pcuabi tests for mmap & co") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- tools/testing/selftests/arm64/morello/mmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 5d259c336f55..62fc14d14d46 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -63,7 +63,7 @@ void syscall_mmap(void) static inline __attribute__((always_inline)) void syscall_mmap2(void) { - const char *msg = "foo"; + const char msg[] = "foo"; unsigned int msg_len = sizeof(msg); /* No need for the terminator */ const char *sample_file = "/limbo.dat"; void *addr;
We no longer need to have standalone __morello_{get,put}_user_cap_asm() helpers; we can implement the access to user capabilities inline, on the model of __{get,put}_mem_asm(). We cannot directly use the latter though, as a different constraint is required for the capability to load/store.
There is currently no way to get Clang to output the X register corresponding to a capability argument, so we also need to add .L__gpr_num_* aliases for C registers, in order to keep the EX_DATA_REG() macro happy.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/gpr-num.h | 6 +++++- arch/arm64/include/asm/morello.h | 4 ---- arch/arm64/include/asm/uaccess.h | 16 ++++++++++++++-- arch/arm64/lib/morello.S | 23 ----------------------- 4 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/arch/arm64/include/asm/gpr-num.h b/arch/arm64/include/asm/gpr-num.h index 05da4a7c5788..bac15df6242f 100644 --- a/arch/arm64/include/asm/gpr-num.h +++ b/arch/arm64/include/asm/gpr-num.h @@ -7,9 +7,11 @@ .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30 .equ .L__gpr_num_x\num, \num .equ .L__gpr_num_w\num, \num + .equ .L__gpr_num_c\num, \num .endr .equ .L__gpr_num_xzr, 31 .equ .L__gpr_num_wzr, 31 + .equ .L__gpr_num_czr, 31
#else /* __ASSEMBLY__ */
@@ -17,9 +19,11 @@ " .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" \ " .equ .L__gpr_num_x\num, \num\n" \ " .equ .L__gpr_num_w\num, \num\n" \ +" .equ .L__gpr_num_c\num, \num\n" \ " .endr\n" \ " .equ .L__gpr_num_xzr, 31\n" \ -" .equ .L__gpr_num_wzr, 31\n" +" .equ .L__gpr_num_wzr, 31\n" \ +" .equ .L__gpr_num_czr, 31\n"
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 68ec2d4767a2..357f403d073d 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -47,10 +47,6 @@ int morello_ptrace_access_remote_cap(struct task_struct *tsk, struct user_cap *user_cap, unsigned int gup_flags);
-/* Low-level uacces helpers, must not be called directly */ -void __morello_get_user_cap_asm(uintcap_t *x, const uintcap_t __user *ptr, int *err); -void __morello_put_user_cap_asm(const uintcap_t *x, uintcap_t __user *ptr, int *err); - #ifdef CONFIG_CHERI_PURECAP_UABI void morello_thread_set_csp(struct pt_regs *regs, user_uintptr_t sp); #endif diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 6a7a323cd36b..cf12e70baa98 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -436,7 +436,13 @@ do { \ do { \ __chk_user_ptr(ptr); \ uaccess_ttbr0_enable(); \ - __morello_get_user_cap_asm(&(x), (ptr), &(err)); \ + asm volatile( \ + "1: ldtr %1, [%2]\n" \ + "2:\n" \ + _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \ + : "+r" (err), "=C" (x) \ + /* TODO [PCuABI] - perform the access via the user capability */\ + : "r" ((ptraddr_t)(user_uintptr_t)(ptr))); \ uaccess_ttbr0_disable(); \ } while (0)
@@ -466,7 +472,13 @@ do { \ do { \ __chk_user_ptr(ptr); \ uaccess_ttbr0_enable(); \ - __morello_put_user_cap_asm(&(x), (ptr), &(err)); \ + asm volatile( \ + "1: sttr %1, [%2]\n" \ + "2:\n" \ + _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %w0) \ + : "+r" (err) \ + /* TODO [PCuABI] - perform the access via the user capability */\ + : "CZ" (x), "r" ((ptraddr_t)(user_uintptr_t)(ptr))); \ uaccess_ttbr0_disable(); \ } while (0)
diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S index 04ef653380d2..0800375ef33d 100644 --- a/arch/arm64/lib/morello.S +++ b/arch/arm64/lib/morello.S @@ -177,29 +177,6 @@ SYM_FUNC_START(morello_task_restore_user_tls) ret SYM_FUNC_END(morello_task_restore_user_tls)
-SYM_FUNC_START(__morello_get_user_cap_asm) - user_ldst 1f, ldtr, c3, x1, 0 - str c3, [x0] - ret - - // Exception fixups -1: mov w3, #-EFAULT - str w3, [x2] - str czr, [x0] - ret -SYM_FUNC_END(__morello_get_user_cap_asm) - -SYM_FUNC_START(__morello_put_user_cap_asm) - ldr c3, [x0] - user_ldst 1f, sttr, c3, x1, 0 - ret - - // Exception fixups -1: mov w3, #-EFAULT - str w3, [x2] - ret -SYM_FUNC_END(__morello_put_user_cap_asm) -
SYM_FUNC_START(__morello_cap_lo_hi_tag) str x0, [x1]
In preparation of enabling capability-based accesses in get_user/put_user, move the PCuABI-related macros from futex.h to uaccess.h (the former includes the latter). Also give the A64/C64 macros a more generic name, which is going to be useful in uaccess.h to support kernel and user accesses in the same macro.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/futex.h | 20 ++++---------------- arch/arm64/include/asm/uaccess.h | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h index 2ef9b5167007..c128e4c2886e 100644 --- a/arch/arm64/include/asm/futex.h +++ b/arch/arm64/include/asm/futex.h @@ -10,18 +10,6 @@
#include <asm/errno.h>
-#ifdef CONFIG_CHERI_PURECAP_UABI -#define __ASM_SWITCH_TO_C64 " bx #4\n" \ - ".arch morello+c64\n" -#define __ASM_SWITCH_TO_A64 " bx #4\n" \ - ".arch morello\n" -#define __ASM_RW_UPTR_CONSTR "+C" -#else -#define __ASM_SWITCH_TO_C64 -#define __ASM_SWITCH_TO_A64 -#define __ASM_RW_UPTR_CONSTR "+r" -#endif - #define FUTEX_MAX_LOOPS 128 /* What's the largest number you can think of? */
#define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg) \ @@ -30,7 +18,7 @@ do { \ \ uaccess_enable_privileged(); \ asm volatile( \ - __ASM_SWITCH_TO_C64 \ + __ASM_UACCESS_BEFORE \ " prfm pstl1strm, [%2]\n" \ "1: ldxr %w1, [%2]\n" \ insn "\n" \ @@ -41,7 +29,7 @@ do { \ " mov %w0, %w6\n" \ "3:\n" \ " dmb ish\n" \ - __ASM_SWITCH_TO_A64 \ + __ASM_UACCESS_AFTER \ _ASM_EXTABLE_UACCESS_ERR(1b, 3b, %w0) \ _ASM_EXTABLE_UACCESS_ERR(2b, 3b, %w0) \ /* TODO [PCuABI]: temporary solution for uaddr. Should be reverted to +Q @@ -108,7 +96,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr, uaddr = __uaccess_mask_ptr(_uaddr); uaccess_enable_privileged(); asm volatile("// futex_atomic_cmpxchg_inatomic\n" - __ASM_SWITCH_TO_C64 + __ASM_UACCESS_BEFORE " prfm pstl1strm, [%2]\n" "1: ldxr %w1, [%2]\n" " sub %w3, %w1, %w5\n" @@ -121,7 +109,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr, "3:\n" " dmb ish\n" "4:\n" - __ASM_SWITCH_TO_A64 + __ASM_UACCESS_AFTER _ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0) _ASM_EXTABLE_UACCESS_ERR(2b, 4b, %w0) /* TODO [PCuABI]: temporary solution for uaddr. Should be reverted to +Q once diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index cf12e70baa98..c28850a79aad 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -27,6 +27,21 @@ #include <asm/memory.h> #include <asm/extable.h>
+#ifdef CONFIG_CHERI_PURECAP_UABI +#define __ASM_SWITCH_TO_C64 " bx #4\n" \ + ".arch morello+c64\n" +#define __ASM_SWITCH_TO_A64 " bx #4\n" \ + ".arch morello\n" +#define __ASM_RW_UPTR_CONSTR "+C" +#else +#define __ASM_SWITCH_TO_C64 +#define __ASM_SWITCH_TO_A64 +#define __ASM_RW_UPTR_CONSTR "+r" +#endif + +#define __ASM_UACCESS_BEFORE __ASM_SWITCH_TO_C64 +#define __ASM_UACCESS_AFTER __ASM_SWITCH_TO_A64 + static inline int __access_ok(const void __user *ptr, unsigned long size);
/*
In PCuABI, {get,put}_user currently access user memory via standard 64-bit pointers. This patch switches to capability-based accesses: instead of extracting the address of the input capability, it is directly used to make the access. As a result, {get,put}_user validate the capability metadata, failing with -EFAULT if the hardware checks fail (causing a capability exception).
Unprivileged load and store instructions operating on a capability base register (e.g. ldtr/sttr x0, [c1]) are only available in the C64 ISA, we therefore need to switch to C64 before executing them (and then return to A64). We also need to use the "C" asm constraint for capability registers. This requirement has recently disappeared in Clang, but for backwards-compatibility we stick to using that constraint for now.
Because __{get,put}_mem_asm can operate on either a kernel or user pointer, fallback macros are defined for the kernel case. The Morello-specific variants that load or store a capability (__morello_raw_{get,put}_user_cap) are only passed user pointers, so the user macros can directly be used there.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/uaccess.h | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index c28850a79aad..5b6134066b7e 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -32,15 +32,21 @@ ".arch morello+c64\n" #define __ASM_SWITCH_TO_A64 " bx #4\n" \ ".arch morello\n" +#define __ASM_RO_UPTR_CONSTR "C" #define __ASM_RW_UPTR_CONSTR "+C" #else #define __ASM_SWITCH_TO_C64 #define __ASM_SWITCH_TO_A64 +#define __ASM_RO_UPTR_CONSTR "r" #define __ASM_RW_UPTR_CONSTR "+r" #endif
#define __ASM_UACCESS_BEFORE __ASM_SWITCH_TO_C64 #define __ASM_UACCESS_AFTER __ASM_SWITCH_TO_A64 +#define __ASM_KACCESS_BEFORE +#define __ASM_KACCESS_AFTER +#define __ASM_RO_KPTR_CONSTR "r" +#define __ASM_RW_KPTR_CONSTR "+r"
static inline int __access_ok(const void __user *ptr, unsigned long size);
@@ -217,12 +223,13 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr) */ #define __get_mem_asm(load, reg, x, addr, err, type) \ asm volatile( \ + __ASM_##type##ACCESS_BEFORE \ "1: " load " " reg "1, [%2]\n" \ "2:\n" \ + __ASM_##type##ACCESS_AFTER \ _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \ : "+r" (err), "=r" (x) \ - /* TODO [PCuABI] - perform the access via the user capability */\ - : "r" ((ptraddr_t)(user_uintptr_t)(addr))) + : __ASM_RO_##type##PTR_CONSTR (addr))
#define __raw_get_mem(ldr, x, ptr, err, type) \ do { \ @@ -307,12 +314,13 @@ do { \
#define __put_mem_asm(store, reg, x, addr, err, type) \ asm volatile( \ + __ASM_##type##ACCESS_BEFORE \ "1: " store " " reg "1, [%2]\n" \ "2:\n" \ + __ASM_##type##ACCESS_AFTER \ _ASM_EXTABLE_##type##ACCESS_ERR(1b, 2b, %w0) \ : "+r" (err) \ - /* TODO [PCuABI] - perform the access via the user capability */\ - : "rZ" (x), "r" ((ptraddr_t)(user_uintptr_t)(addr))) + : "rZ" (x), __ASM_RO_##type##PTR_CONSTR (addr))
#define __raw_put_mem(str, x, ptr, err, type) \ do { \ @@ -452,12 +460,13 @@ do { \ __chk_user_ptr(ptr); \ uaccess_ttbr0_enable(); \ asm volatile( \ + __ASM_UACCESS_BEFORE \ "1: ldtr %1, [%2]\n" \ "2:\n" \ + __ASM_UACCESS_AFTER \ _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \ : "+r" (err), "=C" (x) \ - /* TODO [PCuABI] - perform the access via the user capability */\ - : "r" ((ptraddr_t)(user_uintptr_t)(ptr))); \ + : __ASM_RO_UPTR_CONSTR (ptr)); \ uaccess_ttbr0_disable(); \ } while (0)
@@ -488,12 +497,13 @@ do { \ __chk_user_ptr(ptr); \ uaccess_ttbr0_enable(); \ asm volatile( \ + __ASM_UACCESS_BEFORE \ "1: sttr %1, [%2]\n" \ "2:\n" \ + __ASM_UACCESS_AFTER \ _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %w0) \ : "+r" (err) \ - /* TODO [PCuABI] - perform the access via the user capability */\ - : "CZ" (x), "r" ((ptraddr_t)(user_uintptr_t)(ptr))); \ + : "CZ" (x), __ASM_RO_UPTR_CONSTR (ptr)); \ uaccess_ttbr0_disable(); \ } while (0)
Hi,
On 10/26/23 09:45, Kevin Brodsky wrote:
In PCuABI, {get,put}_user currently access user memory via standard 64-bit pointers. This patch switches to capability-based accesses: instead of extracting the address of the input capability, it is directly used to make the access. As a result, {get,put}_user validate the capability metadata, failing with -EFAULT if the hardware checks fail (causing a capability exception).
Unprivileged load and store instructions operating on a capability base register (e.g. ldtr/sttr x0, [c1]) are only available in the C64 ISA, we therefore need to switch to C64 before executing them (and then return to A64). We also need to use the "C" asm constraint for capability registers. This requirement has recently disappeared in Clang, but for backwards-compatibility we stick to using that constraint for now.
Because __{get,put}_mem_asm can operate on either a kernel or user pointer, fallback macros are defined for the kernel case. The Morello-specific variants that load or store a capability (__morello_raw_{get,put}_user_cap) are only passed user pointers, so the user macros can directly be used there.
Signed-off-by: Kevin Brodskykevin.brodsky@arm.com
arch/arm64/include/asm/uaccess.h | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index c28850a79aad..5b6134066b7e 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -32,15 +32,21 @@ ".arch morello+c64\n" #define __ASM_SWITCH_TO_A64 " bx #4\n" \ ".arch morello\n" +#define __ASM_RO_UPTR_CONSTR "C" #define __ASM_RW_UPTR_CONSTR "+C" #else #define __ASM_SWITCH_TO_C64 #define __ASM_SWITCH_TO_A64 +#define __ASM_RO_UPTR_CONSTR "r" #define __ASM_RW_UPTR_CONSTR "+r" #endif #define __ASM_UACCESS_BEFORE __ASM_SWITCH_TO_C64 #define __ASM_UACCESS_AFTER __ASM_SWITCH_TO_A64 +#define __ASM_KACCESS_BEFORE +#define __ASM_KACCESS_AFTER +#define __ASM_RO_KPTR_CONSTR "r" +#define __ASM_RW_KPTR_CONSTR "+r"
Would it be better to move the assembly constraint macros to a more generic header? We'll need macros like these anytime we handle assembly for purecap vs aarch64; for example we defined similar macros for the vDSO fallback syscalls. Having them in a distinct header would cut down on duplication.
Best, Aditya
On 05/01/2024 19:05, Aditya Deshpande wrote:
+#define __ASM_RO_UPTR_CONSTR "C" #define __ASM_RW_UPTR_CONSTR "+C" #else #define __ASM_SWITCH_TO_C64 #define __ASM_SWITCH_TO_A64 +#define __ASM_RO_UPTR_CONSTR "r" #define __ASM_RW_UPTR_CONSTR "+r" #endif #define __ASM_UACCESS_BEFORE __ASM_SWITCH_TO_C64 #define __ASM_UACCESS_AFTER __ASM_SWITCH_TO_A64 +#define __ASM_KACCESS_BEFORE +#define __ASM_KACCESS_AFTER +#define __ASM_RO_KPTR_CONSTR "r" +#define __ASM_RW_KPTR_CONSTR "+r"
Would it be better to move the assembly constraint macros to a more generic header? We'll need macros like these anytime we handle assembly for purecap vs aarch64; for example we defined similar macros for the vDSO fallback syscalls. Having them in a distinct header would cut down on duplication.
If the long-term plan was to keep using "C" then yes I'd agree. However Clang now supports "r" for capabilities too [1], and the latest release has this improvement, so we can soon move to removing these macros altogether. We'll still need the A64/C64 ones, but those shouldn't be needed beyond uaccess.
Kevin
[1] https://git.morello-project.org/morello/llvm-project/-/merge_requests/258
copy_template currently copies the destination pointer to x6 then operates on that copy, while it directly operates on the source pointer (x1). In both cases a copy of the original pointer is required for the final fixup in copy_*_user, but not in copy_template itself.
Make things a little easier to follow by saving both pointers in copy_*_user, letting copy_template operate on the original pointers (x0 and x1) directly.
While at it, remove the "Returns" comment in copy_template, which is irrelevant.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/lib/copy_from_user.S | 2 ++ arch/arm64/lib/copy_template.S | 6 +----- arch/arm64/lib/copy_to_user.S | 2 ++ 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S index 2445663b4466..46dc57cd8d34 100644 --- a/arch/arm64/lib/copy_from_user.S +++ b/arch/arm64/lib/copy_from_user.S @@ -65,9 +65,11 @@ .endm
end .req x5 +dstin .req x6 srcin .req x15 SYM_FUNC_START(COPY_FUNC_NAME) add end, x0, x2 + mov dstin, x0 mov srcin, x1 #include "copy_template.S" mov x0, #0 // Nothing to copy diff --git a/arch/arm64/lib/copy_template.S b/arch/arm64/lib/copy_template.S index ace9f2dde856..1c54c338d078 100644 --- a/arch/arm64/lib/copy_template.S +++ b/arch/arm64/lib/copy_template.S @@ -18,17 +18,14 @@ * x0 - dest * x1 - src * x2 - n - * Returns: - * x0 - dest */ -dstin .req x0 +dst .req x0 src .req x1 count .req x2 tmp1 .req x3 tmp1w .req w3 tmp2 .req x4 tmp2w .req w4 -dst .req x6
A_l .req x7 A_h .req x8 @@ -49,7 +46,6 @@ Bc_l .req c9 Bc_h .req c10 #endif
- mov dst, dstin cmp count, #16 /*When memory length is less than 16, the accessed are not aligned.*/ b.lo .Ltiny15 diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S index 9f6aed913750..a501e7665e77 100644 --- a/arch/arm64/lib/copy_to_user.S +++ b/arch/arm64/lib/copy_to_user.S @@ -64,9 +64,11 @@ .endm
end .req x5 +dstin .req x6 srcin .req x15 SYM_FUNC_START(COPY_FUNC_NAME) add end, x0, x2 + mov dstin, x0 mov srcin, x1 #include "copy_template.S" mov x0, #0
We have already amended {get,put}_user so that they access user memory directly via the user capability in PCuABI; now is time to convert copy_*_user too. As a result, the copy will be aborted if the user capability is unsuitable to perform the access (potentially partway, in case the tail of the targeted region is out of bounds).
__arch_copy_{from,to}_user present an additional challenge in that they are implemented fully in assembly. Fortunately, the registers holding the source and destination pointers are mostly used as base registers for load/store instructions. After switching to C64, such instructions operate on C registers instead of X, so it becomes simply a matter of modifying the register aliases in PCuABI; the req_reg_pcuabi macro is introduced for that purpose. Explicit ADD instructions are also used in the user_{ldst,ldp,stp} helpers; those are unproblematic as they can operate on both X and C registers.
In the few situations where the pointers are being somehow inspected, we keep operating on their address only, by using the corresponding X register; srcx and dstx are introduced for that purpose. This is necessary in some cases due to the instruction simply not accepting C registers (e.g. TST), while in others it is rather a matter of convenience, as it means we don't need to convert additional register aliases to capabilities (CMP, SUB).
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/assembler.h | 8 ++++++++ arch/arm64/lib/copy_from_user.S | 31 +++++++++++++++++++++++++++--- arch/arm64/lib/copy_template.S | 10 ++++++---- arch/arm64/lib/copy_to_user.S | 15 ++++++++++++--- 4 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 6b31eb7687ba..7d4046ded4df 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -34,6 +34,14 @@ wx\n .req w\n .endr
+ .macro req_reg_pcuabi, name, pcuabi_reg, default_reg +#ifdef CONFIG_CHERI_PURECAP_UABI + \name .req \pcuabi_reg +#else + \name .req \default_reg +#endif + .endm + .macro disable_daif msr daifset, #0xf .endm diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S index 46dc57cd8d34..d35126460624 100644 --- a/arch/arm64/lib/copy_from_user.S +++ b/arch/arm64/lib/copy_from_user.S @@ -66,22 +66,47 @@
end .req x5 dstin .req x6 -srcin .req x15 +req_reg_pcuabi srcin, c15, x15 SYM_FUNC_START(COPY_FUNC_NAME) add end, x0, x2 mov dstin, x0 +#ifdef CONFIG_CHERI_PURECAP_UABI +.arch morello+c64 + bx #4 + /* + * Having switched to C64, argumentless RET is equivalent to RET CLR. + * Because we have been called from A64, only LR is set. We therefore + * set CLR to a valid capability, derived from PCC (as if we had been + * called from C64). Conveniently this will also automatically switch + * us back to A64 when returning (as the LSB of LR should be unset). + */ + cvtp clr, lr + /* + * Accessing memory via X registers in C64 requires using + * alternate-base loads and stores; unfortunately most loads and stores + * used in copy_template.S do not have an alternate-base counterpart. + * The most straightforward solution is to access memory via C + * registers only. We therefore need to create a valid capability for + * the kernel buffer too, which is done by deriving it from DDC. Since + * X-based accesses are validated against DDC, this is functionally + * equivalent. + */ + cvtd c0, x0 + mov srcin, c1 +#else mov srcin, x1 +#endif #include "copy_template.S" mov x0, #0 // Nothing to copy ret
// Exception fixups -9997: cmp dst, dstin +9997: cmp dstx, dstin b.ne 9998f // Before being absolutely sure we couldn't copy anything, try harder USER(9998f, ldtrb tmp1w, [srcin]) strb tmp1w, [dst], #1 -9998: sub x0, end, dst // bytes not copied +9998: sub x0, end, dstx // bytes not copied ret SYM_FUNC_END(COPY_FUNC_NAME) EXPORT_SYMBOL(COPY_FUNC_NAME) diff --git a/arch/arm64/lib/copy_template.S b/arch/arm64/lib/copy_template.S index 1c54c338d078..a07fa44d4c99 100644 --- a/arch/arm64/lib/copy_template.S +++ b/arch/arm64/lib/copy_template.S @@ -19,8 +19,10 @@ * x1 - src * x2 - n */ -dst .req x0 -src .req x1 +req_reg_pcuabi dst, c0, x0 +dstx .req x0 +req_reg_pcuabi src, c1, x1 +srcx .req x1 count .req x2 tmp1 .req x3 tmp1w .req w3 @@ -50,7 +52,7 @@ Bc_h .req c10 /*When memory length is less than 16, the accessed are not aligned.*/ b.lo .Ltiny15
- neg tmp2, src + neg tmp2, srcx ands tmp2, tmp2, #15/* Bytes to reach alignment. */ b.eq .LSrcAligned sub count, count, tmp2 @@ -79,7 +81,7 @@ Bc_h .req c10 .LSrcAligned: #ifdef COPY_CAPTAGS /* src now 16-byte aligned, copy capability tags if dst also aligned */ - tst dst, #15 + tst dstx, #15 b.eq .LSrcAligned_cpycaps #endif cmp count, #64 diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S index a501e7665e77..316349701edf 100644 --- a/arch/arm64/lib/copy_to_user.S +++ b/arch/arm64/lib/copy_to_user.S @@ -65,23 +65,32 @@
end .req x5 dstin .req x6 -srcin .req x15 +req_reg_pcuabi srcin, c15, x15 SYM_FUNC_START(COPY_FUNC_NAME) add end, x0, x2 mov dstin, x0 +#ifdef CONFIG_CHERI_PURECAP_UABI +.arch morello+c64 + bx #4 + /* See comments in copy_from_user.S */ + cvtp clr, lr + cvtd c1, x1 + mov srcin, c1 +#else mov srcin, x1 +#endif #include "copy_template.S" mov x0, #0 ret
// Exception fixups -9997: cmp dst, dstin +9997: cmp dstx, dstin b.ne 9998f // Before being absolutely sure we couldn't copy anything, try harder ldrb tmp1w, [srcin] USER(9998f, sttrb tmp1w, [dst]) add dst, dst, #1 -9998: sub x0, end, dst // bytes not copied +9998: sub x0, end, dstx // bytes not copied ret SYM_FUNC_END(COPY_FUNC_NAME) EXPORT_SYMBOL(COPY_FUNC_NAME)
Hi Kevin,
Thank you for the patch series. Overall it LGTM - I have only left a couple of minor comments!
I built an image with this patch series and put it on the Morello board and verified it worked using the Morello selftests.
Best, Aditya
On 10/26/23 09:44, Kevin Brodsky wrote:
Hi,
This series switches all uaccess routines to operate directly on the user-provided capability pointer in PCuABI, instead of making the access via the extracted 64-bit address. This means that all forms of uaccess now enforce the capability metadata, through the use of capability-based loads and stores, and will fail with -EFAULT following a failed (hardware) capability check. The impacted routines are:
- *get_user* (including get_user_ptr)
- *put_user* (including put_user_ptr)
- *copy_from_user* (including copy_from_user_with_ptr)
- *copy_to_user* (including copy_to_user_with_ptr)
(Note that futex operations are already checked thanks to Luca's work [1].)
Enabling capability checks in uaccess exposed a variety of issues, both in the kernel and in userspace. This series addresses the fallout in the kernel before making the switch:
- Patch 1-3 overhaul strnlen_user() to prevent it from reading beyond the bounds of the user pointer, while leaving its implementation unchanged in !PCuABI.
- Patch 4-6 fixes various capability propagation issues (surprisingly few of those!).
- Patch 7 fixes a bug in a Morello kselftest, detected through capability checking in copy_from_user().
Finally, the switch to capability-based accesses is made:
- Patch 8-10 switch {get,put}_user and variants.
- Patch 11-12 switch copy_{from,to}_user and variants.
After this series, the Morello kselftests and Musl tests still pass, and so do the liburing tests, however the LTP syscalls suite doesn't because of various issues in LTP itself. I have addressed a number of them upstream [2] (series already merged), and will post the remaining fixes to linux-morello-ltp.
Because this precise uaccess checking cannot be achieved with most existing sanitisers or hardware memory safety mechanisms, further bugs are likely to be identified in low-level libraries and test suites running in purecap (I have spotted a couple in the Bionic tests).
Issue:
https://git.morello-project.org/morello/kernel/linux/-/issues/5
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/checked...
Cheers, Kevin
[1]https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... [2]https://lore.kernel.org/ltp/20231023135647.2157030-1-kevin.brodsky@arm.com/
Kevin Brodsky (12): linux/user_ptr.h: Introduce user_ptr_{base,limit} lib: Refactor do_strnlen_user() lib: Make strnlen_user() PCuABI-friendly block: Eliminate invalid user pointer casts tty: Convert another ioctl helper to take arg as user_uintptr_t seccomp: Preserve pointers when reading sock_fprog from userspace kselftests/arm64: morello: Fix message size in mmap test arm64: morello: Implement user capability accessors inline arm64: uaccess: Move macros from futex.h to uaccess.h arm64: uaccess: Switch to capability-based {get,put}_user in PCuABI arm64: lib: Simplify copy_*_user register allocation arm64: lib: Switch to capability-based copy_*_user in PCuABI
Documentation/core-api/user_ptr.rst | 13 ++ arch/arm64/include/asm/assembler.h | 8 + arch/arm64/include/asm/futex.h | 20 +-- arch/arm64/include/asm/gpr-num.h | 6 +- arch/arm64/include/asm/morello.h | 4 - arch/arm64/include/asm/uaccess.h | 49 +++++- arch/arm64/lib/copy_from_user.S | 33 +++- arch/arm64/lib/copy_template.S | 14 +- arch/arm64/lib/copy_to_user.S | 17 +- arch/arm64/lib/morello.S | 23 --- block/blk-zoned.c | 6 +- block/ioctl.c | 22 +-- drivers/tty/tty_ioctl.c | 2 +- include/linux/blkdev.h | 8 +- include/linux/tty.h | 2 +- include/linux/user_ptr.h | 44 +++++ kernel/seccomp.c | 2 +- lib/strnlen_user.c | 163 +++++++++++++++---- tools/testing/selftests/arm64/morello/mmap.c | 2 +- 19 files changed, 319 insertions(+), 119 deletions(-)
On 05/01/2024 19:05, Aditya Deshpande wrote:
Thank you for the patch series. Overall it LGTM - I have only left a couple of minor comments!
I built an image with this patch series and put it on the Morello board and verified it worked using the Morello selftests.
Thank you for the review and testing, very appreciated!
Kevin
Hi,
On 10/26/23 14:14, Kevin Brodsky wrote:
Hi,
This series switches all uaccess routines to operate directly on the user-provided capability pointer in PCuABI, instead of making the access via the extracted 64-bit address. This means that all forms of uaccess now enforce the capability metadata, through the use of capability-based loads and stores, and will fail with -EFAULT following a failed (hardware) capability check. The impacted routines are:
- *get_user* (including get_user_ptr)
- *put_user* (including put_user_ptr)
- *copy_from_user* (including copy_from_user_with_ptr)
- *copy_to_user* (including copy_to_user_with_ptr)
(Note that futex operations are already checked thanks to Luca's work [1].)
Enabling capability checks in uaccess exposed a variety of issues, both in the kernel and in userspace. This series addresses the fallout in the kernel before making the switch:
- Patch 1-3 overhaul strnlen_user() to prevent it from reading beyond the bounds of the user pointer, while leaving its implementation unchanged in !PCuABI.
- Patch 4-6 fixes various capability propagation issues (surprisingly few of those!).
- Patch 7 fixes a bug in a Morello kselftest, detected through capability checking in copy_from_user().
Finally, the switch to capability-based accesses is made:
- Patch 8-10 switch {get,put}_user and variants.
- Patch 11-12 switch copy_{from,to}_user and variants.
After this series, the Morello kselftests and Musl tests still pass, and so do the liburing tests, however the LTP syscalls suite doesn't because of various issues in LTP itself. I have addressed a number of them upstream [2] (series already merged), and will post the remaining fixes to linux-morello-ltp.
Because this precise uaccess checking cannot be achieved with most existing sanitisers or hardware memory safety mechanisms, further bugs are likely to be identified in low-level libraries and test suites running in purecap (I have spotted a couple in the Bionic tests).
The full series looks good to me except a minor comment in Patch 3.
Thanks, Amit
Issue:
https://git.morello-project.org/morello/kernel/linux/-/issues/5
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/checked...
Cheers, Kevin
[1] https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... [2] https://lore.kernel.org/ltp/20231023135647.2157030-1-kevin.brodsky@arm.com/
Kevin Brodsky (12): linux/user_ptr.h: Introduce user_ptr_{base,limit} lib: Refactor do_strnlen_user() lib: Make strnlen_user() PCuABI-friendly block: Eliminate invalid user pointer casts tty: Convert another ioctl helper to take arg as user_uintptr_t seccomp: Preserve pointers when reading sock_fprog from userspace kselftests/arm64: morello: Fix message size in mmap test arm64: morello: Implement user capability accessors inline arm64: uaccess: Move macros from futex.h to uaccess.h arm64: uaccess: Switch to capability-based {get,put}_user in PCuABI arm64: lib: Simplify copy_*_user register allocation arm64: lib: Switch to capability-based copy_*_user in PCuABI
Documentation/core-api/user_ptr.rst | 13 ++ arch/arm64/include/asm/assembler.h | 8 + arch/arm64/include/asm/futex.h | 20 +-- arch/arm64/include/asm/gpr-num.h | 6 +- arch/arm64/include/asm/morello.h | 4 - arch/arm64/include/asm/uaccess.h | 49 +++++- arch/arm64/lib/copy_from_user.S | 33 +++- arch/arm64/lib/copy_template.S | 14 +- arch/arm64/lib/copy_to_user.S | 17 +- arch/arm64/lib/morello.S | 23 --- block/blk-zoned.c | 6 +- block/ioctl.c | 22 +-- drivers/tty/tty_ioctl.c | 2 +- include/linux/blkdev.h | 8 +- include/linux/tty.h | 2 +- include/linux/user_ptr.h | 44 +++++ kernel/seccomp.c | 2 +- lib/strnlen_user.c | 163 +++++++++++++++---- tools/testing/selftests/arm64/morello/mmap.c | 2 +- 19 files changed, 319 insertions(+), 119 deletions(-)
On 26/10/2023 10:44, Kevin Brodsky wrote:
Kevin Brodsky (12): linux/user_ptr.h: Introduce user_ptr_{base,limit} lib: Refactor do_strnlen_user() lib: Make strnlen_user() PCuABI-friendly block: Eliminate invalid user pointer casts tty: Convert another ioctl helper to take arg as user_uintptr_t seccomp: Preserve pointers when reading sock_fprog from userspace kselftests/arm64: morello: Fix message size in mmap test arm64: morello: Implement user capability accessors inline arm64: uaccess: Move macros from futex.h to uaccess.h arm64: uaccess: Switch to capability-based {get,put}_user in PCuABI arm64: lib: Simplify copy_*_user register allocation arm64: lib: Switch to capability-based copy_*_user in PCuABI
Documentation/core-api/user_ptr.rst | 13 ++ arch/arm64/include/asm/assembler.h | 8 + arch/arm64/include/asm/futex.h | 20 +-- arch/arm64/include/asm/gpr-num.h | 6 +- arch/arm64/include/asm/morello.h | 4 - arch/arm64/include/asm/uaccess.h | 49 +++++- arch/arm64/lib/copy_from_user.S | 33 +++- arch/arm64/lib/copy_template.S | 14 +- arch/arm64/lib/copy_to_user.S | 17 +- arch/arm64/lib/morello.S | 23 --- block/blk-zoned.c | 6 +- block/ioctl.c | 22 +-- drivers/tty/tty_ioctl.c | 2 +- include/linux/blkdev.h | 8 +- include/linux/tty.h | 2 +- include/linux/user_ptr.h | 44 +++++ kernel/seccomp.c | 2 +- lib/strnlen_user.c | 163 +++++++++++++++---- tools/testing/selftests/arm64/morello/mmap.c | 2 +- 19 files changed, 319 insertions(+), 119 deletions(-)
This is now in next. Thank you to Aditya and Amit for the reviews!
Kevin
linux-morello@op-lists.linaro.org
-
Aditya Deshpande -
Amit Daniel Kachhap -
Kevin Brodsky