The rseq.h UAPI documents that the rseq_cs field must be cleared before reclaiming memory that contains the targeted struct rseq_cs.
We should extend this comment to also dictate that the rseq_cs field must be cleared before reclaiming memory of the code pointed to by the rseq_cs start_ip and post_commit_offset fields.
While we can expect that use of dlclose(3) will typically unmap both struct rseq_cs and its associated code at once, nothing would theoretically prevent a JIT from reclaiming the code without reclaiming the struct rseq_cs, which would erroneously allow the kernel to consider new code which is not a rseq critical section as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer fw@deneb.enyo.de Signed-off-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com Cc: Florian Weimer fw@deneb.enyo.de Cc: Thomas Gleixner tglx@linutronix.de Cc: Peter Zijlstra (Intel) peterz@infradead.org Cc: "Paul E. McKenney" paulmck@linux.ibm.com Cc: Boqun Feng boqun.feng@gmail.com Cc: "H . Peter Anvin" hpa@zytor.com Cc: Paul Turner pjt@google.com Cc: Dmitry Vyukov dvyukov@google.com Cc: Neel Natu neelnatu@google.com Cc: linux-api@vger.kernel.org --- include/uapi/linux/rseq.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h index 9a402fdb60e9..d94afdfc4b7c 100644 --- a/include/uapi/linux/rseq.h +++ b/include/uapi/linux/rseq.h @@ -100,7 +100,9 @@ struct rseq { * instruction sequence block, as well as when the kernel detects that * it is preempting or delivering a signal outside of the range * targeted by the rseq_cs. Also needs to be set to NULL by user-space - * before reclaiming memory that contains the targeted struct rseq_cs. + * before reclaiming memory that contains the targeted struct rseq_cs + * or reclaiming memory that contains the code referred to by the + * start_ip and post_commit_offset fields of struct rseq_cs. * * Read and set by the kernel. Set by user-space with single-copy * atomicity semantics. This field should only be updated by the
The rseq.h UAPI now documents that the rseq_cs field must be cleared before reclaiming memory that contains the targeted struct rseq_cs, but also that the rseq_cs field must be cleared before reclaiming memory of the code pointed to by the rseq_cs start_ip and post_commit_offset fields.
While we can expect that use of dlclose(3) will typically unmap both struct rseq_cs and its associated code at once, nothing would theoretically prevent a JIT from reclaiming the code without reclaiming the struct rseq_cs, which would erroneously allow the kernel to consider new code which is not a rseq critical section as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer fw@deneb.enyo.de Signed-off-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com Cc: Shuah Khan skhan@linuxfoundation.org Cc: Florian Weimer fw@deneb.enyo.de Cc: Thomas Gleixner tglx@linutronix.de Cc: Peter Zijlstra (Intel) peterz@infradead.org Cc: "Paul E. McKenney" paulmck@linux.ibm.com Cc: Boqun Feng boqun.feng@gmail.com Cc: "H . Peter Anvin" hpa@zytor.com Cc: Paul Turner pjt@google.com Cc: Dmitry Vyukov dvyukov@google.com --- tools/testing/selftests/rseq/rseq.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h index d40d60e7499e..3f63eb362b92 100644 --- a/tools/testing/selftests/rseq/rseq.h +++ b/tools/testing/selftests/rseq/rseq.h @@ -149,11 +149,13 @@ static inline void rseq_clear_rseq_cs(void) /* * rseq_prepare_unload() should be invoked by each thread executing a rseq * critical section at least once between their last critical section and - * library unload of the library defining the rseq critical section - * (struct rseq_cs). This also applies to use of rseq in code generated by - * JIT: rseq_prepare_unload() should be invoked at least once by each - * thread executing a rseq critical section before reclaim of the memory - * holding the struct rseq_cs. + * library unload of the library defining the rseq critical section (struct + * rseq_cs) or the code referred to by the struct rseq_cs start_ip and + * post_commit_offset fields. This also applies to use of rseq in code + * generated by JIT: rseq_prepare_unload() should be invoked at least once by + * each thread executing a rseq critical section before reclaim of the memory + * holding the struct rseq_cs or reclaim of the code pointed to by struct + * rseq_cs start_ip and post_commit_offset fields. */ static inline void rseq_prepare_unload(void) {
----- On Dec 20, 2019, at 3:33 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
The rseq.h UAPI documents that the rseq_cs field must be cleared before reclaiming memory that contains the targeted struct rseq_cs.
We should extend this comment to also dictate that the rseq_cs field must be cleared before reclaiming memory of the code pointed to by the rseq_cs start_ip and post_commit_offset fields.
While we can expect that use of dlclose(3) will typically unmap both struct rseq_cs and its associated code at once, nothing would theoretically prevent a JIT from reclaiming the code without reclaiming the struct rseq_cs, which would erroneously allow the kernel to consider new code which is not a rseq critical section as a rseq critical section following a code reclaim.
Hi Peter,
Is there anything preventing this rseq UAPI documentation fix from being merged ?
Thanks,
Mathieu
Suggested-by: Florian Weimer fw@deneb.enyo.de Signed-off-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com Cc: Florian Weimer fw@deneb.enyo.de Cc: Thomas Gleixner tglx@linutronix.de Cc: Peter Zijlstra (Intel) peterz@infradead.org Cc: "Paul E. McKenney" paulmck@linux.ibm.com Cc: Boqun Feng boqun.feng@gmail.com Cc: "H . Peter Anvin" hpa@zytor.com Cc: Paul Turner pjt@google.com Cc: Dmitry Vyukov dvyukov@google.com Cc: Neel Natu neelnatu@google.com Cc: linux-api@vger.kernel.org
include/uapi/linux/rseq.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h index 9a402fdb60e9..d94afdfc4b7c 100644 --- a/include/uapi/linux/rseq.h +++ b/include/uapi/linux/rseq.h @@ -100,7 +100,9 @@ struct rseq { * instruction sequence block, as well as when the kernel detects that * it is preempting or delivering a signal outside of the range * targeted by the rseq_cs. Also needs to be set to NULL by user-space
* before reclaiming memory that contains the targeted struct rseq_cs.
* before reclaiming memory that contains the targeted struct rseq_cs
* or reclaiming memory that contains the code referred to by the
* start_ip and post_commit_offset fields of struct rseq_cs.
- Read and set by the kernel. Set by user-space with single-copy
- atomicity semantics. This field should only be updated by the
-- 2.17.1
linux-stable-mirror@lists.linaro.org