----- On Jan 25, 2022, at 7:21 AM, Christian Brauner brauner@kernel.org wrote:
On Mon, Jan 24, 2022 at 12:12:40PM -0500, Mathieu Desnoyers wrote:
The rseq rseq_cs.ptr.{ptr32,padding} uapi endianness handling is entirely wrong on 32-bit little endian: a preprocessor logic mistake wrongly uses the big endian field layout on 32-bit little endian architectures.
Fortunately, those ptr32 accessors were never used within the kernel, and only meant as a convenience for user-space.
Remove those and only leave the "ptr64" union field, as this is the only thing really needed to express the ABI. Document how 32-bit architectures are meant to interact with this "ptr64" union field.
Fixes: ec9c82e03a74 ("rseq: uapi: Declare rseq_cs field as union, update includes") Signed-off-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com Cc: Florian Weimer fw@deneb.enyo.de Cc: Thomas Gleixner tglx@linutronix.de Cc: linux-api@vger.kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Boqun Feng boqun.feng@gmail.com Cc: Andy Lutomirski luto@amacapital.net Cc: Dave Watson davejwatson@fb.com Cc: Paul Turner pjt@google.com Cc: Andrew Morton akpm@linux-foundation.org Cc: Russell King linux@arm.linux.org.uk Cc: "H . Peter Anvin" hpa@zytor.com Cc: Andi Kleen andi@firstfloor.org Cc: Christian Brauner christian.brauner@ubuntu.com Cc: Ben Maurer bmaurer@fb.com Cc: Steven Rostedt rostedt@goodmis.org Cc: Josh Triplett josh@joshtriplett.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will.deacon@arm.com Cc: Michael Kerrisk mtk.manpages@gmail.com Cc: Joel Fernandes joelaf@google.com Cc: Paul E. McKenney paulmck@kernel.org
include/uapi/linux/rseq.h | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h index 9a402fdb60e9..31290f2424a7 100644 --- a/include/uapi/linux/rseq.h +++ b/include/uapi/linux/rseq.h @@ -105,22 +105,13 @@ struct rseq { * Read and set by the kernel. Set by user-space with single-copy * atomicity semantics. This field should only be updated by the * thread which registered this data structure. Aligned on 64-bit.
** 32-bit architectures should update the low order bits of the* rseq_cs.ptr64 field, leaving the high order bits initialized* to 0.A bit unfortunate we seem to have to keep the union around even though it's just one field now.
Well, as far as the user-space projects that I know of which use rseq are concerned (glibc, librseq, tcmalloc), those end up with their own copy of the uapi header anyway to deal with the big/little endian field on 32-bit. So I'm very much open to remove the union if we accept that this uapi header is really just meant to express the ABI and is not expected to be used as an API by user-space.
That would mean we also bring a uapi header copy into the kernel rseq selftests as well to minimize the gap between librseq and the kernel sefltests (the kernel sefltests pretty much include a copy of librseq for convenience. librseq is maintained out of tree).
Thoughts ?
Thanks,
Mathieu