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(-)