kernel: introduce uaccess logging

In an upcoming change we are going to add uaccess instrumentation that uses inline access to struct task_struct from the instrumentation routines. Because instrumentation.h is included from many places including (recursively) from sched.h this would otherwise lead to a circular dependency. Break the dependency by moving uaccess instrumentation routines into a separate header, instrumentation-uaccess.h.

Link https://linux-review.googlesource.com/id/I625728db0c8db374e13e4ebc54985ac5c79ace7d

Signed-off-by Peter Collingbourne <[email protected]>

---
 include/linux/instrumented-uaccess.h | 49 ++++++++++++++++++++++++++++
 include/linux/instrumented.h         | 34 -------------------
 include/linux/uaccess.h              |  2 +-
 lib/iov_iter.c                       |  2 +-
 lib/usercopy.c                       |  2 +-
 5 files changed, 52 insertions(+), 37 deletions(-)
 create mode 100644 include/linux/instrumented-uaccess.h

include/linux/instrumented-uaccess.h new file +49 -0

new file mode 100644
index 000000000000..ece549088e50
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This header provides generic wrappers for memory access instrumentation for
+ * uaccess routines that the compiler cannot emit for: KASAN, KCSAN.
+ */
+#ifndef _LINUX_INSTRUMENTED_UACCESS_H
+#define _LINUX_INSTRUMENTED_UACCESS_H
+
+#include <linux/compiler.h>
+#include <linux/kasan-checks.h>
+#include <linux/kcsan-checks.h>
+#include <linux/types.h>
+
+/**
+ * instrument_copy_to_user - instrument reads of copy_to_user
+ *
+ * Instrument reads from kernel memory, that are due to copy_to_user (and
+ * variants). The instrumentation must be inserted before the accesses.
+ *
+ * @to destination address
+ * @from source address
+ * @n number of bytes to copy
+ */
+static __always_inline void
+instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
+{
+       kasan_check_read(from, n);
+       kcsan_check_read(from, n);
+}
+
+/**
+ * instrument_copy_from_user - instrument writes of copy_from_user
+ *
+ * Instrument writes to kernel memory, that are due to copy_from_user (and
+ * variants). The instrumentation should be inserted before the accesses.
+ *
+ * @to destination address
+ * @from source address
+ * @n number of bytes to copy
+ */
+static __always_inline void
+instrument_copy_from_user(const void *to, const void __user *from, unsigned long n)
+{
+       kasan_check_write(to, n);
+       kcsan_check_write(to, n);
+}
+
+#endif /* _LINUX_INSTRUMENTED_UACCESS_H */

include/linux/instrumented.h +0 -34

index 42faebbaa202..b68f415510c7 100644
@@ -102,38 +102,4 @@ static __always_inline void instrument_atomic_read_write(const volatile void *v,
        kcsan_check_atomic_read_write(v, size);
 }
 
-/**
- * instrument_copy_to_user - instrument reads of copy_to_user
- *
- * Instrument reads from kernel memory, that are due to copy_to_user (and
- * variants). The instrumentation must be inserted before the accesses.
- *
- * @to destination address
- * @from source address
- * @n number of bytes to copy
- */
-static __always_inline void
-instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
-{
-       kasan_check_read(from, n);
-       kcsan_check_read(from, n);
-}
-
-/**
- * instrument_copy_from_user - instrument writes of copy_from_user
- *
- * Instrument writes to kernel memory, that are due to copy_from_user (and
- * variants). The instrumentation should be inserted before the accesses.
- *
- * @to destination address
- * @from source address
- * @n number of bytes to copy
- */
-static __always_inline void
-instrument_copy_from_user(const void *to, const void __user *from, unsigned long n)
-{
-       kasan_check_write(to, n);
-       kcsan_check_write(to, n);
-}
-
 #endif /* _LINUX_INSTRUMENTED_H */

include/linux/uaccess.h +1 -1

index ac0394087f7d..c0c467e39657 100644
@@ -3,7 +3,7 @@
 #define __LINUX_UACCESS_H__
 
 #include <linux/fault-inject-usercopy.h>
-#include <linux/instrumented.h>
+#include <linux/instrumented-uaccess.h>
 #include <linux/minmax.h>
 #include <linux/sched.h>
 #include <linux/thread_info.h>

lib/iov_iter.c +1 -1

index 66a740e6e153..3f9dc6df7102 100644
@@ -12,7 +12,7 @@
 #include <linux/compat.h>
 #include <net/checksum.h>
 #include <linux/scatterlist.h>
-#include <linux/instrumented.h>
+#include <linux/instrumented-uaccess.h>
 
 #define PIPE_PARANOIA /* for now */
 

lib/usercopy.c +1 -2

index 7413dd300516..1cd188e62d06 100644
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bitops.h>
 #include <linux/fault-inject-usercopy.h>
-#include <linux/instrumented.h>
+#include <linux/instrumented-uaccess.h>
 #include <linux/uaccess.h>
 
 /* out-of-line parts */
--

2.34.1.173.g76aa8bc2d0-goog

Add the core code to support uaccess logging. Subsequent patches will hook this up to the arch-specific kernel entry and exit code for certain architectures.

Link https://linux-review.googlesource.com/id/I6581765646501a5631b281d670903945ebadc57d

Signed-off-by Peter Collingbourne <[email protected]>

---
v3:
- performance optimizations for entry/exit code
- don't use kcur == NULL to mean overflow
- fix potential double free in clone()
- don't allocate a new kernel-side uaccess buffer for each syscall
- fix uaccess buffer leak on exit
- fix some sparse warnings

v2:
- New interface that avoids multiple syscalls per real syscall and is arch-generic
- Avoid logging uaccesses done by BPF programs
- Add documentation
- Split up into multiple patches
- Various code moves, renames etc as requested by Marco

 fs/exec.c                            |   3 +
 include/linux/instrumented-uaccess.h |   6 +-
 include/linux/sched.h                |   5 ++
 include/linux/uaccess-buffer-info.h  |  46 ++++++++++
 include/linux/uaccess-buffer.h       | 112 +++++++++++++++++++++++
 include/uapi/linux/prctl.h           |   3 +
 include/uapi/linux/uaccess-buffer.h  |  27 ++++++
 kernel/Makefile                      |   1 +
 kernel/bpf/helpers.c                 |   7 +-
 kernel/fork.c                        |   4 +
 kernel/signal.c                      |   4 +-
 kernel/sys.c                         |   6 ++
 kernel/uaccess-buffer.c              | 129 +++++++++++++++++++++++++++
 13 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/uaccess-buffer-info.h
 create mode 100644 include/linux/uaccess-buffer.h
 create mode 100644 include/uapi/linux/uaccess-buffer.h
 create mode 100644 kernel/uaccess-buffer.c

fs/exec.c +3 -0

index 537d92c41105..c9975e790f30 100644
@@ -65,6 +65,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io_uring.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>

@@ -1313,6 +1314,8 @@ int begin_new_exec(struct linux_binprm * bprm)
        me->personality &= ~bprm->per_clear;
 
        clear_syscall_work_syscall_user_dispatch(me);
+       uaccess_buffer_set_descriptor_addr_addr(0);
+       uaccess_buffer_free(current);
 
        /*
         * We have to apply CLOEXEC before we change whether the process is

include/linux/instrumented-uaccess.h +5 -1

index ece549088e50..b967f4436d15 100644
@@ -2,7 +2,8 @@
 
 /*
  * This header provides generic wrappers for memory access instrumentation for
- * uaccess routines that the compiler cannot emit for: KASAN, KCSAN.
+ * uaccess routines that the compiler cannot emit for: KASAN, KCSAN,
+ * uaccess buffers.
  */
 #ifndef _LINUX_INSTRUMENTED_UACCESS_H
 #define _LINUX_INSTRUMENTED_UACCESS_H

@@ -11,6 +12,7 @@
 #include <linux/kasan-checks.h>
 #include <linux/kcsan-checks.h>
 #include <linux/types.h>
+#include <linux/uaccess-buffer.h>
 
 /**
  * instrument_copy_to_user - instrument reads of copy_to_user

@@ -27,6 +29,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
        kasan_check_read(from, n);
        kcsan_check_read(from, n);
+       uaccess_buffer_log_write(to, n);
 }
 
 /**

@@ -44,6 +47,7 @@ instrument_copy_from_user(const void *to, const void __user *from, unsigned long
 {
        kasan_check_write(to, n);
        kcsan_check_write(to, n);
+       uaccess_buffer_log_read(from, n);
 }
 
 #endif /* _LINUX_INSTRUMENTED_UACCESS_H */

include/linux/sched.h +5 -0

index 78c351e35fec..7c5278d7b57d 100644
@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <linux/uaccess-buffer-info.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -1484,6 +1485,10 @@ struct task_struct {
        struct callback_head            l1d_flush_kill;
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+       struct uaccess_buffer_info      uaccess_buffer;
+#endif
+
        /*
         * New fields for task_struct should be added above here, so that
         * they are included in the randomized portion of task_struct.

include/linux/uaccess-buffer-info.h new file +46 -0

new file mode 100644
index 000000000000..15e2d8f7c074
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_INFO_H
+#define _LINUX_UACCESS_BUFFER_INFO_H
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+struct uaccess_buffer_info {
+       /*
+        * The pointer to pointer to struct uaccess_descriptor. This is the
+        * value controlled by prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR).
+        */
+       struct uaccess_descriptor __user *__user *desc_ptr_ptr;
+
+       /*
+        * The pointer to struct uaccess_descriptor read at syscall entry time.
+        */
+       struct uaccess_descriptor __user *desc_ptr;
+
+       /*
+        * A pointer to the kernel's temporary copy of the uaccess log for the
+        * current syscall. We log to a kernel buffer in order to avoid leaking
+        * timing information to userspace.
+        */
+       struct uaccess_buffer_entry *kbegin;
+
+       /*
+        * The position of the next uaccess buffer entry for the current
+        * syscall, or NULL if we are not logging the current syscall.
+        */
+       struct uaccess_buffer_entry *kcur;
+
+       /*
+        * A pointer to the end of the kernel's uaccess log.
+        */
+       struct uaccess_buffer_entry *kend;
+
+       /*
+        * The pointer to the userspace uaccess log, as read from the
+        * struct uaccess_descriptor.
+        */
+       struct uaccess_buffer_entry __user *ubegin;
+};
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_INFO_H */

include/linux/uaccess-buffer.h new file +34 -0

new file mode 100644
index 000000000000..f2f46db274f3
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_H
+#define _LINUX_UACCESS_BUFFER_H
+
+#include <linux/sched.h>
+#include <uapi/linux/uaccess-buffer.h>
+
+#include <asm-generic/errno-base.h>
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+       return test_task_syscall_work(tsk, UACCESS_BUFFER_ENTRY);
+}
+
+void __uaccess_buffer_syscall_entry(void);
+static inline void uaccess_buffer_syscall_entry(void)
+{
+       __uaccess_buffer_syscall_entry();
+}
+
+void __uaccess_buffer_syscall_exit(void);
+static inline void uaccess_buffer_syscall_exit(void)
+{
+       __uaccess_buffer_syscall_exit();
+}
+
+bool __uaccess_buffer_pre_exit_loop(void);
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+       if (!test_syscall_work(UACCESS_BUFFER_ENTRY))
+               return false;
+       return __uaccess_buffer_pre_exit_loop();

include/linux/uaccess-buffer.h new file +8 -0

