Skip to content

Commit f004f58

Browse files
mjeansongregkh
authored andcommitted
rseq: Fix segfault on registration when rseq_cs is non-zero
commit fd881d0 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>
1 parent f2133b8 commit f004f58

File tree

1 file changed

+48
-12
lines changed

1 file changed

+48
-12
lines changed

kernel/rseq.c

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,29 @@ static int rseq_reset_rseq_cpu_node_id(struct task_struct *t)
149149
return 0;
150150
}
151151

152+
/*
153+
* Get the user-space pointer value stored in the 'rseq_cs' field.
154+
*/
155+
static int rseq_get_rseq_cs_ptr_val(struct rseq __user *rseq, u64 *rseq_cs)
156+
{
157+
if (!rseq_cs)
158+
return -EFAULT;
159+
160+
#ifdef CONFIG_64BIT
161+
if (get_user(*rseq_cs, &rseq->rseq_cs))
162+
return -EFAULT;
163+
#else
164+
if (copy_from_user(rseq_cs, &rseq->rseq_cs, sizeof(*rseq_cs)))
165+
return -EFAULT;
166+
#endif
167+
168+
return 0;
169+
}
170+
171+
/*
172+
* If the rseq_cs field of 'struct rseq' contains a valid pointer to
173+
* user-space, copy 'struct rseq_cs' from user-space and validate its fields.
174+
*/
152175
static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
153176
{
154177
struct rseq_cs __user *urseq_cs;
@@ -157,17 +180,16 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
157180
u32 sig;
158181
int ret;
159182

160-
#ifdef CONFIG_64BIT
161-
if (get_user(ptr, &t->rseq->rseq_cs))
162-
return -EFAULT;
163-
#else
164-
if (copy_from_user(&ptr, &t->rseq->rseq_cs, sizeof(ptr)))
165-
return -EFAULT;
166-
#endif
183+
ret = rseq_get_rseq_cs_ptr_val(t->rseq, &ptr);
184+
if (ret)
185+
return ret;
186+
187+
/* If the rseq_cs pointer is NULL, return a cleared struct rseq_cs. */
167188
if (!ptr) {
168189
memset(rseq_cs, 0, sizeof(*rseq_cs));
169190
return 0;
170191
}
192+
/* Check that the pointer value fits in the user-space process space. */
171193
if (ptr >= TASK_SIZE)
172194
return -EINVAL;
173195
urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;
@@ -243,7 +265,7 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags)
243265
return !!event_mask;
244266
}
245267

246-
static int clear_rseq_cs(struct task_struct *t)
268+
static int clear_rseq_cs(struct rseq __user *rseq)
247269
{
248270
/*
249271
* The rseq_cs field is set to NULL on preemption or signal
@@ -254,9 +276,9 @@ static int clear_rseq_cs(struct task_struct *t)
254276
* Set rseq_cs to NULL.
255277
*/
256278
#ifdef CONFIG_64BIT
257-
return put_user(0UL, &t->rseq->rseq_cs);
279+
return put_user(0UL, &rseq->rseq_cs);
258280
#else
259-
if (clear_user(&t->rseq->rseq_cs, sizeof(t->rseq->rseq_cs)))
281+
if (clear_user(&rseq->rseq_cs, sizeof(rseq->rseq_cs)))
260282
return -EFAULT;
261283
return 0;
262284
#endif
@@ -288,11 +310,11 @@ static int rseq_ip_fixup(struct pt_regs *regs)
288310
* Clear the rseq_cs pointer and return.
289311
*/
290312
if (!in_rseq_cs(ip, &rseq_cs))
291-
return clear_rseq_cs(t);
313+
return clear_rseq_cs(t->rseq);
292314
ret = rseq_need_restart(t, rseq_cs.flags);
293315
if (ret <= 0)
294316
return ret;
295-
ret = clear_rseq_cs(t);
317+
ret = clear_rseq_cs(t->rseq);
296318
if (ret)
297319
return ret;
298320
trace_rseq_ip_fixup(ip, rseq_cs.start_ip, rseq_cs.post_commit_offset,
@@ -366,6 +388,7 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
366388
int, flags, u32, sig)
367389
{
368390
int ret;
391+
u64 rseq_cs;
369392

370393
if (flags & RSEQ_FLAG_UNREGISTER) {
371394
if (flags & ~RSEQ_FLAG_UNREGISTER)
@@ -420,6 +443,19 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
420443
return -EINVAL;
421444
if (!access_ok(rseq, rseq_len))
422445
return -EFAULT;
446+
447+
/*
448+
* If the rseq_cs pointer is non-NULL on registration, clear it to
449+
* avoid a potential segfault on return to user-space. The proper thing
450+
* to do would have been to fail the registration but this would break
451+
* older libcs that reuse the rseq area for new threads without
452+
* clearing the fields.
453+
*/
454+
if (rseq_get_rseq_cs_ptr_val(rseq, &rseq_cs))
455+
return -EFAULT;
456+
if (rseq_cs && clear_rseq_cs(rseq))
457+
return -EFAULT;
458+
423459
current->rseq = rseq;
424460
current->rseq_len = rseq_len;
425461
current->rseq_sig = sig;

0 commit comments

Comments
 (0)