Skip to content

Commit b2b05d0

Browse files
mjeansongregkh
authored andcommitted
rseq: Fix segfault on registration when rseq_cs is non-zero
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 <[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] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent e38ec88 commit b2b05d0

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
@@ -120,6 +120,29 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)
120120
return 0;
121121
}
122122

123+
/*
124+
* Get the user-space pointer value stored in the 'rseq_cs' field.
125+
*/
126+
static int rseq_get_rseq_cs_ptr_val(struct rseq __user *rseq, u64 *rseq_cs)
127+
{
128+
if (!rseq_cs)
129+
return -EFAULT;
130+
131+
#ifdef CONFIG_64BIT
132+
if (get_user(*rseq_cs, &rseq->rseq_cs))
133+
return -EFAULT;
134+
#else
135+
if (copy_from_user(rseq_cs, &rseq->rseq_cs, sizeof(*rseq_cs)))
136+
return -EFAULT;
137+
#endif
138+
139+
return 0;
140+
}
141+
142+
/*
143+
* If the rseq_cs field of 'struct rseq' contains a valid pointer to
144+
* user-space, copy 'struct rseq_cs' from user-space and validate its fields.
145+
*/
123146
static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
124147
{
125148
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)
128151
u32 sig;
129152
int ret;
130153

131-
#ifdef CONFIG_64BIT
132-
if (get_user(ptr, &t->rseq->rseq_cs))
133-
return -EFAULT;
134-
#else
135-
if (copy_from_user(&ptr, &t->rseq->rseq_cs, sizeof(ptr)))
136-
return -EFAULT;
137-
#endif
154+
ret = rseq_get_rseq_cs_ptr_val(t->rseq, &ptr);
155+
if (ret)
156+
return ret;
157+
158+
/* If the rseq_cs pointer is NULL, return a cleared struct rseq_cs. */
138159
if (!ptr) {
139160
memset(rseq_cs, 0, sizeof(*rseq_cs));
140161
return 0;
141162
}
163+
/* Check that the pointer value fits in the user-space process space. */
142164
if (ptr >= TASK_SIZE)
143165
return -EINVAL;
144166
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)
214236
return !!event_mask;
215237
}
216238

217-
static int clear_rseq_cs(struct task_struct *t)
239+
static int clear_rseq_cs(struct rseq __user *rseq)
218240
{
219241
/*
220242
* 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)
225247
* Set rseq_cs to NULL.
226248
*/
227249
#ifdef CONFIG_64BIT
228-
return put_user(0UL, &t->rseq->rseq_cs);
250+
return put_user(0UL, &rseq->rseq_cs);
229251
#else
230-
if (clear_user(&t->rseq->rseq_cs, sizeof(t->rseq->rseq_cs)))
252+
if (clear_user(&rseq->rseq_cs, sizeof(rseq->rseq_cs)))
231253
return -EFAULT;
232254
return 0;
233255
#endif
@@ -259,11 +281,11 @@ static int rseq_ip_fixup(struct pt_regs *regs)
259281
* Clear the rseq_cs pointer and return.
260282
*/
261283
if (!in_rseq_cs(ip, &rseq_cs))
262-
return clear_rseq_cs(t);
284+
return clear_rseq_cs(t->rseq);
263285
ret = rseq_need_restart(t, rseq_cs.flags);
264286
if (ret <= 0)
265287
return ret;
266-
ret = clear_rseq_cs(t);
288+
ret = clear_rseq_cs(t->rseq);
267289
if (ret)
268290
return ret;
269291
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,
337359
int, flags, u32, sig)
338360
{
339361
int ret;
362+
u64 rseq_cs;
340363

341364
if (flags & RSEQ_FLAG_UNREGISTER) {
342365
if (flags & ~RSEQ_FLAG_UNREGISTER)
@@ -382,6 +405,19 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, rseq_len,
382405
return -EINVAL;
383406
if (!access_ok(rseq, rseq_len))
384407
return -EFAULT;
408+
409+
/*
410+
* If the rseq_cs pointer is non-NULL on registration, clear it to
411+
* avoid a potential segfault on return to user-space. The proper thing
412+
* to do would have been to fail the registration but this would break
413+
* older libcs that reuse the rseq area for new threads without
414+
* clearing the fields.
415+
*/
416+
if (rseq_get_rseq_cs_ptr_val(rseq, &rseq_cs))
417+
return -EFAULT;
418+
if (rseq_cs && clear_rseq_cs(rseq))
419+
return -EFAULT;
420+
385421
current->rseq = rseq;
386422
current->rseq_sig = sig;
387423
/*

0 commit comments

Comments
 (0)