Re: [PATCH 07/10] x86/fpu: Pre-fault only required size of xstate buffer

[Alexander Mikhalitsyn]

Am Mo., 15. Juni 2026 um 21:40 Uhr schrieb Andrei Vagin <avagin@xxxxxxxxxx>:
>
> The kernel previously used the default task FPU state size (user_size)
> to fault in the user buffer when restoring FPU registers from a signal
> frame. This can lead to attempting to fault in memory past the end of
> the actual frame if the frame was smaller than the default size.
>
> Introduce consistency checks to calculate the actual required size for the
> features enabled in the xfeatures mask, ensure that the provided
> xstate_size is sufficient, and shrink it to the actual required size. Use
> this validated size to fault in the user buffer.
>
> Keep the strict check that the provided xstate_size does not exceed the
> default user_size for now.
>
> Signed-off-by: Andrei Vagin <avagin@xxxxxxxxxx>

Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxxx>

> ---
>  arch/x86/kernel/fpu/signal.c | 40 ++++++++++++++++++++++++++++--------
>  arch/x86/kernel/fpu/xstate.c |  2 +-
>  arch/x86/kernel/fpu/xstate.h |  2 ++
>  3 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index 85021c5ea649..1e7cc114c186 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -29,7 +29,8 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *buf_fx,
>  {
>         int min_xstate_size = sizeof(struct fxregs_state) +
>                               sizeof(struct xstate_header);
> -       void __user *fpstate = buf_fx;
> +       struct fpstate *fpstate = x86_task_fpu(current)->fpstate;
> +       void __user *buf = buf_fx;
>         unsigned int magic2;
>
>         if (__copy_from_user(fx_sw, &buf_fx->sw_reserved[0], sizeof(*fx_sw)))
> @@ -38,8 +39,9 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *buf_fx,
>         /* Check for the first magic field and other error scenarios. */
>         if (fx_sw->magic1 != FP_XSTATE_MAGIC1 ||
>             fx_sw->xstate_size < min_xstate_size ||
> -           fx_sw->xstate_size > x86_task_fpu(current)->fpstate->user_size ||
> -           fx_sw->xstate_size > fx_sw->extended_size)
> +           fx_sw->xstate_size > fpstate->user_size ||
> +           fx_sw->xstate_size > fx_sw->extended_size ||
> +           fx_sw->extended_size - fx_sw->xstate_size < FP_XSTATE_MAGIC2_SIZE)
>                 goto err_setfx;
>
>         /*
> @@ -48,11 +50,27 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *buf_fx,
>          * fpstate layout with out copying the extended state information
>          * in the memory layout.
>          */
> -       if (__get_user(magic2, (__u32 __user *)(fpstate + fx_sw->xstate_size)))
> +       if (__get_user(magic2, (__u32 __user *)(buf + fx_sw->xstate_size)))
>                 return false;
> +       if (unlikely(magic2 != FP_XSTATE_MAGIC2))
> +               goto err_setfx;
>
> -       if (likely(magic2 == FP_XSTATE_MAGIC2))
> -               return true;
> +       if (fx_sw->xstate_size != fpstate->user_size ||
> +           fx_sw->xfeatures != fpstate->user_xfeatures) {
> +               unsigned int xsize;
> +               u64 xfeatures;
> +
> +               /* Calculate size of enabled features only. */
> +               xfeatures = fx_sw->xfeatures & fpstate->user_xfeatures;
> +
> +               xsize = xstate_calculate_size(xfeatures, false);
> +               if (fx_sw->xstate_size < xsize)
> +                       return false;
> +
> +               fx_sw->xstate_size = xsize;
> +       }
> +
> +       return true;
>  err_setfx:
>         /*
>          * The fallback to FX-only state is used to preserve backward
> @@ -279,7 +297,8 @@ static bool restore_fpregs_from_user_compat(void __user *buf_f, void __user *buf
>   * Attempt to restore the FPU registers directly from user memory.
>   * Pagefaults are handled and any errors returned are fatal.
>   */
> -static bool restore_fpregs_from_user(void __user *buf, u64 xrestore_mask, bool fx_only)
> +static bool restore_fpregs_from_user(void __user *buf, u64 xrestore_mask,
> +                                    bool fx_only, size_t xstate_size)
>  {
>         struct fpu *fpu = x86_task_fpu(current);
>         int ret;
> @@ -313,7 +332,7 @@ static bool restore_fpregs_from_user(void __user *buf, u64 xrestore_mask, bool f
>                 if (ret != X86_TRAP_PF)
>                         return false;
>
> -               if (!fault_in_readable(buf, fpu->fpstate->user_size))
> +               if (!fault_in_readable(buf, xstate_size))
>                         goto retry;
>                 return false;
>         }
> @@ -339,6 +358,7 @@ static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx)
>  {
>         bool fx_only = false;
>         u64 xrestore_mask = 0;
> +       size_t xstate_size;
>
>         if (use_xsave()) {
>                 struct _fpx_sw_bytes fx_sw_user;
> @@ -348,13 +368,15 @@ static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx)
>
>                 fx_only = !fx_sw_user.magic1;
>                 xrestore_mask = fx_sw_user.xfeatures;
> +               xstate_size = fx_sw_user.xstate_size;
>         } else {
>                 xrestore_mask = XFEATURE_MASK_FPSSE;
> +               xstate_size = sizeof(struct fxregs_state);
>         }
>
>         if (likely(!buf_f)) {
>                 /* Restore the FPU registers directly from user memory. */
> -               return restore_fpregs_from_user(buf_fx, xrestore_mask, fx_only);
> +               return restore_fpregs_from_user(buf_fx, xrestore_mask, fx_only, xstate_size);
>         }
>
>         return restore_fpregs_from_user_compat(buf_f, buf_fx, xrestore_mask, fx_only);
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index ed39d7051d0d..b7d0d78d2081 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -587,7 +587,7 @@ static bool __init check_xstate_against_struct(int nr)
>         return true;
>  }
>
> -static unsigned int xstate_calculate_size(u64 xfeatures, bool compacted)
> +unsigned int xstate_calculate_size(u64 xfeatures, bool compacted)
>  {
>         unsigned int topmost, offset, i;
>
> diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
> index 38a2862f09d3..c73cf2444de6 100644
> --- a/arch/x86/kernel/fpu/xstate.h
> +++ b/arch/x86/kernel/fpu/xstate.h
> @@ -55,6 +55,8 @@ extern int copy_sigframe_from_user_to_xstate(struct task_struct *tsk, const void
>  extern void fpu__init_cpu_xstate(void);
>  extern void fpu__init_system_xstate(unsigned int legacy_size);
>
> +extern unsigned int xstate_calculate_size(u64 xfeatures, bool compacted);
> +
>  extern void __user *get_xsave_addr_user(struct xregs_state __user *xsave, int xfeature_nr);
>
>  static inline u64 xfeatures_mask_supervisor(void)
> --
> 2.54.0.1189.g8c84645362-goog
>
>