All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static
@ 2026-06-17 20:21 JP Kobryn
  2026-06-17 20:57 ` sashiko-bot
  2026-06-17 21:06 ` Alexei Starovoitov
  0 siblings, 2 replies; 4+ messages in thread
From: JP Kobryn @ 2026-06-17 20:21 UTC (permalink / raw)
  To: roman.gushchin, shakeel.butt, akpm, ast, bpf, linux-mm; +Cc: linux-kernel

The kfuncs in bpf_memcontrol.c are not called by in-kernel C code. They are
only referenced by BPF programs and resolved through BTF.

Since no external linkage is needed, mark these functions static.

Fixes: 5904db9891f8 ("mm: introduce BPF kfuncs to deal with memcg pointers")
Fixes: 5c7db3239c9f ("mm: introduce bpf_get_root_mem_cgroup() BPF kfunc")
Fixes: 99430ab8b804 ("mm: introduce BPF kfuncs to access memcg statistics and events")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606150332.Xy4Egd9s-lkp@intel.com/
Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
---
 mm/bpf_memcontrol.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
index 716df49d7647..eff79bb2c758 100644
--- a/mm/bpf_memcontrol.c
+++ b/mm/bpf_memcontrol.c
@@ -20,7 +20,7 @@ __bpf_kfunc_start_defs();
  *
  * Return: A pointer to the root memory cgroup.
  */
-__bpf_kfunc struct mem_cgroup *bpf_get_root_mem_cgroup(void)
+__bpf_kfunc static struct mem_cgroup *bpf_get_root_mem_cgroup(void)
 {
 	if (mem_cgroup_disabled())
 		return NULL;
@@ -41,7 +41,7 @@ __bpf_kfunc struct mem_cgroup *bpf_get_root_mem_cgroup(void)
  * Return: A pointer to a mem_cgroup structure after bumping
  * the corresponding css's reference counter.
  */
-__bpf_kfunc struct mem_cgroup *
+__bpf_kfunc static struct mem_cgroup *
 bpf_get_mem_cgroup(struct cgroup_subsys_state *css)
 {
 	struct mem_cgroup *memcg = NULL;
@@ -75,7 +75,7 @@ bpf_get_mem_cgroup(struct cgroup_subsys_state *css)
  * Releases a previously acquired memcg reference.
  * Implements KF_RELEASE semantics.
  */
-__bpf_kfunc void bpf_put_mem_cgroup(struct mem_cgroup *memcg)
+__bpf_kfunc static void bpf_put_mem_cgroup(struct mem_cgroup *memcg)
 {
 	css_put(&memcg->css);
 }
@@ -89,8 +89,8 @@ __bpf_kfunc void bpf_put_mem_cgroup(struct mem_cgroup *memcg)
  *
  * Return: The current value of the corresponding events counter.
  */
-__bpf_kfunc unsigned long bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg,
-						   enum vm_event_item event)
+__bpf_kfunc static unsigned long
+bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg, enum vm_event_item event)
 {
 	if (unlikely(!memcg_vm_event_item_valid(event)))
 		return (unsigned long)-1;
@@ -110,7 +110,7 @@ __bpf_kfunc unsigned long bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg,
  *
  * Return: The current memory cgroup size in bytes.
  */
-__bpf_kfunc unsigned long bpf_mem_cgroup_usage(struct mem_cgroup *memcg)
+__bpf_kfunc static unsigned long bpf_mem_cgroup_usage(struct mem_cgroup *memcg)
 {
 	return page_counter_read(&memcg->memory) * PAGE_SIZE;
 }
@@ -122,8 +122,8 @@ __bpf_kfunc unsigned long bpf_mem_cgroup_usage(struct mem_cgroup *memcg)
  *
  * Return: The current value of the memory event counter.
  */
-__bpf_kfunc unsigned long bpf_mem_cgroup_memory_events(struct mem_cgroup *memcg,
-						       enum memcg_memory_event event)
+__bpf_kfunc static unsigned long
+bpf_mem_cgroup_memory_events(struct mem_cgroup *memcg, enum memcg_memory_event event)
 {
 	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))
 		return (unsigned long)-1;
