----- On Aug 1, 2022, at 10:25 AM, Florian Weimer fweimer@xxxxxxxxxx wrote: > * Ingo Molnar: > >> * Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote: >> >>> rseq_abi()->flags and rseq_abi()->rseq_cs->flags 29 upper bits are >>> currently unused. >>> >>> The current behavior when those bits are set is to ignore them. This is >>> not an ideal behavior, because when future features will start using >>> those flags, if user-space fails to correctly validate that the kernel >>> indeed supports those flags (e.g. with a new sys_rseq flags bit) before >>> using them, it may incorrectly assume that the kernel will handle those >>> flags way when in fact those will be silently ignored on older kernels. >>> >>> Validating that unused flags bits are cleared will allow a smoother >>> transition when those flags will start to be used by allowing >>> applications to fail early, and obviously, when they attempt to use the >>> new flags on an older kernel that does not support them. >>> >>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> >>> --- >>> kernel/rseq.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/kernel/rseq.c b/kernel/rseq.c >>> index 81d7dc80787b..bda8175f8f99 100644 >>> --- a/kernel/rseq.c >>> +++ b/kernel/rseq.c >>> @@ -176,7 +176,7 @@ static int rseq_need_restart(struct task_struct *t, u32 >>> cs_flags) >>> u32 flags, event_mask; >>> int ret; >>> >>> - if (WARN_ON_ONCE(cs_flags & RSEQ_CS_NO_RESTART_FLAGS)) >>> + if (WARN_ON_ONCE(cs_flags & RSEQ_CS_NO_RESTART_FLAGS) || cs_flags) >>> return -EINVAL; >>> >>> /* Get thread flags. */ >>> @@ -184,7 +184,7 @@ static int rseq_need_restart(struct task_struct *t, u32 >>> cs_flags) >>> if (ret) >>> return ret; >>> >>> - if (WARN_ON_ONCE(flags & RSEQ_CS_NO_RESTART_FLAGS)) >>> + if (WARN_ON_ONCE(flags & RSEQ_CS_NO_RESTART_FLAGS) || flags) >>> return -EINVAL; >> >> Just to make it clear: no existing libraries/tooling out there have learned >> to rely on the old ABI that ignored unset flags, right? Only then is this >> patch ABI-safe. > > I believe glibc initializes the flag fields to zero before calling the > rseq system call. (I don't know if the rseq system call does its own > initialization; maybe it should if it doesn't do so already.) Initialization and following updates of rseq_abi()->flags and rseq_abi()->rseq_cs->flags is done by user-space, so the rseq system call does not initialize any of those fields. Indeed glibc initialize the rseq_abi()->flags to 0, and does not use rseq_abi()->rseq_cs->flags as of now. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
