Re: [PATCH 2/3] um: avoid copying FP state from init_task

[Thomas Weißschuh]

On Tue, Dec 17, 2024 at 09:27:44PM +0100, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@xxxxxxxxx>
> 
> The init_task instance of struct task_struct is statically allocated and
> does not contain the dynamic area for the userspace FP registers. As
> such, limit the copy to the valid area of init_task and fill the rest
> with zero.
> 
> Note that the FP state is only needed for userspace, and as such it is
> entirely reasonable for init_task to not contain it.
> 
> Reported-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> Closes: https://lore.kernel.org/Z1ySXmjZm-xOqk90@xxxxxxxxxx
> Fixes: 3f17fed21491 ("um: switch to regset API and depend on XSTATE")

No stable backport? The broken commit is now in the 6.13 release.

> Signed-off-by: Benjamin Berg <benjamin.berg@xxxxxxxxx>

Tested-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>

> ---
>  arch/um/kernel/process.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
> index 30bdc0a87dc8..3a67ba8aa62d 100644
> --- a/arch/um/kernel/process.c
> +++ b/arch/um/kernel/process.c
> @@ -191,7 +191,15 @@ void initial_thread_cb(void (*proc)(void *), void *arg)
>  int arch_dup_task_struct(struct task_struct *dst,
>  			 struct task_struct *src)
>  {
> -	memcpy(dst, src, arch_task_struct_size);
> +	/* init_task is not dynamically sized (missing FPU state) */
> +	if (unlikely(src == &init_task)) {
> +		memcpy(dst, src, sizeof(init_task));
> +		memset((void *)dst + sizeof(init_task), 0,
> +		       arch_task_struct_size - sizeof(init_task));
> +	} else {
> +		memcpy(dst, src, arch_task_struct_size);
> +	}

Nitpick:
This could make use of memcpy_and_pad() in various forms.

> +
>  	return 0;
>  }
>  
> -- 
> 2.47.1
>