Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs

On Tue, Jul 03, 2018 at 02:15:34PM -0400, Mathieu Desnoyers wrote:

----- On Jul 3, 2018, at 2:11 PM, Peter Zijlstra peterz@infradead.org wrote:

quoted

On Tue, Jul 03, 2018 at 01:58:37PM -0400, Mathieu Desnoyers wrote:

quoted

I can modify the ABI to put the cpu_id_start and cpu_id fields inside
a union, and update it with a single store.

Thoughts ?

Let's keep them for now, we can always frob this later, they are aligned
and proper, no need to expose that union to userspace.

Isn't it weird to change the API of an exposed public uapi header ?

Sure, just keep it as is. We don't need an exposed union to do a single
store there.

Something like the ugly below preserves API but still does a single
store.

But sure, if you want to expose that union for some reason, then now is
the time.

diff --git a/kernel/rseq.c b/kernel/rseq.c
index 22b6acf1ad63..e956c48b5f83 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c

@@ -85,10 +85,17 @@ static int rseq_update_cpu_id(struct task_struct *t)
 {
 	u32 cpu_id = raw_smp_processor_id();
 
-	if (__put_user(cpu_id, &t->rseq->cpu_id_start))
-		return -EFAULT;
-	if (__put_user(cpu_id, &t->rseq->cpu_id))
+	union {
+		struct {
+			u32 cpu_id_start;
+			u32 cpu_id;
+		};
+		u64 val;
+	} x = { { .cpu_id_start = cpu_id, .cpu_id = cpu_id, } };
+
+	if (__put_user(x.val, (u64 *)&t->rseq->cpu_id_start))
 		return -EFAULT;
+
 	trace_rseq_update(t);
 	return 0;
 }