On Thu, Aug 01, 2024 at 01:06:51PM +0100, Mark Brown wrote:
> Add a context for the GCS state and include it in the signal context when
> running on a system that supports GCS. We reuse the same flags that the
> prctl() uses to specify which GCS features are enabled and also provide the
> current GCS pointer.
>
> We do not support enabling GCS via signal return, there is a conflict
> between specifying GCSPR_EL0 and allocation of a new GCS and this is not
> an ancticipated use case. We also enforce GCS configuration locking on
> signal return.
>
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@xxxxxxxxxx>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> ---
> arch/arm64/include/uapi/asm/sigcontext.h | 9 +++
> arch/arm64/kernel/signal.c | 106 +++++++++++++++++++++++++++++++
> 2 files changed, 115 insertions(+)
[...]
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
[...]
> @@ -999,6 +1092,13 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
> return err;
> }
>
> + if (add_all || task_gcs_el0_enabled(current)) {
> + err = sigframe_alloc(user, &user->gcs_offset,
> + sizeof(struct gcs_context));
> + if (err)
> + return err;
> + }
> +
Who turns on GCS? I have a concern that if libc is new enough to be
built for GCS then the libc startup code will to turn it on, even if
the binary stack running on top of libc is old.
Whether a given library should break old binaries is a bit of a grey
area, but I think that libraries that deliberately export stable ABIs
probably shouldn't.
With that in mind, does any GCS state need to be saved at all?
Is there any scenario where it is legitimate for the signal handler to
change the shadow stack mode or to return with an altered GCSPR_EL0?
Is the guarded stack considered necessary (or at least beneficial) for
backtracing, or is the regular stack sufficient?
(I'm assuming that unwind tables / debug info should allow the shadow
stack to be unwound anyway; rather this question is about whether
software can straightforwardly find out the interrupted GCSPR_EL0
without this information... and whether it needs to.)
> if (system_supports_sve() || system_supports_sme()) {
> unsigned int vq = 0;
>
> @@ -1099,6 +1199,12 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
> __put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
> }
>
> + if (system_supports_gcs() && err == 0 && user->gcs_offset) {
> + struct gcs_context __user *gcs_ctx =
> + apply_user_offset(user, user->gcs_offset);
> + err |= preserve_gcs_context(gcs_ctx);
> + }
> +
[...]
Cheers
---Dave
