[PATCH] perf/x86/intel/lbr: Zero the xstate buffer on allocation

[PATCH] perf/x86/intel/lbr: Zero the xstate buffer on allocation

[Thomas Gleixner]

XRSTORS requires a valid xstate buffer to work correctly. XSAVES does not
guarantee to write a fully valid buffer according to the SDM:

  "XSAVES does not write to any parts of the XSAVE header other than the
   XSTATE_BV and XCOMP_BV fields."

XRSTORS triggers a #GP:

  "If bytes 63:16 of the XSAVE header are not all zero."

It's dubious at best how this can work at all when the buffer is not zeroed
before use.

Allocate the buffers with __GFP_ZERO to prevent XRSTORS failure.

Fixes: ce711ea3cab9 ("perf/x86/intel/lbr: Support XSAVES/XRSTORS for LBR context switch")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org
---
 arch/x86/events/intel/lbr.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -731,7 +731,8 @@ void reserve_lbr_buffers(void)
 		if (!kmem_cache || cpuc->lbr_xsave)
 			continue;
 
-		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache, GFP_KERNEL,
+		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache,
+							GFP_KERNEL | __GFP_ZERO,
 							cpu_to_node(cpu));
 	}
 }

Re: [PATCH] perf/x86/intel/lbr: Zero the xstate buffer on allocation

[Liang, Kan]

On 6/11/2021 9:03 AM, Thomas Gleixner wrote:
> XRSTORS requires a valid xstate buffer to work correctly. XSAVES does not
> guarantee to write a fully valid buffer according to the SDM:
> 
>    "XSAVES does not write to any parts of the XSAVE header other than the
>     XSTATE_BV and XCOMP_BV fields."
> 
> XRSTORS triggers a #GP:
> 
>    "If bytes 63:16 of the XSAVE header are not all zero."
> 
> It's dubious at best how this can work at all when the buffer is not zeroed
> before use.
>

I didn't run into any issues when I did the test. I guess the reserved 
bits in the buffer may always happen to be 0. That's why it didn't set 
off the alarm to me. Thank you very much for the fix.

Thanks,
Kan

> Allocate the buffers with __GFP_ZERO to prevent XRSTORS failure.
> 
> Fixes: ce711ea3cab9 ("perf/x86/intel/lbr: Support XSAVES/XRSTORS for LBR context switch")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: x86@kernel.org
> ---
>   arch/x86/events/intel/lbr.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -731,7 +731,8 @@ void reserve_lbr_buffers(void)
>   		if (!kmem_cache || cpuc->lbr_xsave)
>   			continue;
>   
> -		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache, GFP_KERNEL,
> +		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache,
> +							GFP_KERNEL | __GFP_ZERO,
>   							cpu_to_node(cpu));
>   	}
>   }
> 

[tip: perf/urgent] perf/x86/intel/lbr: Zero the xstate buffer on allocation

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     7f049fbdd57f6ea71dc741d903c19c73b2f70950
Gitweb:        https://git.kernel.org/tip/7f049fbdd57f6ea71dc741d903c19c73b2f70950
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Fri, 11 Jun 2021 15:03:16 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 24 Jun 2021 08:49:03 +02:00

perf/x86/intel/lbr: Zero the xstate buffer on allocation

XRSTORS requires a valid xstate buffer to work correctly. XSAVES does not
guarantee to write a fully valid buffer according to the SDM:

  "XSAVES does not write to any parts of the XSAVE header other than the
   XSTATE_BV and XCOMP_BV fields."

XRSTORS triggers a #GP:

  "If bytes 63:16 of the XSAVE header are not all zero."

It's dubious at best how this can work at all when the buffer is not zeroed
before use.

Allocate the buffers with __GFP_ZERO to prevent XRSTORS failure.

Fixes: ce711ea3cab9 ("perf/x86/intel/lbr: Support XSAVES/XRSTORS for LBR context switch")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/87wnr0wo2z.ffs@nanos.tec.linutronix.de
---
 arch/x86/events/intel/lbr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 4409d2c..e8453de 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -731,7 +731,8 @@ void reserve_lbr_buffers(void)
 		if (!kmem_cache || cpuc->lbr_xsave)
 			continue;
 
-		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache, GFP_KERNEL,
+		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache,
+							GFP_KERNEL | __GFP_ZERO,
 							cpu_to_node(cpu));
 	}
 }