+{
+       current->uaccess_buffer.desc_ptr_ptr =
+               (struct uaccess_descriptor __user * __user *)addr;
+       if (addr)
+               set_syscall_work(UACCESS_BUFFER_ENTRY);
+       else
+               clear_syscall_work(UACCESS_BUFFER_ENTRY);
+       return 0;

include/linux/uaccess-buffer.h new file +50 -0

+               __uaccess_buffer_log_read(from, n);
+}
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n);
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+       if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+               __uaccess_buffer_log_write(to, n);
+}
+
+#else
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+       return false;
+}
+static inline void uaccess_buffer_syscall_entry(void)
+{
+}
+static inline void uaccess_buffer_syscall_exit(void)
+{
+}
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+       return false;
+}
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{
+}
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)
+{
+       return -EINVAL;
+}
+static inline void uaccess_buffer_free(struct task_struct *tsk)
+{
+}
+
+#define copy_from_user_nolog(to, from, len) copy_from_user(to, from, len)
+
+static inline void uaccess_buffer_log_read(const void __user *from,
+                                          unsigned long n)
+{
+}
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+}
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_H */

include/uapi/linux/prctl.h +3 -0

index bb73e9a0b24f..74b37469c7b3 100644
@@ -272,4 +272,7 @@ struct prctl_mm_map {
 # define PR_SCHED_CORE_SCOPE_THREAD_GROUP      1
 # define PR_SCHED_CORE_SCOPE_PROCESS_GROUP     2
 
+/* Configure uaccess logging feature */
+#define PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR    63
+
 #endif /* _LINUX_PRCTL_H */

include/uapi/linux/uaccess-buffer.h new file +27 -0

new file mode 100644
index 000000000000..bf10f7c78857
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_UACCESS_BUFFER_H
+#define _UAPI_LINUX_UACCESS_BUFFER_H
+
+#include <linux/types.h>
+
+/* Location of the uaccess log. */
+struct uaccess_descriptor {
+       /* Address of the uaccess_buffer_entry array. */
+       __u64 addr;
+       /* Size of the uaccess_buffer_entry array in number of elements. */
+       __u64 size;
+};
+
+/* Format of the entries in the uaccess log. */
+struct uaccess_buffer_entry {
+       /* Address being accessed. */
+       __u64 addr;
+       /* Number of bytes that were accessed. */
+       __u64 size;
+       /* UACCESS_BUFFER_* flags. */
+       __u64 flags;
+};
+
+#define UACCESS_BUFFER_FLAG_WRITE      1 /* access was a write */
+
+#endif /* _UAPI_LINUX_UACCESS_BUFFER_H */

kernel/Makefile +1 -0

index 186c49582f45..d4d9be5146c3 100644
@@ -114,6 +114,7 @@ obj-$(CONFIG_KCSAN) += kcsan/
 obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o
 obj-$(CONFIG_HAVE_STATIC_CALL_INLINE) += static_call.o
 obj-$(CONFIG_CFI_CLANG) += cfi.o
+obj-$(CONFIG_HAVE_ARCH_UACCESS_BUFFER) += uaccess-buffer.o
 
 obj-$(CONFIG_PERF_EVENTS) += events/
 

kernel/bpf/helpers.c +6 -1

index 649f07623df6..ab6520a633ef 100644
@@ -15,6 +15,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/security.h>
+#include <linux/uaccess-buffer.h>
 
 #include "../../lib/kstrtox.h"
 

@@ -637,7 +638,11 @@ const struct bpf_func_proto bpf_event_output_data_proto =  {
 BPF_CALL_3(bpf_copy_from_user, void *, dst, u32, size,
           const void __user *, user_ptr)
 {
-       int ret = copy_from_user(dst, user_ptr, size);
+       /*
+        * Avoid logging uaccesses here as the BPF program may not be following
+        * the uaccess log rules.
+        */
+       int ret = copy_from_user_nolog(dst, user_ptr, size);
 
        if (unlikely(ret)) {
                memset(dst, 0, size);

kernel/fork.c +4 -0

index 3244cc56b697..8be2ca528a65 100644
@@ -96,6 +96,7 @@
 #include <linux/scs.h>
 #include <linux/io_uring.h>
 #include <linux/bpf.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>

@@ -754,6 +755,7 @@ void __put_task_struct(struct task_struct *tsk)
        delayacct_tsk_free(tsk);
        put_signal_struct(tsk->signal);
        sched_core_free(tsk);
+       uaccess_buffer_free(tsk);
 
        if (!profile_handoff_task(tsk))
                free_task(tsk);

@@ -890,6 +892,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
        if (memcg_charge_kernel_stack(tsk))
                goto free_stack;
 
+       uaccess_buffer_free(orig);
+
        stack_vm_area = task_stack_vm_area(tsk);
 
        err = arch_dup_task_struct(tsk, orig);

kernel/signal.c +3 -1

index a629b11bf3e0..69bf21518bd0 100644
@@ -45,6 +45,7 @@
 #include <linux/posix-timers.h>
 #include <linux/cgroup.h>
 #include <linux/audit.h>
+#include <linux/uaccess-buffer.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>

@@ -1031,7 +1032,8 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type)
        if (sig_fatal(p, sig) &&
            !(signal->flags & SIGNAL_GROUP_EXIT) &&
            !sigismember(&t->real_blocked, sig) &&
-           (sig == SIGKILL || !p->ptrace)) {
+           (sig == SIGKILL ||
+            !(p->ptrace || uaccess_buffer_maybe_blocked(p)))) {
                /*
                 * This signal will be fatal to the whole group.
                 */

kernel/sys.c +1 -0

index 8fdac0d90504..c71a9a9c0f68 100644
@@ -42,6 +42,7 @@
 #include <linux/version.h>
 #include <linux/ctype.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>

kernel/sys.c +1 -0

+               break;
        default:
                error = -EINVAL;
                break;

kernel/uaccess-buffer.c new file +45 -0

new file mode 100644
index 000000000000..088e43f7611c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for uaccess logging via uaccess buffers.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#include <linux/compat.h>
+#include <linux/mm.h>
+#include <linux/prctl.h>
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/uaccess-buffer.h>
+
+static void uaccess_buffer_log(unsigned long addr, unsigned long size,
+                             unsigned long flags)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       struct uaccess_buffer_entry *entry = buf->kcur;
+
+       if (entry == buf->kend || unlikely(uaccess_kernel()))
+               return;
+       entry->addr = addr;
+       entry->size = size;
+       entry->flags = flags;
+
+       ++buf->kcur;
+}
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+       uaccess_buffer_log((unsigned long)from, n, 0);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_read);
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+       uaccess_buffer_log((unsigned long)to, n, UACCESS_BUFFER_FLAG_WRITE);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_write);
+
+bool __uaccess_buffer_pre_exit_loop(void)

kernel/uaccess-buffer.c new file +30 -0

+       set_current_blocked(&tmp_mask);
+       return true;
+}
+
+void __uaccess_buffer_post_exit_loop(void)
+{
+       spin_lock_irq(&current->sighand->siglock);
+       current->blocked = current->real_blocked;
+       recalc_sigpending();
+       spin_unlock_irq(&current->sighand->siglock);
+}
+
+void uaccess_buffer_free(struct task_struct *tsk)
+{
+       struct uaccess_buffer_info *buf = &tsk->uaccess_buffer;
+
+       kfree(buf->kbegin);
+       clear_syscall_work(UACCESS_BUFFER_EXIT);
+       buf->kbegin = buf->kcur = buf->kend = NULL;
+}
+
+void __uaccess_buffer_syscall_entry(void)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       struct uaccess_descriptor desc;
+
+       if (get_user(buf->desc_ptr, buf->desc_ptr_ptr) || !buf->desc_ptr ||
+           put_user(0, buf->desc_ptr_ptr) ||
+           copy_from_user(&desc, buf->desc_ptr, sizeof(desc)))
+               return;

kernel/uaccess-buffer.c new file +34 -1

+               return;
+
+       set_syscall_work(UACCESS_BUFFER_EXIT);
+       buf->kcur = buf->kbegin;
+       buf->kend = buf->kbegin + desc.size;
+       buf->ubegin =
+               (struct uaccess_buffer_entry __user *)(unsigned long)desc.addr;
+}
+
+void __uaccess_buffer_syscall_exit(void)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       u64 num_entries = buf->kcur - buf->kbegin;
+       struct uaccess_descriptor desc;
+
+       clear_syscall_work(UACCESS_BUFFER_EXIT);
+       desc.addr = (u64)(unsigned long)(buf->ubegin + num_entries);
+       desc.size = buf->kend - buf->kcur;
+       buf->kcur = NULL;
+       if (copy_to_user(buf->ubegin, buf->kbegin,
+                        num_entries * sizeof(struct uaccess_buffer_entry)) == 0)
+               (void)copy_to_user(buf->desc_ptr, &desc, sizeof(desc));
+}
+
+size_t copy_from_user_nolog(void *to, const void __user *from, size_t len)
+{
+       size_t retval;
+
+       clear_syscall_work(UACCESS_BUFFER_EXIT);
+       retval = copy_from_user(to, from, len);
+       if (current->uaccess_buffer.kcur)
+               set_syscall_work(UACCESS_BUFFER_EXIT);
+       return retval;
+}
--

2.34.1.173.g76aa8bc2d0-goog

Quoted 221 lines · Peter Collingbourne

Add the core code to support uaccess logging. Subsequent patches will hook this up to the arch-specific kernel entry and exit code for certain architectures.

Link https://linux-review.googlesource.com/id/I6581765646501a5631b281d670903945ebadc57d

Signed-off-by Peter Collingbourne <[email protected]>

---
v3:
- performance optimizations for entry/exit code
- don't use kcur == NULL to mean overflow
- fix potential double free in clone()
- don't allocate a new kernel-side uaccess buffer for each syscall
- fix uaccess buffer leak on exit
- fix some sparse warnings

v2:
- New interface that avoids multiple syscalls per real syscall and is arch-generic
- Avoid logging uaccesses done by BPF programs
- Add documentation
- Split up into multiple patches
- Various code moves, renames etc as requested by Marco

 fs/exec.c                            |   3 +
 include/linux/instrumented-uaccess.h |   6 +-
 include/linux/sched.h                |   5 ++
 include/linux/uaccess-buffer-info.h  |  46 ++++++++++
 include/linux/uaccess-buffer.h       | 112 +++++++++++++++++++++++
 include/uapi/linux/prctl.h           |   3 +
 include/uapi/linux/uaccess-buffer.h  |  27 ++++++
 kernel/Makefile                      |   1 +
 kernel/bpf/helpers.c                 |   7 +-
 kernel/fork.c                        |   4 +
 kernel/signal.c                      |   4 +-
 kernel/sys.c                         |   6 ++
 kernel/uaccess-buffer.c              | 129 +++++++++++++++++++++++++++
 13 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/uaccess-buffer-info.h
 create mode 100644 include/linux/uaccess-buffer.h
 create mode 100644 include/uapi/linux/uaccess-buffer.h
 create mode 100644 kernel/uaccess-buffer.c

fs/exec.c +3 -0

index 537d92c41105..c9975e790f30 100644
@@ -65,6 +65,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io_uring.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>

@@ -1313,6 +1314,8 @@ int begin_new_exec(struct linux_binprm * bprm)
        me->personality &= ~bprm->per_clear;
 
        clear_syscall_work_syscall_user_dispatch(me);
+       uaccess_buffer_set_descriptor_addr_addr(0);
+       uaccess_buffer_free(current);
 
        /*
         * We have to apply CLOEXEC before we change whether the process is

include/linux/instrumented-uaccess.h +5 -1

index ece549088e50..b967f4436d15 100644
@@ -2,7 +2,8 @@
 
 /*
  * This header provides generic wrappers for memory access instrumentation for
- * uaccess routines that the compiler cannot emit for: KASAN, KCSAN.
+ * uaccess routines that the compiler cannot emit for: KASAN, KCSAN,
+ * uaccess buffers.
  */
 #ifndef _LINUX_INSTRUMENTED_UACCESS_H
 #define _LINUX_INSTRUMENTED_UACCESS_H

@@ -11,6 +12,7 @@
 #include <linux/kasan-checks.h>
 #include <linux/kcsan-checks.h>
 #include <linux/types.h>
