Re: [PATCH bpf-next] bpf: Overwrite scratch PTE when allocating arena pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Overwriting a present PTE (the scratch page) with a new page via set_pte_at() without first clearing it and flushing the TLB violates Break-Before-Make (BBM) architecture rules and causes silent data corruption via stale TLB translations.
--

commit f66bc91b18a1403f8777f0486173da0584f46ca3
Author: Tejun Heo <tj@kernel.org>

bpf: Overwrite scratch PTE when allocating arena pages

apply_range_set_cb() previously returned -EBUSY when encountering a
populated PTE. This patch removes that check to allow unconditionally
overwriting the scratch page left behind by kernel-fault recovery during
new arena allocations.

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index a641b19255aad..9e0b9f2170204 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -157,15 +157,13 @@ static int apply_range_set_cb(pte_t *pte, unsigned long addr, void *data)
>  
>  	if (!data)
>  		return 0;
> -	/* sanity check */
> -	if (unlikely(!pte_none(ptep_get(pte))))
> -		return -EBUSY;
>  
>  	page = d->pages[d->i];
>  	/* paranoia, similar to vmap_pages_pte_range() */
>  	if (WARN_ON_ONCE(!pfn_valid(page_to_pfn(page))))
>  		return -EINVAL;
>  
> +	/* May be none or the scratch page, overwrite either way */
>  	set_pte_at(&init_mm, addr, pte, mk_pte(page, PAGE_KERNEL));

[Severity: Critical]
Does unconditionally overwriting a present PTE with set_pte_at() without
first clearing it violate Break-Before-Make architecture rules?

On architectures like arm64, overwriting a valid, present PTE without a
sequence like ptep_get_and_clear() can trigger hardware TLB conflict aborts
or panics.

Additionally, since this changes the PFN of a present PTE (the scratch page),
doesn't this require a TLB flush?

Looking at the caller arena_alloc_pages(), it appears to only invoke
flush_vmap_cache() after the apply_to_page_range() loop:

	ret = apply_to_page_range(&init_mm,
				  kern_vm_start + uaddr32 + (mapped << PAGE_SHIFT),
				  this_batch << PAGE_SHIFT, apply_range_set_cb, &data);
	...
	flush_vmap_cache(kern_vm_start + uaddr32, mapped << PAGE_SHIFT);

Since flush_vmap_cache() flushes data caches and not the TLB, could CPUs
retain stale TLB mappings to the scratch page? If so, subsequent accesses
might hit the shared scratch page instead of the newly allocated page,
bypassing memory isolation.

>  	d->i++;
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531165852.555930-1-tj@kernel.org?part=1