Re: [PATCH] x86/mce: Fix buggy error path in cpu_bank_alloc() leading to UAF

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.03.2026 11:49, Andrew Cooper wrote:
> When cleaning up from a mcabanks_alloc() failure, the memory is freed but
> stale pointers are left in the percpu variables.
> 
> Use cpu_bank_free() which is idempotent and behaves correctly.
> 
> Fixes: 2e6c8f182c9c ("x86: distinguish CPU offlining from CPU removal")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> 2e6c8f182c9c updated the success path but missed the associated error path.

I don't think so, see below.

> Prior to that, the paths were at least consistent since their introduction in
> commit 78c579426fb5 ("x86/MCE: Implement clearbank callback for AMD").
> ---
>  xen/arch/x86/cpu/mcheck/mce.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 9a91807cfb33..684871b216a4 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -694,8 +694,7 @@ static int cpu_bank_alloc(unsigned int cpu)
>  
>      if ( !poll || !clr )
>      {
> -        mcabanks_free(poll);
> -        mcabanks_free(clr);
> +        cpu_bank_free(cpu);

But this way we'll leak the allocation that may have succeeded, as the per-CPU
data is updated only afterwards. Yet it's those per-CPU items which
cpu_bank_free() reads.

I don't think there's any bug to fix here: Neither of the two pointers is
allocated anywhere else, so the per-CPU slots are always both NULL or both
non-NULL.

Jan