+#include <linux/uaccess-buffer.h>
 
 /**
  * instrument_copy_to_user - instrument reads of copy_to_user

@@ -27,6 +29,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
        kasan_check_read(from, n);
        kcsan_check_read(from, n);
+       uaccess_buffer_log_write(to, n);
 }
 
 /**

@@ -44,6 +47,7 @@ instrument_copy_from_user(const void *to, const void __user *from, unsigned long
 {
        kasan_check_write(to, n);
        kcsan_check_write(to, n);
+       uaccess_buffer_log_read(from, n);
 }
 
 #endif /* _LINUX_INSTRUMENTED_UACCESS_H */

include/linux/sched.h +5 -0

index 78c351e35fec..7c5278d7b57d 100644
@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <linux/uaccess-buffer-info.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -1484,6 +1485,10 @@ struct task_struct {
        struct callback_head            l1d_flush_kill;
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+       struct uaccess_buffer_info      uaccess_buffer;
+#endif
+
        /*
         * New fields for task_struct should be added above here, so that
         * they are included in the randomized portion of task_struct.

include/linux/uaccess-buffer-info.h new file +46 -0

new file mode 100644
index 000000000000..15e2d8f7c074
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_INFO_H
+#define _LINUX_UACCESS_BUFFER_INFO_H
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+struct uaccess_buffer_info {
+       /*
+        * The pointer to pointer to struct uaccess_descriptor. This is the
+        * value controlled by prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR).
+        */
+       struct uaccess_descriptor __user *__user *desc_ptr_ptr;
+
+       /*
+        * The pointer to struct uaccess_descriptor read at syscall entry time.
+        */
+       struct uaccess_descriptor __user *desc_ptr;
+
+       /*
+        * A pointer to the kernel's temporary copy of the uaccess log for the
+        * current syscall. We log to a kernel buffer in order to avoid leaking
+        * timing information to userspace.
+        */
+       struct uaccess_buffer_entry *kbegin;
+
+       /*
+        * The position of the next uaccess buffer entry for the current
+        * syscall, or NULL if we are not logging the current syscall.
+        */
+       struct uaccess_buffer_entry *kcur;
+
+       /*
+        * A pointer to the end of the kernel's uaccess log.
+        */
+       struct uaccess_buffer_entry *kend;
+
+       /*
+        * The pointer to the userspace uaccess log, as read from the
+        * struct uaccess_descriptor.
+        */
+       struct uaccess_buffer_entry __user *ubegin;
+};
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_INFO_H */

include/linux/uaccess-buffer.h new file +38 -0

new file mode 100644
index 000000000000..f2f46db274f3
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_H
+#define _LINUX_UACCESS_BUFFER_H
+
+#include <linux/sched.h>
+#include <uapi/linux/uaccess-buffer.h>
+
+#include <asm-generic/errno-base.h>
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+       return test_task_syscall_work(tsk, UACCESS_BUFFER_ENTRY);
+}
+
+void __uaccess_buffer_syscall_entry(void);
+static inline void uaccess_buffer_syscall_entry(void)
+{
+       __uaccess_buffer_syscall_entry();
+}
+
+void __uaccess_buffer_syscall_exit(void);
+static inline void uaccess_buffer_syscall_exit(void)
+{
+       __uaccess_buffer_syscall_exit();
+}
+
+bool __uaccess_buffer_pre_exit_loop(void);
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+       if (!test_syscall_work(UACCESS_BUFFER_ENTRY))
+               return false;
+       return __uaccess_buffer_pre_exit_loop();
+}
+
+void __uaccess_buffer_post_exit_loop(void);
+static inline void uaccess_buffer_post_exit_loop(bool pending)

include/linux/uaccess-buffer.h line 44

Quoted 6 lines

include/linux/uaccess-buffer.h new file +6 -0

+{
+       if (pending)
+               __uaccess_buffer_post_exit_loop();
+}
+
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)

I would move the implementation to .c file. It's a rare path.

Quoted 12 lines

include/linux/uaccess-buffer.h new file +12 -0

+{
+       current->uaccess_buffer.desc_ptr_ptr =
+               (struct uaccess_descriptor __user * __user *)addr;
+       if (addr)
+               set_syscall_work(UACCESS_BUFFER_ENTRY);
+       else
+               clear_syscall_work(UACCESS_BUFFER_ENTRY);
+       return 0;
+}
+
+size_t copy_from_user_nolog(void *to, const void __user *from, size_t len);
+

include/linux/uaccess-buffer.h line 62

Quoted 6 lines

include/linux/uaccess-buffer.h new file +6 -0

+void uaccess_buffer_free(struct task_struct *tsk);
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n);
+static inline void uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+       if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))

UACCESS_BUFFER_EXIT is only defined in future patches, so this won't compile.

Quoted 197 lines

include/linux/uaccess-buffer.h new file +50 -0

+               __uaccess_buffer_log_read(from, n);
+}
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n);
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+       if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+               __uaccess_buffer_log_write(to, n);
+}
+
+#else
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+       return false;
+}
+static inline void uaccess_buffer_syscall_entry(void)
+{
+}
+static inline void uaccess_buffer_syscall_exit(void)
+{
+}
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+       return false;
+}
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{
+}
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)
+{
+       return -EINVAL;
+}
+static inline void uaccess_buffer_free(struct task_struct *tsk)
+{
+}
+
+#define copy_from_user_nolog(to, from, len) copy_from_user(to, from, len)
+
+static inline void uaccess_buffer_log_read(const void __user *from,
+                                          unsigned long n)
+{
+}
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+}
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_H */

include/uapi/linux/prctl.h +3 -0

