Skip to content

Commit fd881d0

Browse files
mjeansonIngo Molnar
authored and
Ingo Molnar
committed
rseq: Fix segfault on registration when rseq_cs is non-zero
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 <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Reviewed-by: Mathieu Desnoyers <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Linus Torvalds <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 82354fc commit fd881d0

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
@@ -236,6 +236,29 @@ static int rseq_reset_rseq_cpu_node_id(struct task_struct *t)
236236
return -EFAULT;
237237
}
238238

239+
/*
240+
* Get the user-space pointer value stored in the 'rseq_cs' field.
241+
*/
242+
static int rseq_get_rseq_cs_ptr_val(struct rseq __user *rseq, u64 *rseq_cs)
243+
{
244+
if (!rseq_cs)
245+
return -EFAULT;
246+
247+
#ifdef CONFIG_64BIT
248+
if (get_user(*rseq_cs, &rseq->rseq_cs))
249+
return -EFAULT;
250+
#else
251+
if (copy_from_user(rseq_cs, &rseq->rseq_cs, sizeof(*rseq_cs)))
252+
return -EFAULT;
253+
#endif
254+
255+
return 0;
256+
}
257+
258+
/*
259+
* If the rseq_cs field of 'struct rseq' contains a valid pointer to
260+
* user-space, copy 'struct rseq_cs' from user-space and validate its fields.
261+
*/
239262
static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
240263
{
241264
struct rseq_cs __user *urseq_cs;
@@ -244,17 +267,16 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
244267
u32 sig;
245268
int ret;
246269

247-
#ifdef CONFIG_64BIT
248-
if (get_user(ptr, &t->rseq->rseq_cs))
249-
return -EFAULT;
250-
#else
251-
if (copy_from_user(&ptr, &t->rseq->rseq_cs, sizeof(ptr)))
252-
return -EFAULT;
253-
#endif
270+
ret = rseq_get_rseq_cs_ptr_val(t->rseq, &ptr);
271+
if (ret)
272+
return ret;
273+
274+
/* If the rseq_cs pointer is NULL, return a cleared struct rseq_cs. */
254275
if (!ptr) {
255276
memset(rseq_cs, 0, sizeof(*rseq_cs));
256277
return 0;
257278
}
279+
/* Check that the pointer value fits in the user-space process space. */
258280
if (ptr >= TASK_SIZE)
259281
return -EINVAL;
260282
urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;
@@ -330,7 +352,7 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags)
330352
return !!event_mask;
331353
}
332354

333-
static int clear_rseq_cs(struct task_struct *t)
355+
static int clear_rseq_cs(struct rseq __user *rseq)
334356
{
335357
/*
336358
* The rseq_cs field is set to NULL on preemption or signal
@@ -341,9 +363,9 @@ static int clear_rseq_cs(struct task_struct *t)
341363
* Set rseq_cs to NULL.
342364
*/
343365
#ifdef CONFIG_64BIT
344-
return put_user(0UL, &t->rseq->rseq_cs);
366+
return put_user(0UL, &rseq->rseq_cs);
345367
#else
346-
if (clear_user(&t->rseq->rseq_cs, sizeof(t->rseq->rseq_cs)))
368+
if (clear_user(&rseq->rseq_cs, sizeof(rseq->rseq_cs)))
347369
return -EFAULT;
348370
return 0;
349371
#endif
@@ -375,11 +397,11 @@ static int rseq_ip_fixup(struct pt_regs *regs)
375397
* Clear the rseq_cs pointer and return.
376398
*/
377399
if (!in_rseq_cs(ip, &rseq_cs))
378-
return clear_rseq_cs(t);
400+
return clear_rseq_cs(t->rseq);
379401
ret = rseq_need_restart(t, rseq_cs.flags);
380402
if (ret <= 0)
381403
return ret;
382-
ret = clear_rseq_cs(t);
404+
ret = clear_rseq_cs(t->rseq);
383405
if (ret)
384406
return ret;
385407
trace_rseq_ip_fixup(ip, rseq_cs.start_ip, rseq_cs.post_commit_offset,
@@ -453,6 +475,7 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
453475
int, flags, u32, sig)
454476
{
455477
int ret;
478+
u64 rseq_cs;
456479

457480
if (flags & RSEQ_FLAG_UNREGISTER) {
458481
if (flags & ~RSEQ_FLAG_UNREGISTER)
@@ -507,6 +530,19 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
507530
return -EINVAL;
508531
if (!access_ok(rseq, rseq_len))
509532
return -EFAULT;
533+
534+
/*
535+
* If the rseq_cs pointer is non-NULL on registration, clear it to
536+
* avoid a potential segfault on return to user-space. The proper thing
537+
* to do would have been to fail the registration but this would break
538+
* older libcs that reuse the rseq area for new threads without
539+
* clearing the fields.
540+
*/
541+
if (rseq_get_rseq_cs_ptr_val(rseq, &rseq_cs))
542+
return -EFAULT;
543+
if (rseq_cs && clear_rseq_cs(rseq))
544+
return -EFAULT;
545+
510546
#ifdef CONFIG_DEBUG_RSEQ
511547
/*
512548
* Initialize the in-kernel rseq fields copy for validation of

0 commit comments

Comments
 (0)