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

[Peter Zijlstra]

On Tue, Jul 03, 2018 at 02:15:34PM -0400, Mathieu Desnoyers wrote:
> ----- On Jul 3, 2018, at 2:11 PM, Peter Zijlstra peterz@xxxxxxxxxxxxx wrote:
> 
> > On Tue, Jul 03, 2018 at 01:58:37PM -0400, Mathieu Desnoyers wrote:
> >> 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;
 }