index bb73e9a0b24f..74b37469c7b3 100644
@@ -272,4 +272,7 @@ struct prctl_mm_map {
 # define PR_SCHED_CORE_SCOPE_THREAD_GROUP      1
 # define PR_SCHED_CORE_SCOPE_PROCESS_GROUP     2
 
+/* Configure uaccess logging feature */
+#define PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR    63
+
 #endif /* _LINUX_PRCTL_H */

include/uapi/linux/uaccess-buffer.h new file +27 -0

new file mode 100644
index 000000000000..bf10f7c78857
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_UACCESS_BUFFER_H
+#define _UAPI_LINUX_UACCESS_BUFFER_H
+
+#include <linux/types.h>
+
+/* Location of the uaccess log. */
+struct uaccess_descriptor {
+       /* Address of the uaccess_buffer_entry array. */
+       __u64 addr;
+       /* Size of the uaccess_buffer_entry array in number of elements. */
+       __u64 size;
+};
+
+/* Format of the entries in the uaccess log. */
+struct uaccess_buffer_entry {
+       /* Address being accessed. */
+       __u64 addr;
+       /* Number of bytes that were accessed. */
+       __u64 size;
+       /* UACCESS_BUFFER_* flags. */
+       __u64 flags;
+};
+
+#define UACCESS_BUFFER_FLAG_WRITE      1 /* access was a write */
+
+#endif /* _UAPI_LINUX_UACCESS_BUFFER_H */

kernel/Makefile +1 -0

index 186c49582f45..d4d9be5146c3 100644
@@ -114,6 +114,7 @@ obj-$(CONFIG_KCSAN) += kcsan/
 obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o
 obj-$(CONFIG_HAVE_STATIC_CALL_INLINE) += static_call.o
 obj-$(CONFIG_CFI_CLANG) += cfi.o
+obj-$(CONFIG_HAVE_ARCH_UACCESS_BUFFER) += uaccess-buffer.o
 
 obj-$(CONFIG_PERF_EVENTS) += events/
 

kernel/bpf/helpers.c +6 -1

index 649f07623df6..ab6520a633ef 100644
@@ -15,6 +15,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/security.h>
+#include <linux/uaccess-buffer.h>
 
 #include "../../lib/kstrtox.h"
 

@@ -637,7 +638,11 @@ const struct bpf_func_proto bpf_event_output_data_proto =  {
 BPF_CALL_3(bpf_copy_from_user, void *, dst, u32, size,
           const void __user *, user_ptr)
 {
-       int ret = copy_from_user(dst, user_ptr, size);
+       /*
+        * Avoid logging uaccesses here as the BPF program may not be following
+        * the uaccess log rules.
+        */
+       int ret = copy_from_user_nolog(dst, user_ptr, size);
 
        if (unlikely(ret)) {
                memset(dst, 0, size);

kernel/fork.c +4 -0

index 3244cc56b697..8be2ca528a65 100644
@@ -96,6 +96,7 @@
 #include <linux/scs.h>
 #include <linux/io_uring.h>
 #include <linux/bpf.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>

@@ -754,6 +755,7 @@ void __put_task_struct(struct task_struct *tsk)
        delayacct_tsk_free(tsk);
        put_signal_struct(tsk->signal);
        sched_core_free(tsk);
+       uaccess_buffer_free(tsk);
 
        if (!profile_handoff_task(tsk))
                free_task(tsk);

@@ -890,6 +892,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
        if (memcg_charge_kernel_stack(tsk))
                goto free_stack;
 
+       uaccess_buffer_free(orig);
+
        stack_vm_area = task_stack_vm_area(tsk);
 
        err = arch_dup_task_struct(tsk, orig);

kernel/signal.c +3 -1

index a629b11bf3e0..69bf21518bd0 100644
@@ -45,6 +45,7 @@
 #include <linux/posix-timers.h>
 #include <linux/cgroup.h>
 #include <linux/audit.h>
+#include <linux/uaccess-buffer.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>

@@ -1031,7 +1032,8 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type)
        if (sig_fatal(p, sig) &&
            !(signal->flags & SIGNAL_GROUP_EXIT) &&
            !sigismember(&t->real_blocked, sig) &&
-           (sig == SIGKILL || !p->ptrace)) {
+           (sig == SIGKILL ||
+            !(p->ptrace || uaccess_buffer_maybe_blocked(p)))) {
                /*
                 * This signal will be fatal to the whole group.
                 */

kernel/sys.c +1 -0

index 8fdac0d90504..c71a9a9c0f68 100644
@@ -42,6 +42,7 @@
 #include <linux/version.h>
 #include <linux/ctype.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>

kernel/sys.c SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, · line 2537

Quoted 7 lines

kernel/sys.c +4 -0

@@ -2530,6 +2531,11 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
                error = sched_core_share_pid(arg2, arg3, arg4, arg5);
                break;
 #endif
+       case PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR:
+               if (arg3 || arg4 || arg5)
+                       return -EINVAL;
+               error = uaccess_buffer_set_descriptor_addr_addr(arg2);

Does this miss uaccess_buffer_free() for the 0 case?
It seems that when we set it to 0 we always want to free as well (e.g. in exec). I wonder if freeing should be done by uaccess_buffer_set_descriptor_addr_addr() itself.
Both uaccess_buffer_set_descriptor_addr_addr() and uaccess_buffer_free() reset task work, which is fine but is somewhat suboptimal logically.
Then task exit could do uaccess_buffer_set_descriptor_addr_addr(0).

Quoted 64 lines

kernel/sys.c +1 -0

+               break;
        default:
                error = -EINVAL;
                break;

kernel/uaccess-buffer.c new file +53 -0

new file mode 100644
index 000000000000..088e43f7611c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for uaccess logging via uaccess buffers.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#include <linux/compat.h>
+#include <linux/mm.h>
+#include <linux/prctl.h>
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/uaccess-buffer.h>
+
+static void uaccess_buffer_log(unsigned long addr, unsigned long size,
+                             unsigned long flags)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       struct uaccess_buffer_entry *entry = buf->kcur;
+
+       if (entry == buf->kend || unlikely(uaccess_kernel()))
+               return;
+       entry->addr = addr;
+       entry->size = size;
+       entry->flags = flags;
+
+       ++buf->kcur;
+}
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+       uaccess_buffer_log((unsigned long)from, n, 0);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_read);
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+       uaccess_buffer_log((unsigned long)to, n, UACCESS_BUFFER_FLAG_WRITE);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_write);
+
+bool __uaccess_buffer_pre_exit_loop(void)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       struct uaccess_descriptor __user *desc_ptr;
+       sigset_t tmp_mask;
+
+       if (get_user(desc_ptr, buf->desc_ptr_ptr) || !desc_ptr)
+               return false;
+

kernel/uaccess-buffer.c line 55

Quoted 2 lines

kernel/uaccess-buffer.c new file +2 -0

+       current->real_blocked = current->blocked;
+       sigfillset(&tmp_mask);

This and __uaccess_buffer_post_exit_loop() runs only when we have a signal/timer interrupt between setting the descriptor address in userspace and entering the next syscall, right?
Just want to make sure this code is not executed for normal uaccess tracing for performance reasons.

Quoted 37 lines

kernel/uaccess-buffer.c new file +37 -0

+       set_current_blocked(&tmp_mask);
+       return true;
+}
+
+void __uaccess_buffer_post_exit_loop(void)
+{
+       spin_lock_irq(&current->sighand->siglock);
+       current->blocked = current->real_blocked;
+       recalc_sigpending();
+       spin_unlock_irq(&current->sighand->siglock);
+}
+
+void uaccess_buffer_free(struct task_struct *tsk)
+{
+       struct uaccess_buffer_info *buf = &tsk->uaccess_buffer;
+
+       kfree(buf->kbegin);
+       clear_syscall_work(UACCESS_BUFFER_EXIT);
+       buf->kbegin = buf->kcur = buf->kend = NULL;
+}
+
+void __uaccess_buffer_syscall_entry(void)
+{
+       struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+       struct uaccess_descriptor desc;
+
+       if (get_user(buf->desc_ptr, buf->desc_ptr_ptr) || !buf->desc_ptr ||
+           put_user(0, buf->desc_ptr_ptr) ||
+           copy_from_user(&desc, buf->desc_ptr, sizeof(desc)))
+               return;
+
+       if (desc.size > 1024)
+               desc.size = 1024;
+
+       if (buf->kend - buf->kbegin != desc.size)
+               buf->kbegin =
+                       krealloc_array(buf->kbegin, desc.size,

kernel/uaccess-buffer.c line 95

Quoted 3 lines

kernel/uaccess-buffer.c new file +3 -0

+                                      sizeof(struct uaccess_buffer_entry),
+                                      GFP_KERNEL);
+       if (!buf->kbegin)

I think we also need to set at least buf->kend to NULL here. I am not sure what can go wrong now, but it's a strange state. On next iteration we will do "buf->kend - buf->kbegin", where kend is a dangling pointer and kbegin is NULL.

Add the core code to support uaccess logging. Subsequent patches will hook this up to the arch-specific kernel entry and exit code for certain architectures.

Link https://linux-review.googlesource.com/id/I6581765646501a5631b281d670903945ebadc57d

Signed-off-by Peter Collingbourne <[email protected]>

---
v3:
- performance optimizations for entry/exit code
- don't use kcur == NULL to mean overflow
- fix potential double free in clone()
- don't allocate a new kernel-side uaccess buffer for each syscall
- fix uaccess buffer leak on exit
- fix some sparse warnings

v2:
- New interface that avoids multiple syscalls per real syscall and is arch-generic
- Avoid logging uaccesses done by BPF programs
- Add documentation
- Split up into multiple patches
- Various code moves, renames etc as requested by Marco

 fs/exec.c                            |   3 +
 include/linux/instrumented-uaccess.h |   6 +-
 include/linux/sched.h                |   5 ++
 include/linux/uaccess-buffer-info.h  |  46 ++++++++++
 include/linux/uaccess-buffer.h       | 112 +++++++++++++++++++++++
 include/uapi/linux/prctl.h           |   3 +
 include/uapi/linux/uaccess-buffer.h  |  27 ++++++
 kernel/Makefile                      |   1 +
 kernel/bpf/helpers.c                 |   7 +-
 kernel/fork.c                        |   4 +
 kernel/signal.c                      |   4 +-
 kernel/sys.c                         |   6 ++
 kernel/uaccess-buffer.c              | 129 +++++++++++++++++++++++++++
 13 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/uaccess-buffer-info.h
 create mode 100644 include/linux/uaccess-buffer.h
 create mode 100644 include/uapi/linux/uaccess-buffer.h
 create mode 100644 kernel/uaccess-buffer.c

fs/exec.c +3 -0

index 537d92c41105..c9975e790f30 100644
@@ -65,6 +65,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io_uring.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>

@@ -1313,6 +1314,8 @@ int begin_new_exec(struct linux_binprm * bprm)
 	me->personality &= ~bprm->per_clear;
 
 	clear_syscall_work_syscall_user_dispatch(me);
+	uaccess_buffer_set_descriptor_addr_addr(0);
+	uaccess_buffer_free(current);
 
 	/*
 	 * We have to apply CLOEXEC before we change whether the process is

include/linux/instrumented-uaccess.h +5 -1

index ece549088e50..b967f4436d15 100644
@@ -2,7 +2,8 @@
 
 /*
  * This header provides generic wrappers for memory access instrumentation for
- * uaccess routines that the compiler cannot emit for: KASAN, KCSAN.
+ * uaccess routines that the compiler cannot emit for: KASAN, KCSAN,
+ * uaccess buffers.
  */
 #ifndef _LINUX_INSTRUMENTED_UACCESS_H
 #define _LINUX_INSTRUMENTED_UACCESS_H

@@ -11,6 +12,7 @@
 #include <linux/kasan-checks.h>
 #include <linux/kcsan-checks.h>
 #include <linux/types.h>
+#include <linux/uaccess-buffer.h>
 
 /**
  * instrument_copy_to_user - instrument reads of copy_to_user

@@ -27,6 +29,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
 	kasan_check_read(from, n);
 	kcsan_check_read(from, n);
+	uaccess_buffer_log_write(to, n);
 }
 
 /**

@@ -44,6 +47,7 @@ instrument_copy_from_user(const void *to, const void __user *from, unsigned long
 {
 	kasan_check_write(to, n);
 	kcsan_check_write(to, n);
+	uaccess_buffer_log_read(from, n);
 }
 
 #endif /* _LINUX_INSTRUMENTED_UACCESS_H */

include/linux/sched.h +5 -0

index 78c351e35fec..7c5278d7b57d 100644
@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <linux/uaccess-buffer-info.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -1484,6 +1485,10 @@ struct task_struct {
 	struct callback_head		l1d_flush_kill;
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+	struct uaccess_buffer_info	uaccess_buffer;
+#endif
+
 	/*
 	 * New fields for task_struct should be added above here, so that
 	 * they are included in the randomized portion of task_struct.

include/linux/uaccess-buffer-info.h new file +46 -0

new file mode 100644
index 000000000000..15e2d8f7c074
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_INFO_H
+#define _LINUX_UACCESS_BUFFER_INFO_H
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+struct uaccess_buffer_info {
+	/*
+	 * The pointer to pointer to struct uaccess_descriptor. This is the
+	 * value controlled by prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR).
+	 */
+	struct uaccess_descriptor __user *__user *desc_ptr_ptr;
+
+	/*
+	 * The pointer to struct uaccess_descriptor read at syscall entry time.
+	 */
+	struct uaccess_descriptor __user *desc_ptr;
+
+	/*
+	 * A pointer to the kernel's temporary copy of the uaccess log for the
+	 * current syscall. We log to a kernel buffer in order to avoid leaking
+	 * timing information to userspace.
+	 */
+	struct uaccess_buffer_entry *kbegin;
+
+	/*
+	 * The position of the next uaccess buffer entry for the current
+	 * syscall, or NULL if we are not logging the current syscall.
+	 */
+	struct uaccess_buffer_entry *kcur;
+
+	/*
+	 * A pointer to the end of the kernel's uaccess log.
+	 */
+	struct uaccess_buffer_entry *kend;
+
+	/*
+	 * The pointer to the userspace uaccess log, as read from the
+	 * struct uaccess_descriptor.
+	 */
+	struct uaccess_buffer_entry __user *ubegin;
+};
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_INFO_H */

include/linux/uaccess-buffer.h new file +0 -0

new file mode 100644
index 000000000000..f2f46db274f3

include/linux/uaccess-buffer.h new file +35 -0

+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+	return test_task_syscall_work(tsk, UACCESS_BUFFER_ENTRY);
+}
+
+void __uaccess_buffer_syscall_entry(void);
+static inline void uaccess_buffer_syscall_entry(void)
+{
+	__uaccess_buffer_syscall_entry();
+}
+
+void __uaccess_buffer_syscall_exit(void);
+static inline void uaccess_buffer_syscall_exit(void)
+{
+	__uaccess_buffer_syscall_exit();
+}
+
+bool __uaccess_buffer_pre_exit_loop(void);
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+	if (!test_syscall_work(UACCESS_BUFFER_ENTRY))
+		return false;
+	return __uaccess_buffer_pre_exit_loop();
+}
+
+void __uaccess_buffer_post_exit_loop(void);
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{
+	if (pending)
+		__uaccess_buffer_post_exit_loop();
+}
+
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)
+{

include/linux/uaccess-buffer.h new file +35 -0

+
+void uaccess_buffer_free(struct task_struct *tsk);
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n);
+static inline void uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+	if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+		__uaccess_buffer_log_read(from, n);
+}
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n);
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+	if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+		__uaccess_buffer_log_write(to, n);
+}
+
+#else
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+	return false;
+}
+static inline void uaccess_buffer_syscall_entry(void)
+{
+}
+static inline void uaccess_buffer_syscall_exit(void)
+{
+}
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+	return false;
+}
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{

include/linux/uaccess-buffer.h new file +12 -0

+
+static inline void uaccess_buffer_log_read(const void __user *from,
+					   unsigned long n)
+{
+}
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+}
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_H */

include/uapi/linux/prctl.h +3 -0

