All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jiayuan Chen" <jiayuan.chen@linux.dev>
Cc: bpf@vger.kernel.org, linux-rt-devel@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 1/4] bpf: Add a sleepable page allocator for map memory
Date: Wed, 05 Aug 2026 09:32:50 +0000	[thread overview]
Message-ID: <20260805093251.636CC1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260805091720.139924-2-jiayuan.chen@linux.dev>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Using __GFP_ZERO to zero pages intended for user-space mapping skips required dcache flushes on cache-aliasing architectures.
- [Medium] bpf_map_alloc_page_sleepable() and __bpf_alloc_page() silently bypass task/VMA NUMA mempolicy when the requested node is NUMA_NO_NODE.

Pre-existing issues:
- [High] The fallback allocation path in __bpf_alloc_page() omits __GFP_ZERO, returning uninitialized kernel memory that can be exposed to user space.
--

--- Patch [1]: [PATCH bpf-next v2 1/4] bpf: Add a sleepable page allocator for map memory ---
commit 2a4210c942ca919834ce20218ab8bedc6cbb40a8
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

bpf: Add a sleepable page allocator for map memory

This patch adds bpf_map_alloc_page_sleepable() to provide a blocking page
allocator for sleepable contexts. This avoids using the non-blocking
allocator, which never reclaims or engages the OOM machinery, for contexts
that can actually sleep.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8d111da886553..67d8157c6623e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -602,15 +602,14 @@ static bool can_alloc_pages(void)
>  		!IS_ENABLED(CONFIG_PREEMPT_RT);
>  }
>  
> +#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NOWARN)
> +
>  static struct page *__bpf_alloc_page(int nid)
>  {
>  	if (!can_alloc_pages())
>  		return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0);

[Severity: High]
This is a pre-existing issue, but does this fallback path leak uninitialized
kernel memory? 

The call to alloc_pages_nolock() passes only __GFP_ACCOUNT and completely
omits __GFP_ZERO. If a BPF map is allocated on PREEMPT_RT where
can_alloc_pages() returns false, this could expose uninitialized pages to
user space.

>  
> -	return alloc_pages_node(nid,
> -				GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT
> -				| __GFP_NOWARN,
> -				0);
> +	return alloc_pages_node(nid, BPF_PAGE_GFP, 0);

[Severity: Medium]
When nid is NUMA_NO_NODE, does alloc_pages_node() bypass the task's NUMA
mempolicy?

Since alloc_pages_node() automatically resolves NUMA_NO_NODE to the local
CPU node rather than defaulting to the task's mempolicy, this could cause
pages to land on the wrong nodes for user applications.

>  }
>  
[ ... ]
> @@ -636,6 +635,20 @@ int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
>  	return ret;
>  }
>  
> +/*
> + * For callers that know they run in a sleepable context, e.g. a user page
> + * fault handler. can_alloc_pages() is a conservative guess made for BPF
> + * program context - notably it is always false on PREEMPT_RT - so going
> + * through bpf_map_alloc_pages() there would needlessly pick the
> + * non-blocking allocator, which never reclaims and never engages the OOM
> + * machinery.
> + */
> +struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map)
> +{
> +	might_sleep();
> +	return alloc_pages_node(map->numa_node, BPF_PAGE_GFP, 0);

[Severity: High]
Does using __GFP_ZERO (via BPF_PAGE_GFP) skip required dcache flushes on
cache-aliasing architectures? 

When allocating pages destined for user-space mapping (like the arena page
fault handler), __GFP_ZERO uses clear_page() which leaves zeros in the
kernel's dcache. Could user processes reading from these pages see stale,
uninitialized physical memory instead of zeros?

[Severity: Medium]
Similar to the issue in __bpf_alloc_page(), does using alloc_pages_node()
here bypass the task mempolicy when map->numa_node is NUMA_NO_NODE?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805091720.139924-1-jiayuan.chen@linux.dev?part=1

  reply	other threads:[~2026-08-05  9:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05  9:15 [PATCH bpf-next v2 0/4] bpf: arena: handle memory.max on fault-in with reclaim/OOM Jiayuan Chen
2026-08-05  9:15 ` [PATCH bpf-next v2 1/4] bpf: Add a sleepable page allocator for map memory Jiayuan Chen
2026-08-05  9:32   ` sashiko-bot [this message]
2026-08-05 10:36     ` Jiayuan Chen
2026-08-05  9:15 ` [PATCH bpf-next v2 2/4] bpf: arena: allocate the fault-in page outside the lock Jiayuan Chen
2026-08-05  9:15 ` [PATCH bpf-next v2 3/4] selftests/bpf: Add read_cgroup_file() to cgroup_helpers Jiayuan Chen
2026-08-05  9:15 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add a test for arena fault-in under memory.max Jiayuan Chen
2026-08-05  9:29   ` sashiko-bot
2026-08-05 11:18     ` Jiayuan Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805093251.636CC1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=jiayuan.chen@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.