Re: [PATCH 1/7] arm64/gcs: task_gcs_el0_enable() should use passed task

[Mark Rutland <mark.rutland@arm.com>]

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 <jeremy.linton@arm.com>
> ---
>  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
>