]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
rseq: Fix segfault on registration when rseq_cs is non-zero
authorMichael Jeanson <mjeanson@efficios.com>
Thu, 6 Mar 2025 21:12:21 +0000 (16:12 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 17 Jul 2025 16:32:15 +0000 (18:32 +0200)
commit fd881d0a085fc54354414aed990ccf05f282ba53 upstream.

The rseq_cs field is documented as being set to 0 by user-space prior to
registration, however this is not currently enforced by the kernel. This
can result in a segfault on return to user-space if the value stored in
the rseq_cs field doesn't point to a valid struct rseq_cs.

The correct solution to this would be to fail the rseq registration when
the rseq_cs field is non-zero. However, some older versions of glibc
will reuse the rseq area of previous threads without clearing the
rseq_cs field and will also terminate the process if the rseq
registration fails in a secondary thread. This wasn't caught in testing
because in this case the leftover rseq_cs does point to a valid struct
rseq_cs.

What we can do is clear the rseq_cs field on registration when it's
non-zero which will prevent segfaults on registration and won't break
the glibc versions that reuse rseq areas on thread creation.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250306211223.109455-1-mjeanson@efficios.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
kernel/rseq.c

index d38ab944105d7ff919c8dce46f31499f393f0a51..840927ac417b194213170bf499d027465d11bf37 100644 (file)
@@ -120,6 +120,29 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)
        return 0;
 }
 
+/*
+ * Get the user-space pointer value stored in the 'rseq_cs' field.
+ */
+static int rseq_get_rseq_cs_ptr_val(struct rseq __user *rseq, u64 *rseq_cs)
+{
+       if (!rseq_cs)
+               return -EFAULT;
+
+#ifdef CONFIG_64BIT
+       if (get_user(*rseq_cs, &rseq->rseq_cs))
+               return -EFAULT;
+#else
+       if (copy_from_user(rseq_cs, &rseq->rseq_cs, sizeof(*rseq_cs)))
+               return -EFAULT;
+#endif
+
+       return 0;
+}
+
+/*
+ * If the rseq_cs field of 'struct rseq' contains a valid pointer to
+ * user-space, copy 'struct rseq_cs' from user-space and validate its fields.
+ */
 static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
 {
        struct rseq_cs __user *urseq_cs;
@@ -128,17 +151,16 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
        u32 sig;
        int ret;
 
-#ifdef CONFIG_64BIT
-       if (get_user(ptr, &t->rseq->rseq_cs))
-               return -EFAULT;
-#else
-       if (copy_from_user(&ptr, &t->rseq->rseq_cs, sizeof(ptr)))
-               return -EFAULT;
-#endif
+       ret = rseq_get_rseq_cs_ptr_val(t->rseq, &ptr);
+       if (ret)
+               return ret;
+
+       /* If the rseq_cs pointer is NULL, return a cleared struct rseq_cs. */
        if (!ptr) {
                memset(rseq_cs, 0, sizeof(*rseq_cs));
                return 0;
        }
+       /* Check that the pointer value fits in the user-space process space. */
        if (ptr >= TASK_SIZE)
                return -EINVAL;
        urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;
@@ -214,7 +236,7 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags)
        return !!event_mask;
 }
 
-static int clear_rseq_cs(struct task_struct *t)
+static int clear_rseq_cs(struct rseq __user *rseq)
 {
        /*
         * The rseq_cs field is set to NULL on preemption or signal
@@ -225,9 +247,9 @@ static int clear_rseq_cs(struct task_struct *t)
         * Set rseq_cs to NULL.
         */
 #ifdef CONFIG_64BIT
-       return put_user(0UL, &t->rseq->rseq_cs);
+       return put_user(0UL, &rseq->rseq_cs);
 #else
-       if (clear_user(&t->rseq->rseq_cs, sizeof(t->rseq->rseq_cs)))
+       if (clear_user(&rseq->rseq_cs, sizeof(rseq->rseq_cs)))
                return -EFAULT;
        return 0;
 #endif
@@ -259,11 +281,11 @@ static int rseq_ip_fixup(struct pt_regs *regs)
         * Clear the rseq_cs pointer and return.
         */
        if (!in_rseq_cs(ip, &rseq_cs))
-               return clear_rseq_cs(t);
+               return clear_rseq_cs(t->rseq);
        ret = rseq_need_restart(t, rseq_cs.flags);
        if (ret <= 0)
                return ret;
-       ret = clear_rseq_cs(t);
+       ret = clear_rseq_cs(t->rseq);
        if (ret)
                return ret;
        trace_rseq_ip_fixup(ip, rseq_cs.start_ip, rseq_cs.post_commit_offset,
@@ -337,6 +359,7 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
                int, flags, u32, sig)
 {
        int ret;
+       u64 rseq_cs;
 
        if (flags & RSEQ_FLAG_UNREGISTER) {
                if (flags & ~RSEQ_FLAG_UNREGISTER)
@@ -382,6 +405,19 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
                return -EINVAL;
        if (!access_ok(rseq, rseq_len))
                return -EFAULT;
+
+       /*
+        * If the rseq_cs pointer is non-NULL on registration, clear it to
+        * avoid a potential segfault on return to user-space. The proper thing
+        * to do would have been to fail the registration but this would break
+        * older libcs that reuse the rseq area for new threads without
+        * clearing the fields.
+        */
+       if (rseq_get_rseq_cs_ptr_val(rseq, &rseq_cs))
+               return -EFAULT;
+       if (rseq_cs && clear_rseq_cs(rseq))
+               return -EFAULT;
+
        current->rseq = rseq;
        current->rseq_sig = sig;
        /*