index bb73e9a0b24f..74b37469c7b3 100644
@@ -272,4 +272,7 @@ struct prctl_mm_map {
 # define PR_SCHED_CORE_SCOPE_THREAD_GROUP	1
 # define PR_SCHED_CORE_SCOPE_PROCESS_GROUP	2
 
+/* Configure uaccess logging feature */
+#define PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR	63
+
 #endif /* _LINUX_PRCTL_H */

include/uapi/linux/uaccess-buffer.h new file +27 -0

new file mode 100644
index 000000000000..bf10f7c78857
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_UACCESS_BUFFER_H
+#define _UAPI_LINUX_UACCESS_BUFFER_H
+
+#include <linux/types.h>
+
+/* Location of the uaccess log. */
+struct uaccess_descriptor {
+	/* Address of the uaccess_buffer_entry array. */
+	__u64 addr;
+	/* Size of the uaccess_buffer_entry array in number of elements. */
+	__u64 size;
+};
+
+/* Format of the entries in the uaccess log. */
+struct uaccess_buffer_entry {
+	/* Address being accessed. */
+	__u64 addr;
+	/* Number of bytes that were accessed. */
+	__u64 size;
+	/* UACCESS_BUFFER_* flags. */
+	__u64 flags;
+};
+
+#define UACCESS_BUFFER_FLAG_WRITE	1 /* access was a write */
+
+#endif /* _UAPI_LINUX_UACCESS_BUFFER_H */

kernel/Makefile +1 -0

index 186c49582f45..d4d9be5146c3 100644
@@ -114,6 +114,7 @@ obj-$(CONFIG_KCSAN) += kcsan/
 obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o
 obj-$(CONFIG_HAVE_STATIC_CALL_INLINE) += static_call.o
 obj-$(CONFIG_CFI_CLANG) += cfi.o
+obj-$(CONFIG_HAVE_ARCH_UACCESS_BUFFER) += uaccess-buffer.o
 
 obj-$(CONFIG_PERF_EVENTS) += events/
 

kernel/bpf/helpers.c +6 -1

index 649f07623df6..ab6520a633ef 100644
@@ -15,6 +15,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/security.h>
+#include <linux/uaccess-buffer.h>
 
 #include "../../lib/kstrtox.h"
 

@@ -637,7 +638,11 @@ const struct bpf_func_proto bpf_event_output_data_proto =  {
 BPF_CALL_3(bpf_copy_from_user, void *, dst, u32, size,
 	   const void __user *, user_ptr)
 {
-	int ret = copy_from_user(dst, user_ptr, size);
+	/*
+	 * Avoid logging uaccesses here as the BPF program may not be following
+	 * the uaccess log rules.
+	 */
+	int ret = copy_from_user_nolog(dst, user_ptr, size);
 
 	if (unlikely(ret)) {
 		memset(dst, 0, size);

kernel/fork.c +4 -0

index 3244cc56b697..8be2ca528a65 100644
@@ -96,6 +96,7 @@
 #include <linux/scs.h>
 #include <linux/io_uring.h>
 #include <linux/bpf.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>

@@ -754,6 +755,7 @@ void __put_task_struct(struct task_struct *tsk)
 	delayacct_tsk_free(tsk);
 	put_signal_struct(tsk->signal);
 	sched_core_free(tsk);
+	uaccess_buffer_free(tsk);
 
 	if (!profile_handoff_task(tsk))
 		free_task(tsk);

@@ -890,6 +892,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	if (memcg_charge_kernel_stack(tsk))
 		goto free_stack;
 
+	uaccess_buffer_free(orig);
+
 	stack_vm_area = task_stack_vm_area(tsk);
 
 	err = arch_dup_task_struct(tsk, orig);

kernel/signal.c +3 -1

index a629b11bf3e0..69bf21518bd0 100644
@@ -45,6 +45,7 @@
 #include <linux/posix-timers.h>
 #include <linux/cgroup.h>
 #include <linux/audit.h>
+#include <linux/uaccess-buffer.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>

@@ -1031,7 +1032,8 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type)
 	if (sig_fatal(p, sig) &&
 	    !(signal->flags & SIGNAL_GROUP_EXIT) &&
 	    !sigismember(&t->real_blocked, sig) &&
-	    (sig == SIGKILL || !p->ptrace)) {
+	    (sig == SIGKILL ||
+	     !(p->ptrace || uaccess_buffer_maybe_blocked(p)))) {
 		/*
 		 * This signal will be fatal to the whole group.
 		 */

kernel/sys.c +6 -0

index 8fdac0d90504..c71a9a9c0f68 100644
@@ -42,6 +42,7 @@
 #include <linux/version.h>
 #include <linux/ctype.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>

@@ -2530,6 +2531,11 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		error = sched_core_share_pid(arg2, arg3, arg4, arg5);
 		break;
 #endif
+	case PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = uaccess_buffer_set_descriptor_addr_addr(arg2);
+		break;
 	default:
 		error = -EINVAL;
 		break;

kernel/uaccess-buffer.c new file +129 -0

new file mode 100644
index 000000000000..088e43f7611c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for uaccess logging via uaccess buffers.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#include <linux/compat.h>
+#include <linux/mm.h>
+#include <linux/prctl.h>
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/uaccess-buffer.h>
+
+static void uaccess_buffer_log(unsigned long addr, unsigned long size,
+			      unsigned long flags)
+{
+	struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+	struct uaccess_buffer_entry *entry = buf->kcur;
+
+	if (entry == buf->kend || unlikely(uaccess_kernel()))
+		return;
+	entry->addr = addr;
+	entry->size = size;
+	entry->flags = flags;
+
+	++buf->kcur;
+}
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+	uaccess_buffer_log((unsigned long)from, n, 0);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_read);
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+	uaccess_buffer_log((unsigned long)to, n, UACCESS_BUFFER_FLAG_WRITE);
+}
+EXPORT_SYMBOL(__uaccess_buffer_log_write);
+
+bool __uaccess_buffer_pre_exit_loop(void)
+{
+	struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+	struct uaccess_descriptor __user *desc_ptr;
+	sigset_t tmp_mask;
+
+	if (get_user(desc_ptr, buf->desc_ptr_ptr) || !desc_ptr)
+		return false;
+
+	current->real_blocked = current->blocked;
+	sigfillset(&tmp_mask);
+	set_current_blocked(&tmp_mask);
+	return true;
+}
+
+void __uaccess_buffer_post_exit_loop(void)
+{
+	spin_lock_irq(&current->sighand->siglock);
+	current->blocked = current->real_blocked;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+}
+
+void uaccess_buffer_free(struct task_struct *tsk)
+{
+	struct uaccess_buffer_info *buf = &tsk->uaccess_buffer;
+
+	kfree(buf->kbegin);
+	clear_syscall_work(UACCESS_BUFFER_EXIT);
+	buf->kbegin = buf->kcur = buf->kend = NULL;
+}
+
+void __uaccess_buffer_syscall_entry(void)
+{
+	struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+	struct uaccess_descriptor desc;
+
+	if (get_user(buf->desc_ptr, buf->desc_ptr_ptr) || !buf->desc_ptr ||
+	    put_user(0, buf->desc_ptr_ptr) ||
+	    copy_from_user(&desc, buf->desc_ptr, sizeof(desc)))
+		return;
+
+	if (desc.size > 1024)
+		desc.size = 1024;
+
+	if (buf->kend - buf->kbegin != desc.size)
+		buf->kbegin =
+			krealloc_array(buf->kbegin, desc.size,
+				       sizeof(struct uaccess_buffer_entry),
+				       GFP_KERNEL);
+	if (!buf->kbegin)
+		return;
+
+	set_syscall_work(UACCESS_BUFFER_EXIT);
+	buf->kcur = buf->kbegin;
+	buf->kend = buf->kbegin + desc.size;
+	buf->ubegin =
+		(struct uaccess_buffer_entry __user *)(unsigned long)desc.addr;
+}
+
+void __uaccess_buffer_syscall_exit(void)
+{
+	struct uaccess_buffer_info *buf = &current->uaccess_buffer;
+	u64 num_entries = buf->kcur - buf->kbegin;
+	struct uaccess_descriptor desc;
+
+	clear_syscall_work(UACCESS_BUFFER_EXIT);
+	desc.addr = (u64)(unsigned long)(buf->ubegin + num_entries);
+	desc.size = buf->kend - buf->kcur;
+	buf->kcur = NULL;
+	if (copy_to_user(buf->ubegin, buf->kbegin,
+			 num_entries * sizeof(struct uaccess_buffer_entry)) == 0)
+		(void)copy_to_user(buf->desc_ptr, &desc, sizeof(desc));
+}
+
+size_t copy_from_user_nolog(void *to, const void __user *from, size_t len)
+{
+	size_t retval;
+
+	clear_syscall_work(UACCESS_BUFFER_EXIT);
+	retval = copy_from_user(to, from, len);
+	if (current->uaccess_buffer.kcur)
+		set_syscall_work(UACCESS_BUFFER_EXIT);
+	return retval;
+}

Signature · 1 line

2.34.1.173.g76aa8bc2d0-goog

Quoted 187 lines · Peter Collingbourne

Add the core code to support uaccess logging. Subsequent patches will hook this up to the arch-specific kernel entry and exit code for certain architectures.

Link https://linux-review.googlesource.com/id/I6581765646501a5631b281d670903945ebadc57d

Signed-off-by Peter Collingbourne <[email protected]>

---
v3:
- performance optimizations for entry/exit code
- don't use kcur == NULL to mean overflow
- fix potential double free in clone()
- don't allocate a new kernel-side uaccess buffer for each syscall
- fix uaccess buffer leak on exit
- fix some sparse warnings

v2:
- New interface that avoids multiple syscalls per real syscall and is arch-generic
- Avoid logging uaccesses done by BPF programs
- Add documentation
- Split up into multiple patches
- Various code moves, renames etc as requested by Marco

 fs/exec.c                            |   3 +
 include/linux/instrumented-uaccess.h |   6 +-
 include/linux/sched.h                |   5 ++
 include/linux/uaccess-buffer-info.h  |  46 ++++++++++
 include/linux/uaccess-buffer.h       | 112 +++++++++++++++++++++++
 include/uapi/linux/prctl.h           |   3 +
 include/uapi/linux/uaccess-buffer.h  |  27 ++++++
 kernel/Makefile                      |   1 +
 kernel/bpf/helpers.c                 |   7 +-
 kernel/fork.c                        |   4 +
 kernel/signal.c                      |   4 +-
 kernel/sys.c                         |   6 ++
 kernel/uaccess-buffer.c              | 129 +++++++++++++++++++++++++++
 13 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/uaccess-buffer-info.h
 create mode 100644 include/linux/uaccess-buffer.h
 create mode 100644 include/uapi/linux/uaccess-buffer.h
 create mode 100644 kernel/uaccess-buffer.c

fs/exec.c +3 -0

index 537d92c41105..c9975e790f30 100644
@@ -65,6 +65,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io_uring.h>
 #include <linux/syscall_user_dispatch.h>
+#include <linux/uaccess-buffer.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>

@@ -1313,6 +1314,8 @@ int begin_new_exec(struct linux_binprm * bprm)
      me->personality &= ~bprm->per_clear;
 
      clear_syscall_work_syscall_user_dispatch(me);
+     uaccess_buffer_set_descriptor_addr_addr(0);
+     uaccess_buffer_free(current);
 
      /*
       * We have to apply CLOEXEC before we change whether the process is

include/linux/instrumented-uaccess.h +5 -1

index ece549088e50..b967f4436d15 100644
@@ -2,7 +2,8 @@
 
 /*
  * This header provides generic wrappers for memory access instrumentation for
- * uaccess routines that the compiler cannot emit for: KASAN, KCSAN.
+ * uaccess routines that the compiler cannot emit for: KASAN, KCSAN,
+ * uaccess buffers.
  */
 #ifndef _LINUX_INSTRUMENTED_UACCESS_H
 #define _LINUX_INSTRUMENTED_UACCESS_H

@@ -11,6 +12,7 @@
 #include <linux/kasan-checks.h>
 #include <linux/kcsan-checks.h>
 #include <linux/types.h>
+#include <linux/uaccess-buffer.h>
 
 /**
  * instrument_copy_to_user - instrument reads of copy_to_user

@@ -27,6 +29,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
      kasan_check_read(from, n);
      kcsan_check_read(from, n);
+     uaccess_buffer_log_write(to, n);
 }
 
 /**

@@ -44,6 +47,7 @@ instrument_copy_from_user(const void *to, const void __user *from, unsigned long
 {
      kasan_check_write(to, n);
      kcsan_check_write(to, n);
+     uaccess_buffer_log_read(from, n);
 }
 
 #endif /* _LINUX_INSTRUMENTED_UACCESS_H */

include/linux/sched.h +5 -0

index 78c351e35fec..7c5278d7b57d 100644
@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <linux/uaccess-buffer-info.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -1484,6 +1485,10 @@ struct task_struct {
      struct callback_head            l1d_flush_kill;
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+     struct uaccess_buffer_info      uaccess_buffer;
+#endif
+
      /*
       * New fields for task_struct should be added above here, so that
       * they are included in the randomized portion of task_struct.

include/linux/uaccess-buffer-info.h new file +46 -0

new file mode 100644
index 000000000000..15e2d8f7c074
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_INFO_H
+#define _LINUX_UACCESS_BUFFER_INFO_H
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER
+
+struct uaccess_buffer_info {
+     /*
+      * The pointer to pointer to struct uaccess_descriptor. This is the
+      * value controlled by prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR).
+      */
+     struct uaccess_descriptor __user *__user *desc_ptr_ptr;
+
+     /*
+      * The pointer to struct uaccess_descriptor read at syscall entry time.
+      */
+     struct uaccess_descriptor __user *desc_ptr;
+
+     /*
+      * A pointer to the kernel's temporary copy of the uaccess log for the
+      * current syscall. We log to a kernel buffer in order to avoid leaking
+      * timing information to userspace.
+      */
+     struct uaccess_buffer_entry *kbegin;
+
+     /*
+      * The position of the next uaccess buffer entry for the current
+      * syscall, or NULL if we are not logging the current syscall.
+      */
+     struct uaccess_buffer_entry *kcur;
+
+     /*
+      * A pointer to the end of the kernel's uaccess log.
+      */
+     struct uaccess_buffer_entry *kend;
+
+     /*
+      * The pointer to the userspace uaccess log, as read from the
+      * struct uaccess_descriptor.
+      */
+     struct uaccess_buffer_entry __user *ubegin;
+};
+
+#endif
+
+#endif  /* _LINUX_UACCESS_BUFFER_INFO_H */

include/linux/uaccess-buffer.h new file +5 -0

new file mode 100644
index 000000000000..f2f46db274f3
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UACCESS_BUFFER_H
+#define _LINUX_UACCESS_BUFFER_H
+
+#include <linux/sched.h>

include/linux/uaccess-buffer.h line 10

Quoted 5 lines

include/linux/uaccess-buffer.h new file +5 -0

+#include <uapi/linux/uaccess-buffer.h>
+
+#include <asm-generic/errno-base.h>
+
+#ifdef CONFIG_HAVE_ARCH_UACCESS_BUFFER

Kernel-doc comments for each of the below would be useful (if __ prefixed functions are implementation details, they can be left as-is).

Quoted 40 lines

include/linux/uaccess-buffer.h new file +40 -0

+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+     return test_task_syscall_work(tsk, UACCESS_BUFFER_ENTRY);
+}
+
+void __uaccess_buffer_syscall_entry(void);
+static inline void uaccess_buffer_syscall_entry(void)
+{
+     __uaccess_buffer_syscall_entry();
+}
+
+void __uaccess_buffer_syscall_exit(void);
+static inline void uaccess_buffer_syscall_exit(void)
+{
+     __uaccess_buffer_syscall_exit();
+}
+
+bool __uaccess_buffer_pre_exit_loop(void);
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+     if (!test_syscall_work(UACCESS_BUFFER_ENTRY))
+             return false;
+     return __uaccess_buffer_pre_exit_loop();
+}
+
+void __uaccess_buffer_post_exit_loop(void);
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{
+     if (pending)
+             __uaccess_buffer_post_exit_loop();
+}
+
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)
+{
+     current->uaccess_buffer.desc_ptr_ptr =
+             (struct uaccess_descriptor __user * __user *)addr;
+     if (addr)
+             set_syscall_work(UACCESS_BUFFER_ENTRY);
+     else

include/linux/uaccess-buffer.h line 55

Quoted 5 lines

include/linux/uaccess-buffer.h new file +5 -0

+             clear_syscall_work(UACCESS_BUFFER_ENTRY);
+     return 0;
+}
+
+size_t copy_from_user_nolog(void *to, const void __user *from, size_t len);