@@ -140,7 +140,8 @@ __bpf_kfunc unsigned long bpf_mem_cgroup_memory_events(struct mem_cgroup *memcg,
  *
  * Return: The value of the page state counter in bytes.
  */
-__bpf_kfunc unsigned long bpf_mem_cgroup_page_state(struct mem_cgroup *memcg, int idx)
+__bpf_kfunc static unsigned long
+bpf_mem_cgroup_page_state(struct mem_cgroup *memcg, int idx)
 {
 	if (unlikely(!memcg_stat_item_valid(idx)))
 		return (unsigned long)-1;
@@ -154,7 +155,7 @@ __bpf_kfunc unsigned long bpf_mem_cgroup_page_state(struct mem_cgroup *memcg, in
  *
  * Propagate memory cgroup's statistics up the cgroup tree.
  */
-__bpf_kfunc void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
+__bpf_kfunc static void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
 {
 	mem_cgroup_flush_stats(memcg);
 }
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static
  2026-06-17 20:21 [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static JP Kobryn
@ 2026-06-17 20:57 ` sashiko-bot
  2026-06-17 21:06 ` Alexei Starovoitov
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-17 20:57 UTC (permalink / raw)
  To: JP Kobryn; +Cc: bpf

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

Pre-existing issues:
- [High] The kfunc `bpf_mem_cgroup_memory_events` lacks a negative bounds check for its enum parameter, allowing an out-of-bounds array read.
- [High] The kfunc `bpf_mem_cgroup_vm_events` uses a signed enum parameter without a negative bounds check, leading to an out-of-bounds array read.
--

commit 69ebb23c6ec72d0bfb8ea52e71248a3737fbb2f4
Author: JP Kobryn <jp.kobryn@linux.dev>

mm/bpf_memcontrol: mark BPF memcg kfuncs static

The patch marks kfuncs in bpf_memcontrol.c as static since they are only
referenced by BPF programs and resolved through BTF. This removes unnecessary
external linkage for these functions.

Closes: https://lore.kernel.org/oe-kbuild-all/202606150332.Xy4Egd9s-lkp@intel.com/

> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d76477..eff79bb2c7581 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c

[ ... ]

> @@ -89,8 +89,8 @@ __bpf_kfunc void bpf_put_mem_cgroup(struct mem_cgroup *memcg)
>   *
>   * Return: The current value of the corresponding events counter.
>   */
> -__bpf_kfunc unsigned long bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg,
> -						   enum vm_event_item event)
> +__bpf_kfunc static unsigned long
> +bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg, enum vm_event_item event)
>  {
>  	if (unlikely(!memcg_vm_event_item_valid(event)))

[Severity: High]
This is a pre-existing issue, but does memcg_vm_event_item_valid()
adequately check for negative values?

Since the BPF verifier does not validate enum ranges, the event argument
could be negative. Looking at memcg_vm_event_item_valid() in
mm/memcontrol.c, it checks:

    if (idx >= NR_VM_EVENT_ITEMS)
        return false;

Because both operands are signed, negative values bypass the check.

Can this lead to an out-of-bounds read when the negative index is
eventually passed to memcg_events(memcg, event)?

>  		return (unsigned long)-1;

[ ... ]

> @@ -122,8 +122,8 @@ __bpf_kfunc unsigned long bpf_mem_cgroup_usage(struct mem_cgroup *memcg)
>   *
>   * Return: The current value of the memory event counter.
>   */
> -__bpf_kfunc unsigned long bpf_mem_cgroup_memory_events(struct mem_cgroup *memcg,
> -						       enum memcg_memory_event event)
> +__bpf_kfunc static unsigned long
> +bpf_mem_cgroup_memory_events(struct mem_cgroup *memcg, enum memcg_memory_event event)
>  {
>  	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))

[Severity: High]
This isn't a bug introduced by this patch, but does this bounds check
handle negative values properly?

Because the event parameter is a signed enum and the BPF verifier allows
arbitrary integers for enum arguments, a negative value bypasses the
event >= MEMCG_NR_MEMORY_EVENTS check.

Can this result in an out-of-bounds read when accessing
&memcg->memory_events[event] right below?

>  		return (unsigned long)-1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617202147.78347-1-jp.kobryn@linux.dev?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static
  2026-06-17 20:21 [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static JP Kobryn
  2026-06-17 20:57 ` sashiko-bot
@ 2026-06-17 21:06 ` Alexei Starovoitov
  2026-06-17 22:31   ` Roman Gushchin
  1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-06-17 21:06 UTC (permalink / raw)
  To: JP Kobryn
  Cc: Roman Gushchin, Shakeel Butt, Andrew Morton, Alexei Starovoitov,
	bpf, linux-mm, LKML

On Wed, Jun 17, 2026 at 1:22 PM JP Kobryn <jp.kobryn@linux.dev> wrote:
>
> The kfuncs in bpf_memcontrol.c are not called by in-kernel C code. They are
> only referenced by BPF programs and resolved through BTF.
>
> Since no external linkage is needed, mark these functions static.
>
> Fixes: 5904db9891f8 ("mm: introduce BPF kfuncs to deal with memcg pointers")
> Fixes: 5c7db3239c9f ("mm: introduce bpf_get_root_mem_cgroup() BPF kfunc")
> Fixes: 99430ab8b804 ("mm: introduce BPF kfuncs to access memcg statistics and events")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202606150332.Xy4Egd9s-lkp@intel.com/
> Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
> ---
>  mm/bpf_memcontrol.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d7647..eff79bb2c758 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c
> @@ -20,7 +20,7 @@ __bpf_kfunc_start_defs();
>   *
>   * Return: A pointer to the root memory cgroup.
>   */
> -__bpf_kfunc struct mem_cgroup *bpf_get_root_mem_cgroup(void)
> +__bpf_kfunc static struct mem_cgroup *bpf_get_root_mem_cgroup(void)

sashiko disappoints. It couldn't notice that kfuncs should not be static.
We have few "__bpf_kfunc static" in net/ipv4/
that work by sort-of "luck", since their addresses are taken.

In general kfuncs cannot be declared static.

pw-bot: cr


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static
  2026-06-17 21:06 ` Alexei Starovoitov
@ 2026-06-17 22:31   ` Roman Gushchin
  0 siblings, 0 replies; 4+ messages in thread
From: Roman Gushchin @ 2026-06-17 22:31 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: JP Kobryn, Shakeel Butt, Andrew Morton, Alexei Starovoitov, bpf,
	linux-mm, LKML

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Wed, Jun 17, 2026 at 1:22 PM JP Kobryn <jp.kobryn@linux.dev> wrote:
>>
>> The kfuncs in bpf_memcontrol.c are not called by in-kernel C code. They are
>> only referenced by BPF programs and resolved through BTF.
>>
>> Since no external linkage is needed, mark these functions static.
>>
>> Fixes: 5904db9891f8 ("mm: introduce BPF kfuncs to deal with memcg pointers")
>> Fixes: 5c7db3239c9f ("mm: introduce bpf_get_root_mem_cgroup() BPF kfunc")
>> Fixes: 99430ab8b804 ("mm: introduce BPF kfuncs to access memcg statistics and events")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202606150332.Xy4Egd9s-lkp@intel.com/
>> Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
>> ---
>>  mm/bpf_memcontrol.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
>> index 716df49d7647..eff79bb2c758 100644
>> --- a/mm/bpf_memcontrol.c
>> +++ b/mm/bpf_memcontrol.c
>> @@ -20,7 +20,7 @@ __bpf_kfunc_start_defs();
>>   *
>>   * Return: A pointer to the root memory cgroup.
>>   */
>> -__bpf_kfunc struct mem_cgroup *bpf_get_root_mem_cgroup(void)
>> +__bpf_kfunc static struct mem_cgroup *bpf_get_root_mem_cgroup(void)
>
> sashiko disappoints. It couldn't notice that kfuncs should not be static.
> We have few "__bpf_kfunc static" in net/ipv4/
> that work by sort-of "luck", since their addresses are taken.
>
> In general kfuncs cannot be declared static.

It seems like it thought hard on it but failed based on the existing
code and a bit vague documentation in Documentation/bpf/kfuncs.rst:

  kfunc definitions should also always be annotated with the ``__bpf_kfunc``
  macro. This prevents issues such as the compiler inlining the kfunc if it's a
  static kernel function, or the function ...

So it reads like bpf kfuncs can be static.

This is the reasoning from the logs (a dismissed concern, to be precise):
{
      "type": "Dead Code / Linker Error",

      "description": "Marking kfuncs as static could cause the compiler
      to optimize them away, breaking BPF verifier BTF resolution.",

      "reasoning": "The patch adds `static` to multiple `__bpf_kfunc`
      annotated functions that are not called anywhere in C
      code. Normally, a compiler would remove unused static functions or
      fail to expose them in object files, which could prevent the BTF
      ID generation logic (`resolve_btfids`) from finding them. However,
      the `__bpf_kfunc` macro expands to include `__used` and
      `__retain`, which strictly instructs the compiler and linker to
      preserve the functions in the object file regardless of
      usage. Additionally, `resolve_btfids` and `BTF_ID_FLAGS`
      successfully parse and resolve `STB_LOCAL` static symbols, making
      this change completely safe and correct.",

      "locations": [
        {
          "file": "mm/bpf_memcontrol.c",
          "function_or_symbol": null,
          "line_range": null,
          "why_this_location_matters": "This applies to all the
          functions modified by the patch."
        }
      ]
    }


Thanks

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-17 22:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 20:21 [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static JP Kobryn
2026-06-17 20:57 ` sashiko-bot
2026-06-17 21:06 ` Alexei Starovoitov
2026-06-17 22:31   ` Roman Gushchin

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.