Re: [PATCH v8 23/38] arm64/signal: Set up and restore the GCS context for signal handlers

Thiago Jung Bauermann
2024-02-20T02:03:49Z

Mark Brown <broonie@kernel.org> writes:

> +#ifdef CONFIG_ARM64_GCS
> +static int gcs_restore_signal(void)
> +{
> + u64 gcspr_el0, cap;
> + int ret;
> +
> + if (!system_supports_gcs())
> + return 0;
> +
> + if (!(current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE))
> + return 0;
> +
> + gcspr_el0 = read_sysreg_s(SYS_GCSPR_EL0);
> +
> + /*
> + * GCSPR_EL0 should be pointing at a capped GCS, read the cap...
> + */
> + gcsb_dsync();
> + ret = copy_from_user(&cap, (__user void*)gcspr_el0, sizeof(cap));
> + if (ret)
> + return -EFAULT;
> +
> + /*
> + * ...then check that the cap is the actual GCS before
> + * restoring it.
> + */
> + if (!gcs_signal_cap_valid(gcspr_el0, cap))
> + return -EINVAL;
> +
> + /* Invalidate the token to prevent reuse */
> + put_user_gcs(0, (__user void*)gcspr_el0, &ret);
> + if (ret != 0)
> + return -EFAULT;

You had mentioned that "ideally we'd be doing a compare and exchange
here to substitute in a zero". Is a compare and exchange not necessary
anymore, or is it just being left for later? In the latter case, a TODO
or FIXME comment mentioning it would be useful here.

> +
> + current->thread.gcspr_el0 = gcspr_el0 + sizeof(cap);
> + write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0);
> +
> + return 0;
> +}

--
Thiago