copy_from_user() has unsigned long for @len and also return type. I'd make them match.

Quoted 39 lines

include/linux/uaccess-buffer.h new file +39 -0

+
+void uaccess_buffer_free(struct task_struct *tsk);
+
+void __uaccess_buffer_log_read(const void __user *from, unsigned long n);
+static inline void uaccess_buffer_log_read(const void __user *from, unsigned long n)
+{
+     if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+             __uaccess_buffer_log_read(from, n);
+}
+
+void __uaccess_buffer_log_write(void __user *to, unsigned long n);
+static inline void uaccess_buffer_log_write(void __user *to, unsigned long n)
+{
+     if (unlikely(test_syscall_work(UACCESS_BUFFER_EXIT)))
+             __uaccess_buffer_log_write(to, n);
+}
+
+#else
+
+static inline bool uaccess_buffer_maybe_blocked(struct task_struct *tsk)
+{
+     return false;
+}
+static inline void uaccess_buffer_syscall_entry(void)
+{
+}
+static inline void uaccess_buffer_syscall_exit(void)
+{
+}
+static inline bool uaccess_buffer_pre_exit_loop(void)
+{
+     return false;
+}
+static inline void uaccess_buffer_post_exit_loop(bool pending)
+{
+}
+static inline int uaccess_buffer_set_descriptor_addr_addr(unsigned long addr)
+{
+     return -EINVAL;

include/linux/uaccess-buffer.h line 100

Quoted 6 lines

include/linux/uaccess-buffer.h new file +6 -0

+}
+static inline void uaccess_buffer_free(struct task_struct *tsk)
+{
+}
+
+#define copy_from_user_nolog(to, from, len) copy_from_user(to, from, len)

"#define copy_from_user_nolog copy_from_user" ?

With uaccess logging the contract is that the kernel must not report accessing more data than necessary, as this can lead to false positive reports in downstream consumers. This generally works out of the box when instrumenting copy_{from,to}_user(), but with the data argument to mount() we use copy_from_user() to copy PAGE_SIZE bytes (or as much as we can, if the PAGE_SIZE sized access failed) and figure out later how much we actually need.

To prevent this from leading to a false positive report, use copy_from_user_nolog(), which will prevent the access from being logged. Recall that it is valid for the kernel to report accessing less data than it actually accessed, as uaccess logging is a best-effort mechanism for reporting uaccesses.

Link https://linux-review.googlesource.com/id/I5629b92a725c817acd9a861288338dd605cafee6

Signed-off-by Peter Collingbourne <[email protected]>

---
 fs/namespace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

fs/namespace.c +1 -0

index 659a8f39c61a..8f5f2aaca64e 100644
@@ -31,6 +31,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>
+#include <linux/uaccess-buffer.h>
 
 #include "pnode.h"
 #include "internal.h"

fs/namespace.c +0 -1

        /*
         * Not all architectures have an exact copy_from_user(). Resort to
--

2.34.1.173.g76aa8bc2d0-goog

Quoted 34 lines · Peter Collingbourne

With uaccess logging the contract is that the kernel must not report accessing more data than necessary, as this can lead to false positive reports in downstream consumers. This generally works out of the box when instrumenting copy_{from,to}_user(), but with the data argument to mount() we use copy_from_user() to copy PAGE_SIZE bytes (or as much as we can, if the PAGE_SIZE sized access failed) and figure out later how much we actually need.

To prevent this from leading to a false positive report, use copy_from_user_nolog(), which will prevent the access from being logged. Recall that it is valid for the kernel to report accessing less data than it actually accessed, as uaccess logging is a best-effort mechanism for reporting uaccesses.

Link https://linux-review.googlesource.com/id/I5629b92a725c817acd9a861288338dd605cafee6

Signed-off-by Peter Collingbourne <[email protected]>

---
 fs/namespace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

fs/namespace.c +1 -0

index 659a8f39c61a..8f5f2aaca64e 100644
@@ -31,6 +31,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>
+#include <linux/uaccess-buffer.h>
 
 #include "pnode.h"
 #include "internal.h"

fs/namespace.c static void *copy_mount_options(const void __user * data) · line 3206

Quoted 10 lines

fs/namespace.c +6 -1

@@ -3197,7 +3198,12 @@ static void *copy_mount_options(const void __user * data)
        if (!copy)
                return ERR_PTR(-ENOMEM);
 
-       left = copy_from_user(copy, data, PAGE_SIZE);
+       /*
+        * Use copy_from_user_nolog to avoid reporting overly large accesses in
+        * the uaccess buffer, as this can lead to false positive reports in
+        * downstream consumers.
+        */
+       left = copy_from_user_nolog(copy, data, PAGE_SIZE);

A late idea...
Maybe it's better to log them with a new UACCESS_BUFFER_FLAG_OVERREAD flag. Better for user-space, at least can detect UAFs by checking the first byte. And a more logical kernel annotation (maybe will be used in some other tools? or if we ever check user tags in the kernel).

Probably not too important today since we use this only in 2 places, but longer term may be better.

Btw, what's the story with BPF accesses? Can we log them theoretically?

Previously the comment said:

fs/namespace.c +4 -0

+       /*
+        * Avoid copy_from_user() here as it may leak information about the BPF
+        * program to userspace via the uaccess buffer.
+        */

but now it says something very generic:

/*
* Avoid logging uaccesses here as the BPF program may not be following
* the uaccess log rules.
*/

Quoted 50 lines

Quoted 33 lines

With uaccess logging the contract is that the kernel must not report accessing more data than necessary, as this can lead to false positive reports in downstream consumers. This generally works out of the box when instrumenting copy_{from,to}_user(), but with the data argument to mount() we use copy_from_user() to copy PAGE_SIZE bytes (or as much as we can, if the PAGE_SIZE sized access failed) and figure out later how much we actually need.

To prevent this from leading to a false positive report, use copy_from_user_nolog(), which will prevent the access from being logged. Recall that it is valid for the kernel to report accessing less data than it actually accessed, as uaccess logging is a best-effort mechanism for reporting uaccesses.

Link https://linux-review.googlesource.com/id/I5629b92a725c817acd9a861288338dd605cafee6

Signed-off-by Peter Collingbourne <[email protected]>

---
 fs/namespace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

fs/namespace.c +1 -0

index 659a8f39c61a..8f5f2aaca64e 100644
@@ -31,6 +31,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>
+#include <linux/uaccess-buffer.h>
 
 #include "pnode.h"
 #include "internal.h"

fs/namespace.c static void *copy_mount_options(const void __user * data) · line 3206

Quoted 10 lines

fs/namespace.c +6 -1

@@ -3197,7 +3198,12 @@ static void *copy_mount_options(const void __user * data)
        if (!copy)
                return ERR_PTR(-ENOMEM);
 
-       left = copy_from_user(copy, data, PAGE_SIZE);
+       /*
+        * Use copy_from_user_nolog to avoid reporting overly large accesses in
+        * the uaccess buffer, as this can lead to false positive reports in
+        * downstream consumers.
+        */
+       left = copy_from_user_nolog(copy, data, PAGE_SIZE);

A late idea...
Maybe it's better to log them with a new UACCESS_BUFFER_FLAG_OVERREAD flag. Better for user-space, at least can detect UAFs by checking the first byte. And a more logical kernel annotation (maybe will be used in some other tools? or if we ever check user tags in the kernel).

Quoted 2 lines

Probably not too important today since we use this only in 2 places, but longer term may be better.

I'm not sure about this. The overreads are basically an implementation detail of the kernel, so I'm not sure it makes sense to expose them. A scheme where we expose all overreads wouldn't necessarily help with UAF, because what if for example the kernel reads *behind* the user-provided pointer? I guess it could lead to false positives.

Quoted 9 lines

Btw, what's the story with BPF accesses? Can we log them theoretically?

Previously the comment said:

fs/namespace.c +4 -0

+       /*
+        * Avoid copy_from_user() here as it may leak information about the BPF
+        * program to userspace via the uaccess buffer.
+        */

Quoted 6 lines

but now it says something very generic:

/*
* Avoid logging uaccesses here as the BPF program may not be following
* the uaccess log rules.
*/

Yes we should be able to log them theoretically, but we don't need to do that now. See my reply here:

https://lore.kernel.org/all/CAMn1gO5B5Q3hfN6kugv2wmdFGNhJb75iRX1zmCkw3wnueN1dtg@mail.gmail.com/#:~:text=This%20comment%20was,the%20comment%20accordingly.

Quoted 50 lines · Peter Collingbourne

Quoted 50 lines

Quoted 33 lines

With uaccess logging the contract is that the kernel must not report accessing more data than necessary, as this can lead to false positive reports in downstream consumers. This generally works out of the box when instrumenting copy_{from,to}_user(), but with the data argument to mount() we use copy_from_user() to copy PAGE_SIZE bytes (or as much as we can, if the PAGE_SIZE sized access failed) and figure out later how much we actually need.

