linux-kernel - Re: [PATCH v1 1/1] rseq: Validate read-only fields under DEBUG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241112144739.GJ6497@noisy.programming.kicks-ass.net>
Date: Tue, 12 Nov 2024 15:47:39 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: linux-kernel@...r.kernel.org, "Paul E . McKenney" <paulmck@...nel.org>,
	Boqun Feng <boqun.feng@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...nel.org>, Ingo Molnar <mingo@...hat.com>,
	Peter Oskolkov <posk@...gle.com>,
	Dmitry Vyukov <dvyukov@...gle.com>, Marco Elver <elver@...gle.com>,
	Florian Weimer <fweimer@...hat.com>,
	Carlos O'Donell <carlos@...hat.com>, DJ Delorie <dj@...hat.com>,
	libc-alpha@...rceware.org
Subject: Re: [PATCH v1 1/1] rseq: Validate read-only fields under DEBUG_RSEQ
 config

On Tue, Nov 12, 2024 at 09:27:23AM -0500, Mathieu Desnoyers wrote:

> Your approach looks indeed better than mine, I'll steal it with your
> permission. :)

Ofc.


> > > +#ifdef CONFIG_DEBUG_RSEQ
> > > +	/*
> > > +	 * Initialize the in-kernel rseq fields copy for validation of
> > > +	 * read-only fields.
> > > +	 */
> > > +	if (get_user(rseq_kernel_fields(current)->cpu_id_start, &rseq->cpu_id_start) ||
> > > +	    get_user(rseq_kernel_fields(current)->cpu_id, &rseq->cpu_id) ||
> > > +	    get_user(rseq_kernel_fields(current)->node_id, &rseq->node_id) ||
> > > +	    get_user(rseq_kernel_fields(current)->mm_cid, &rseq->mm_cid))
> > > +		return -EFAULT;
> > > +#endif
> > 
> > So I didn't change the above, but wouldn't it make more sense to do
> > rseq_reset_rseq_cpu_node_id() here, but without the validation?
> 
> Indeed we could do this (for both DEBUG_RSEQ={y,n}), but it would add extra
> useless stores to those userspace fields on rseq registration, which is
> performed on every thread creation starting from glibc 2.35. The
> rseq_set_notify_resume() invoked at the end of registration ensures that
> those fields get populated before return to userspace.
> 
> So I am not against a more robust approach, but I'm reluctant to add redundant
> work on thread creation.

Ah, indeed. Oh well..