On Tue, Mar 18, 2025 at 03:48:35PM -0500, Jeremy Linton wrote:
> Mark Rutland noticed that the task parameter is ignored and
> 'current' is being used instead. Since this is usually
> what its passed, it hasn't yet been causing problems but likely
> will as the code gets more testing.
Are we sure nothing is relying upon the bug?
For example, in copy_thread_gcs():
copy_thread_gcs(p, ...) {
...
gcs = gcs_alloc_thread_stack(p, ...) {
...
if (!task_gcs_el0_enabled(p))
return 0;
...
< actually allocate here >
}
...
p->thread.gcs_el0_mode = current->thread.gcs_el0_mode;
...
}
Either that later assignment is redundant, or copy_thread_gcs() was
accidentally relying upon task_gcs_el0_enabled() reading from 'current'
rather than 'p', and this change opens up another bug...
Mark.
>
> Fixes: fc84bc5378a8 ("arm64/gcs: Context switch GCS state for EL0")
> Signed-off-by: Jeremy Linton <[email protected]>
> ---
> arch/arm64/include/asm/gcs.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/gcs.h b/arch/arm64/include/asm/gcs.h
> index f50660603ecf..5bc432234d3a 100644
> --- a/arch/arm64/include/asm/gcs.h
> +++ b/arch/arm64/include/asm/gcs.h
> @@ -58,7 +58,7 @@ static inline u64 gcsss2(void)
>
> static inline bool task_gcs_el0_enabled(struct task_struct *task)
> {
> - return current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE;
> + return task->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE;
> }
>
> void gcs_set_el0_mode(struct task_struct *task);
> --
> 2.48.1
>