To prevent this from leading to a false positive report, use copy_from_user_nolog(), which will prevent the access from being logged. Recall that it is valid for the kernel to report accessing less data than it actually accessed, as uaccess logging is a best-effort mechanism for reporting uaccesses.

Link https://linux-review.googlesource.com/id/I5629b92a725c817acd9a861288338dd605cafee6

Signed-off-by Peter Collingbourne <[email protected]>

---
 fs/namespace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

fs/namespace.c +1 -0

index 659a8f39c61a..8f5f2aaca64e 100644
@@ -31,6 +31,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>
+#include <linux/uaccess-buffer.h>
 
 #include "pnode.h"
 #include "internal.h"

fs/namespace.c static void *copy_mount_options(const void __user * data) · line 3206

Quoted 10 lines

fs/namespace.c +6 -1

@@ -3197,7 +3198,12 @@ static void *copy_mount_options(const void __user * data)
        if (!copy)
                return ERR_PTR(-ENOMEM);
 
-       left = copy_from_user(copy, data, PAGE_SIZE);
+       /*
+        * Use copy_from_user_nolog to avoid reporting overly large accesses in
+        * the uaccess buffer, as this can lead to false positive reports in
+        * downstream consumers.
+        */
+       left = copy_from_user_nolog(copy, data, PAGE_SIZE);

A late idea...
Maybe it's better to log them with a new UACCESS_BUFFER_FLAG_OVERREAD flag. Better for user-space, at least can detect UAFs by checking the first byte. And a more logical kernel annotation (maybe will be used in some other tools? or if we ever check user tags in the kernel).

Quoted 8 lines

Quoted 2 lines

Probably not too important today since we use this only in 2 places, but longer term may be better.

I'm not sure about this. The overreads are basically an implementation detail of the kernel, so I'm not sure it makes sense to expose them. A scheme where we expose all overreads wouldn't necessarily help with UAF, because what if for example the kernel reads *behind* the user-provided pointer? I guess it could lead to false positives.

If user-space uses logging to check addressability, then it can safely check only the first byte (right? there must be at least 1 byte passed by user-space at that address). And that's enough to detect UAFs.

Quoted 16 lines

Quoted 9 lines

Btw, what's the story with BPF accesses? Can we log them theoretically?

Previously the comment said:

fs/namespace.c +4 -0

+       /*
+        * Avoid copy_from_user() here as it may leak information about the BPF
+        * program to userspace via the uaccess buffer.
+        */

Quoted 6 lines

but now it says something very generic:

/*
* Avoid logging uaccesses here as the BPF program may not be following
* the uaccess log rules.
*/

Quoted 4 lines

Yes we should be able to log them theoretically, but we don't need to do that now. See my reply here:

https://lore.kernel.org/all/CAMn1gO5B5Q3hfN6kugv2wmdFGNhJb75iRX1zmCkw3wnueN1dtg@mail.gmail.com/#:~:text=This%20comment%20was,the%20comment%20accordingly.

I see. These could be marked with another flag.
I don't have a strong opinion about this. But I am mentioning this because my experience is that it's better to expose more raw info from kernel in these cases, rather than hardcoding policies into kernel code (what's ignored/why/when) b/c a delay from another kernel change to wide deployment is 5+ years and user-space code may need to detect and deal with all various versions of the kernel logic. Say, fuzzing may still want to know about the mount options (rather than no signal that the kernel reads at least something at that address). But adding them later with a flag is not really a backwards compatible change b/c you now have addressability checking code that's not checking the new flag and will produce false positives.

Quoted 60 lines · Peter Collingbourne

Quoted 58 lines

Quoted 50 lines

Quoted 33 lines

With uaccess logging the contract is that the kernel must not report accessing more data than necessary, as this can lead to false positive reports in downstream consumers. This generally works out of the box when instrumenting copy_{from,to}_user(), but with the data argument to mount() we use copy_from_user() to copy PAGE_SIZE bytes (or as much as we can, if the PAGE_SIZE sized access failed) and figure out later how much we actually need.

To prevent this from leading to a false positive report, use copy_from_user_nolog(), which will prevent the access from being logged. Recall that it is valid for the kernel to report accessing less data than it actually accessed, as uaccess logging is a best-effort mechanism for reporting uaccesses.

Link https://linux-review.googlesource.com/id/I5629b92a725c817acd9a861288338dd605cafee6

Signed-off-by Peter Collingbourne <[email protected]>

---
 fs/namespace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

fs/namespace.c +1 -0

index 659a8f39c61a..8f5f2aaca64e 100644
@@ -31,6 +31,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>
+#include <linux/uaccess-buffer.h>
 
 #include "pnode.h"
 #include "internal.h"

fs/namespace.c static void *copy_mount_options(const void __user * data) · line 3206

Quoted 10 lines

fs/namespace.c +6 -1

@@ -3197,7 +3198,12 @@ static void *copy_mount_options(const void __user * data)
        if (!copy)
                return ERR_PTR(-ENOMEM);
 
-       left = copy_from_user(copy, data, PAGE_SIZE);
+       /*
+        * Use copy_from_user_nolog to avoid reporting overly large accesses in
+        * the uaccess buffer, as this can lead to false positive reports in
+        * downstream consumers.
+        */
+       left = copy_from_user_nolog(copy, data, PAGE_SIZE);

A late idea...
Maybe it's better to log them with a new UACCESS_BUFFER_FLAG_OVERREAD flag. Better for user-space, at least can detect UAFs by checking the first byte. And a more logical kernel annotation (maybe will be used in some other tools? or if we ever check user tags in the kernel).

Quoted 2 lines

Probably not too important today since we use this only in 2 places, but longer term may be better.

I'm not sure about this. The overreads are basically an implementation detail of the kernel, so I'm not sure it makes sense to expose them. A scheme where we expose all overreads wouldn't necessarily help with UAF, because what if for example the kernel reads *behind* the user-provided pointer? I guess it could lead to false positives.

Quoted 3 lines

If user-space uses logging to check addressability, then it can safely check only the first byte (right? there must be at least 1 byte passed by user-space at that address). And that's enough to detect UAFs.

I was thinking more e.g. what if the kernel reads an entire page with copy_from_user() and takes a subset of it later. Then the first byte could point to some other random allocation in the same page and lead to a false UAF report if we just consider the first byte.

So I think the use cases for accesses with this flag set may be limited to things like fuzzers.

Quoted 21 lines

Quoted 16 lines

Quoted 9 lines

Btw, what's the story with BPF accesses? Can we log them theoretically?

Previously the comment said:

fs/namespace.c +4 -0

+       /*
+        * Avoid copy_from_user() here as it may leak information about the BPF
+        * program to userspace via the uaccess buffer.
+        */

Quoted 6 lines

but now it says something very generic:

/*
* Avoid logging uaccesses here as the BPF program may not be following
* the uaccess log rules.
*/

Quoted 4 lines

Yes we should be able to log them theoretically, but we don't need to do that now. See my reply here:

https://lore.kernel.org/all/CAMn1gO5B5Q3hfN6kugv2wmdFGNhJb75iRX1zmCkw3wnueN1dtg@mail.gmail.com/#:~:text=This%20comment%20was,the%20comment%20accordingly.

Quoted 12 lines

I see. These could be marked with another flag.
I don't have a strong opinion about this. But I am mentioning this because my experience is that it's better to expose more raw info from kernel in these cases, rather than hardcoding policies into kernel code (what's ignored/why/when) b/c a delay from another kernel change to wide deployment is 5+ years and user-space code may need to detect and deal with all various versions of the kernel logic. Say, fuzzing may still want to know about the mount options (rather than no signal that the kernel reads at least something at that address). But adding them later with a flag is not really a backwards compatible change b/c you now have addressability checking code that's not checking the new flag and will produce false positives.

I think this is a good point. I'll see about adding flags for the BPF and overread cases.

Peter

Add uaccess logging support on architectures that use CONFIG_GENERIC_ENTRY (currently only s390 and x86).

Link https://linux-review.googlesource.com/id/I3c5eb19a7e4a1dbe6095f6971f7826c4b0663f7d

Signed-off-by Peter Collingbourne <[email protected]>

---
 arch/Kconfig                 |  6 ++++++
 include/linux/entry-common.h |  2 ++
 include/linux/thread_info.h  |  4 ++++
 kernel/entry/common.c        | 10 ++++++++++
 4 files changed, 22 insertions(+)

arch/Kconfig +6 -0

index d3c4ab249e9c..c4dcab5279ac 100644
@@ -31,6 +31,7 @@ config HOTPLUG_SMT
        bool
 
 config GENERIC_ENTRY
+       select HAVE_ARCH_UACCESS_BUFFER
        bool
 
 config KPROBES

@@ -1312,6 +1313,11 @@ config ARCH_HAS_PARANOID_L1D_FLUSH
 config DYNAMIC_SIGFRAME
        bool
 
+config HAVE_ARCH_UACCESS_BUFFER
+       bool
+       help
+         Select if the architecture's syscall entry/exit code supports uaccess buffers.
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"

include/linux/entry-common.h +2 -0

index 2e2b8d6140ed..973fcd1d48a3 100644
@@ -42,12 +42,14 @@
                                 SYSCALL_WORK_SYSCALL_EMU |             \
                                 SYSCALL_WORK_SYSCALL_AUDIT |           \
                                 SYSCALL_WORK_SYSCALL_USER_DISPATCH |   \
+                                SYSCALL_WORK_UACCESS_BUFFER_ENTRY |    \
                                 ARCH_SYSCALL_WORK_ENTER)
 #define SYSCALL_WORK_EXIT      (SYSCALL_WORK_SYSCALL_TRACEPOINT |      \
                                 SYSCALL_WORK_SYSCALL_TRACE |           \
                                 SYSCALL_WORK_SYSCALL_AUDIT |           \
                                 SYSCALL_WORK_SYSCALL_USER_DISPATCH |   \
                                 SYSCALL_WORK_SYSCALL_EXIT_TRAP |       \
+                                SYSCALL_WORK_UACCESS_BUFFER_EXIT |     \
                                 ARCH_SYSCALL_WORK_EXIT)
 
 /*

include/linux/thread_info.h +4 -0

index ad0c4e041030..b0f8ea86967f 100644
@@ -46,6 +46,8 @@ enum syscall_work_bit {
        SYSCALL_WORK_BIT_SYSCALL_AUDIT,
        SYSCALL_WORK_BIT_SYSCALL_USER_DISPATCH,
        SYSCALL_WORK_BIT_SYSCALL_EXIT_TRAP,
+       SYSCALL_WORK_BIT_UACCESS_BUFFER_ENTRY,
+       SYSCALL_WORK_BIT_UACCESS_BUFFER_EXIT,
 };
 
 #define SYSCALL_WORK_SECCOMP           BIT(SYSCALL_WORK_BIT_SECCOMP)

@@ -55,6 +57,8 @@ enum syscall_work_bit {
 #define SYSCALL_WORK_SYSCALL_AUDIT     BIT(SYSCALL_WORK_BIT_SYSCALL_AUDIT)
 #define SYSCALL_WORK_SYSCALL_USER_DISPATCH BIT(SYSCALL_WORK_BIT_SYSCALL_USER_DISPATCH)
 #define SYSCALL_WORK_SYSCALL_EXIT_TRAP BIT(SYSCALL_WORK_BIT_SYSCALL_EXIT_TRAP)
+#define SYSCALL_WORK_UACCESS_BUFFER_ENTRY      BIT(SYSCALL_WORK_BIT_UACCESS_BUFFER_ENTRY)
+#define SYSCALL_WORK_UACCESS_BUFFER_EXIT       BIT(SYSCALL_WORK_BIT_UACCESS_BUFFER_EXIT)
 #endif
 
 #include <asm/thread_info.h>

kernel/entry/common.c +10 -1

index d5a61d565ad5..57c4bb01a554 100644
@@ -6,6 +6,7 @@
 #include <linux/livepatch.h>
 #include <linux/audit.h>
 #include <linux/tick.h>
+#include <linux/uaccess-buffer.h>
 
 #include "common.h"
 

@@ -70,6 +71,9 @@ static long syscall_trace_enter(struct pt_regs *regs, long syscall,
                        return ret;
        }
 
+       if (work & SYSCALL_WORK_UACCESS_BUFFER_ENTRY)
+               uaccess_buffer_syscall_entry();
+
        /* Either of the above might have changed the syscall number */
        syscall = syscall_get_nr(current, regs);
 

@@ -197,14 +201,17 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
 static void exit_to_user_mode_prepare(struct pt_regs *regs)
 {
        unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
+       bool uaccess_buffer_pending;
 
        lockdep_assert_irqs_disabled();
 
        /* Flush pending rcuog wakeup before the last need_resched() check */
        tick_nohz_user_enter_prepare();
 
+       uaccess_buffer_pending = uaccess_buffer_pre_exit_loop();
        if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
                ti_work = exit_to_user_mode_loop(regs, ti_work);
+       uaccess_buffer_post_exit_loop(uaccess_buffer_pending);
 
        arch_exit_to_user_mode_prepare(regs, ti_work);
 

@@ -247,6 +254,9 @@ static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
 
        audit_syscall_exit(regs);
 
+       if (work & SYSCALL_WORK_UACCESS_BUFFER_EXIT)
+               uaccess_buffer_syscall_exit();
+
        if (work & SYSCALL_WORK_SYSCALL_TRACEPOINT)
                trace_sys_exit(regs, syscall_get_return_value(current, regs));
 
--

2.34.1.173.g76aa8bc2d0-goog

arm64 does not use CONFIG_GENERIC_ENTRY, so add the support for uaccess logging directly to the architecture.

Link https://linux-review.googlesource.com/id/I88de539fb9c4a9d27fa8cccbe201a6e4382faf89

Signed-off-by Peter Collingbourne <[email protected]>

---
 arch/arm64/Kconfig                   | 1 +
 arch/arm64/include/asm/thread_info.h | 7 ++++++-
 arch/arm64/kernel/ptrace.c           | 7 +++++++
 arch/arm64/kernel/signal.c           | 5 +++++
 arch/arm64/kernel/syscall.c          | 1 +
 5 files changed, 20 insertions(+), 1 deletion(-)

arch/arm64/Kconfig +1 -0

index c4207cf9bb17..6023946abe4a 100644
@@ -161,6 +161,7 @@ config ARM64
        select HAVE_ARCH_THREAD_STRUCT_WHITELIST
        select HAVE_ARCH_TRACEHOOK
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+       select HAVE_ARCH_UACCESS_BUFFER
        select HAVE_ARCH_VMAP_STACK
        select HAVE_ARM_SMCCC
        select HAVE_ASM_MODVERSIONS

arch/arm64/include/asm/thread_info.h +6 -1

index e1317b7c4525..0461b36251ea 100644
@@ -82,6 +82,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 #define TIF_SVE_VL_INHERIT     24      /* Inherit SVE vl_onexec across exec */
 #define TIF_SSBD               25      /* Wants SSB mitigation */
 #define TIF_TAGGED_ADDR                26      /* Allow tagged user addresses */
+#define TIF_UACCESS_BUFFER_ENTRY 27     /* thread has non-zero uaccess_desc_addr_addr */
+#define TIF_UACCESS_BUFFER_EXIT  28     /* thread has non-zero kcur */
 
 #define _TIF_SIGPENDING                (1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED      (1 << TIF_NEED_RESCHED)

@@ -98,6 +100,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 #define _TIF_SVE               (1 << TIF_SVE)
 #define _TIF_MTE_ASYNC_FAULT   (1 << TIF_MTE_ASYNC_FAULT)
 #define _TIF_NOTIFY_SIGNAL     (1 << TIF_NOTIFY_SIGNAL)
+#define _TIF_UACCESS_BUFFER_ENTRY      (1 << TIF_UACCESS_BUFFER_ENTRY)
+#define _TIF_UACCESS_BUFFER_EXIT       (1 << TIF_UACCESS_BUFFER_EXIT)
 
 #define _TIF_WORK_MASK         (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
                                 _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \

@@ -106,7 +110,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 
 #define _TIF_SYSCALL_WORK      (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
                                 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
-                                _TIF_SYSCALL_EMU)
+                                _TIF_SYSCALL_EMU | _TIF_UACCESS_BUFFER_ENTRY | \
+                                _TIF_UACCESS_BUFFER_EXIT)
 
 #ifdef CONFIG_SHADOW_CALL_STACK
 #define INIT_SCS                                                       \

arch/arm64/kernel/ptrace.c +7 -0

index 88a9034fb9b5..283372eccaeb 100644
@@ -29,6 +29,7 @@
 #include <linux/regset.h>
 #include <linux/tracehook.h>
 #include <linux/elf.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/compat.h>
 #include <asm/cpufeature.h>

@@ -1854,6 +1855,9 @@ int syscall_trace_enter(struct pt_regs *regs)
        if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
                trace_sys_enter(regs, regs->syscallno);
 
+       if (flags & _TIF_UACCESS_BUFFER_ENTRY)
+               uaccess_buffer_syscall_entry();
+
        audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
                            regs->regs[2], regs->regs[3]);
 

@@ -1866,6 +1870,9 @@ void syscall_trace_exit(struct pt_regs *regs)
 
        audit_syscall_exit(regs);
 
+       if (flags & _TIF_UACCESS_BUFFER_EXIT)
+               uaccess_buffer_syscall_exit();
+
        if (flags & _TIF_SYSCALL_TRACEPOINT)
                trace_sys_exit(regs, syscall_get_return_value(current, regs));
 

arch/arm64/kernel/signal.c +5 -0

index 8f6372b44b65..5bbd98e5c257 100644
@@ -20,6 +20,7 @@
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
 #include <linux/syscalls.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/daifflags.h>
 #include <asm/debug-monitors.h>

@@ -919,6 +920,8 @@ static void do_signal(struct pt_regs *regs)
 
 void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
 {
+       bool uaccess_buffer_pending = uaccess_buffer_pre_exit_loop();
+
        do {
                if (thread_flags & _TIF_NEED_RESCHED) {
                        /* Unmask Debug and SError for the next task */

@@ -950,6 +953,8 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
                local_daif_mask();
                thread_flags = READ_ONCE(current_thread_info()->flags);
        } while (thread_flags & _TIF_WORK_MASK);
+
+       uaccess_buffer_post_exit_loop(uaccess_buffer_pending);
 }
 
 unsigned long __ro_after_init signal_minsigstksz;

arch/arm64/kernel/syscall.c +0 -0

index 50a0f1a38e84..d59022b594f2 100644

arch/arm64/kernel/syscall.c +0 -1

 #include <asm/daifflags.h>
 #include <asm/debug-monitors.h>
--

2.34.1.173.g76aa8bc2d0-goog

Quoted 119 lines · Peter Collingbourne

arm64 does not use CONFIG_GENERIC_ENTRY, so add the support for uaccess logging directly to the architecture.

Link https://linux-review.googlesource.com/id/I88de539fb9c4a9d27fa8cccbe201a6e4382faf89

Signed-off-by Peter Collingbourne <[email protected]>

---
 arch/arm64/Kconfig                   | 1 +
 arch/arm64/include/asm/thread_info.h | 7 ++++++-
 arch/arm64/kernel/ptrace.c           | 7 +++++++
 arch/arm64/kernel/signal.c           | 5 +++++
 arch/arm64/kernel/syscall.c          | 1 +
 5 files changed, 20 insertions(+), 1 deletion(-)

arch/arm64/Kconfig +1 -0

index c4207cf9bb17..6023946abe4a 100644
@@ -161,6 +161,7 @@ config ARM64
        select HAVE_ARCH_THREAD_STRUCT_WHITELIST
        select HAVE_ARCH_TRACEHOOK
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+       select HAVE_ARCH_UACCESS_BUFFER
        select HAVE_ARCH_VMAP_STACK
        select HAVE_ARM_SMCCC
        select HAVE_ASM_MODVERSIONS

arch/arm64/include/asm/thread_info.h +6 -1

index e1317b7c4525..0461b36251ea 100644
@@ -82,6 +82,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 #define TIF_SVE_VL_INHERIT     24      /* Inherit SVE vl_onexec across exec */
 #define TIF_SSBD               25      /* Wants SSB mitigation */
 #define TIF_TAGGED_ADDR                26      /* Allow tagged user addresses */
+#define TIF_UACCESS_BUFFER_ENTRY 27     /* thread has non-zero uaccess_desc_addr_addr */
+#define TIF_UACCESS_BUFFER_EXIT  28     /* thread has non-zero kcur */
 
 #define _TIF_SIGPENDING                (1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED      (1 << TIF_NEED_RESCHED)

@@ -98,6 +100,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 #define _TIF_SVE               (1 << TIF_SVE)
 #define _TIF_MTE_ASYNC_FAULT   (1 << TIF_MTE_ASYNC_FAULT)
 #define _TIF_NOTIFY_SIGNAL     (1 << TIF_NOTIFY_SIGNAL)
+#define _TIF_UACCESS_BUFFER_ENTRY      (1 << TIF_UACCESS_BUFFER_ENTRY)
+#define _TIF_UACCESS_BUFFER_EXIT       (1 << TIF_UACCESS_BUFFER_EXIT)
 
 #define _TIF_WORK_MASK         (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
                                 _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \

@@ -106,7 +110,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 
 #define _TIF_SYSCALL_WORK      (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
                                 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
-                                _TIF_SYSCALL_EMU)
+                                _TIF_SYSCALL_EMU | _TIF_UACCESS_BUFFER_ENTRY | \
+                                _TIF_UACCESS_BUFFER_EXIT)
 
 #ifdef CONFIG_SHADOW_CALL_STACK
 #define INIT_SCS                                                       \

arch/arm64/kernel/ptrace.c +7 -0

index 88a9034fb9b5..283372eccaeb 100644
@@ -29,6 +29,7 @@
 #include <linux/regset.h>
 #include <linux/tracehook.h>
 #include <linux/elf.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/compat.h>
 #include <asm/cpufeature.h>

@@ -1854,6 +1855,9 @@ int syscall_trace_enter(struct pt_regs *regs)
        if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
                trace_sys_enter(regs, regs->syscallno);
 
+       if (flags & _TIF_UACCESS_BUFFER_ENTRY)
+               uaccess_buffer_syscall_entry();
+
        audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
                            regs->regs[2], regs->regs[3]);
 

@@ -1866,6 +1870,9 @@ void syscall_trace_exit(struct pt_regs *regs)
 
        audit_syscall_exit(regs);
 
+       if (flags & _TIF_UACCESS_BUFFER_EXIT)
+               uaccess_buffer_syscall_exit();
+
        if (flags & _TIF_SYSCALL_TRACEPOINT)
                trace_sys_exit(regs, syscall_get_return_value(current, regs));
 

arch/arm64/kernel/signal.c +5 -0

index 8f6372b44b65..5bbd98e5c257 100644
@@ -20,6 +20,7 @@
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
 #include <linux/syscalls.h>
+#include <linux/uaccess-buffer.h>
 
 #include <asm/daifflags.h>
 #include <asm/debug-monitors.h>

@@ -919,6 +920,8 @@ static void do_signal(struct pt_regs *regs)
 
 void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
 {
+       bool uaccess_buffer_pending = uaccess_buffer_pre_exit_loop();
+
        do {
                if (thread_flags & _TIF_NEED_RESCHED) {
                        /* Unmask Debug and SError for the next task */

@@ -950,6 +953,8 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
                local_daif_mask();
                thread_flags = READ_ONCE(current_thread_info()->flags);
        } while (thread_flags & _TIF_WORK_MASK);
+
+       uaccess_buffer_post_exit_loop(uaccess_buffer_pending);
 }