[PATCH bpf-next 0/2] Limit local storage value size to max kmalloc_nolock() allocation

[PATCH bpf-next 0/2] Limit local storage value size to max kmalloc_nolock() allocation

[Jonathan Bastari Thamrin via B4 Relay]

BPF local storage elements are allocated using kmalloc_nolock()
but map creation is still allowed with values up to kmalloc()'s
larger limit. BPF_LOCAL_STORAGE_MAX_VALUE_SIZE is changed to be
based on KMALLOC_MAX_CACHE_SIZE instead and maps with larger
values fail on creation with E2BIG. Cgroup storage
(BPF_MAP_TYPE_CGROUP_STORAGE) uses the previous limit since it
allocates with kmalloc().

A selftest is also added to both find the largest task storage
size and test that this value can be stored. It also checks
that the size is at least 4096 to check that the limit isn't too
low.

---
Jonathan Bastari Thamrin (2):
      bpf: Limit local storage value size to max kmalloc_nolock() allocation
      selftests/bpf: Test local storage max value size

 include/linux/bpf_local_storage.h                  | 10 +--
 kernel/bpf/local_storage.c                         | 13 +++-
 .../selftests/bpf/prog_tests/task_local_storage.c  | 87 ++++++++++++++++++++++
 3 files changed, 103 insertions(+), 7 deletions(-)
---
base-commit: d346f59069676eda84b3313cbd2c2ab21045d3ee
change-id: 20261104-local-storage-max-c3709fcd6fbc

Best regards,
-- 
Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

[PATCH bpf-next 1/2] bpf: Limit local storage value size to max kmalloc_nolock() allocation

[Jonathan Bastari Thamrin via B4 Relay]

From: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

Since commit f484f4a3e058 ("bpf: Replace bpf memory allocator
with kmalloc_nolock() in local storage") for task and cgroup
local storage and commit 5063e7758899 ("bpf: Use kmalloc_nolock()
universally in local storage") for the rest, elements are
allocated using kmalloc_nolock() which has a maximum
allocation size of KMALLOC_MAX_CACHE_SIZE. Map creation still
doesn't match this and accepts values up to ~64KB. This causes
map updates to fail with ENOMEM due to the map not being able
to store anything.

BPF_LOCAL_STORAGE_MAX_VALUE_SIZE is now based on
KMALLOC_MAX_CACHE_SIZE and maps with larger values fail on
creation with E2BIG. Cgroup storage (BPF_MAP_TYPE_CGROUP_STORAGE)
still uses the previous limit as its own constant since it
allocates with kmalloc().

Fixes: f484f4a3e058 ("bpf: Replace bpf memory allocator with kmalloc_nolock() in local storage")
Fixes: 5063e7758899 ("bpf: Use kmalloc_nolock() universally in local storage")
Reported-by: Slava Imameev <slava.imameev@crowdstrike.com>
Closes: https://lore.kernel.org/bpf/20260410014341.47043-1-slava.imameev@crowdstrike.com/
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/CAADnVQKeFF--bgnZZSU12UY0muuwYA=7EdzLyOi837oZs+bXTA@mail.gmail.com/
Signed-off-by: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
---
 include/linux/bpf_local_storage.h | 10 +++++-----
 kernel/bpf/local_storage.c        | 13 +++++++++++--
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
index 6c643ac51079..e1ce75b0fb0a 100644
--- a/include/linux/bpf_local_storage.h
+++ b/include/linux/bpf_local_storage.h
@@ -117,13 +117,13 @@ bpf_local_storage_unpin_owner(struct bpf_local_storage *local_storage)
 	last = refcount_dec_and_test(&local_storage->owner_refcnt);
 }
 
-/* U16_MAX is much more than enough for sk local storage
- * considering a tcp_sock is ~2k.
+/* Elements come from kmalloc_nolock() where the max size
+ * is KMALLOC_MAX_CACHE_SIZE.
  */
-#define BPF_LOCAL_STORAGE_MAX_VALUE_SIZE				       \
+#define BPF_LOCAL_STORAGE_MAX_VALUE_SIZE                                       \
 	min_t(u32,                                                             \
-	      (KMALLOC_MAX_SIZE - MAX_BPF_STACK -                              \
-	       sizeof(struct bpf_local_storage_elem)),                         \
+	      (KMALLOC_MAX_CACHE_SIZE -                                        \
+	       offsetof(struct bpf_local_storage_elem, sdata.data)),           \
 	      (U16_MAX - sizeof(struct bpf_local_storage_elem)))
 
 #define SELEM(_SDATA)                                                          \
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 83cd527a2542..22a9841d4fd5 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -18,6 +18,15 @@
 #define LOCAL_STORAGE_CREATE_FLAG_MASK					\
 	(BPF_F_NUMA_NODE | BPF_F_ACCESS_MASK)
 
+/* Cgroup storage allocates with kmalloc() where the max size is
+ * larger than kmalloc_nolock() used by other local storages.
+ */
+#define BPF_CGROUP_STORAGE_MAX_VALUE_SIZE                                      \
+	min_t(u32,                                                             \
+	      (KMALLOC_MAX_SIZE - MAX_BPF_STACK -                              \
+	       sizeof(struct bpf_local_storage_elem)),                         \
+	      (U16_MAX - sizeof(struct bpf_local_storage_elem)))
+
 struct bpf_cgroup_storage_map {
 	struct bpf_map map;
 
@@ -296,12 +305,12 @@ static int cgroup_storage_get_next_key(struct bpf_map *_map, void *key,
 
 static struct bpf_map *cgroup_storage_map_alloc(union bpf_attr *attr)
 {
-	__u32 max_value_size = BPF_LOCAL_STORAGE_MAX_VALUE_SIZE;
+	__u32 max_value_size = BPF_CGROUP_STORAGE_MAX_VALUE_SIZE;
 	int numa_node = bpf_map_attr_numa_node(attr);
 	struct bpf_cgroup_storage_map *map;
 
 	/* percpu is bound by PCPU_MIN_UNIT_SIZE, non-percu
-	 * is the same as other local storages.
+	 * uses the cgroup storage limit above
 	 */
 	if (attr->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
 		max_value_size = min_t(__u32, max_value_size,

-- 
2.56.0

[PATCH bpf-next 2/2] selftests/bpf: Test local storage max value size

[Jonathan Bastari Thamrin via B4 Relay]

From: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

Add a test that finds the largest value size accepted for a task
storage map using binary search, then checks that a value of this
size can be stored. It also checks the size is >=4096 to make sure
the limit is not set too low. This test fails without the previous
patch with ENOMEM.

Signed-off-by: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
---
 .../selftests/bpf/prog_tests/task_local_storage.c  | 87 ++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
index 5b2b56cc3a4f..c7e9fa6743b9 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
@@ -495,6 +495,91 @@ static void test_uptr_map_failure(const char *map_name, int expected_errno)
 	uptr_map_failure__destroy(skel);
 }
 
+static int create_task_storage_map(__u32 value_size)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, create_attr);
+	struct btf *btf;
+	int int_id, char_id, arr_id, err, saved_errno;
+	int map_fd = -1;
+
+	btf = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf, "btf__new"))
+		goto done;
+
+	int_id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+	char_id = btf__add_int(btf, "char", 1, 0);
+	arr_id = btf__add_array(btf, int_id, char_id, value_size);
+	err = btf__load_into_kernel(btf);
+	if (!ASSERT_OK(err, "btf__load_into_kernel"))
+		goto done;
+
+	create_attr.map_flags = BPF_F_NO_PREALLOC;
+	create_attr.btf_fd = btf__fd(btf);
+	create_attr.btf_key_type_id = int_id;
+	create_attr.btf_value_type_id = arr_id;
+	map_fd = bpf_map_create(BPF_MAP_TYPE_TASK_STORAGE, "big_ls_map",
+				sizeof(int), value_size, 0, &create_attr);
+
+done:
+	saved_errno = errno;
+	btf__free(btf);
+	errno = saved_errno;
+	return map_fd;
+}
+
+/* Return the largest value_size the kernel accepts for a local storage
+ * map, or -1 on an unexpected error.
+ */
+static int find_max_value_size(void)
+{
+	int lo = 1, hi = 65535, mid, map_fd;
+
+	while (lo < hi) {
+		mid = lo + (hi - lo + 1) / 2;
+
+		map_fd = create_task_storage_map(mid);
+		if (map_fd >= 0) {
+			close(map_fd);
+			lo = mid;
+		} else {
+			if (!ASSERT_EQ(errno, E2BIG, "create_task_storage_map errno"))
+				return -1;
+			hi = mid - 1;
+		}
+	}
+	return lo;
+}
+
+static void test_local_storage_max_value_size(void)
+{
+	int map_fd, task_fd = -1, max_size, err;
+	void *buf = NULL;
+
+	max_size = find_max_value_size();
+	if (!ASSERT_GE(max_size, 4096, "find_max_value_size"))
+		return;
+
+	map_fd = create_task_storage_map(max_size);
+	if (!ASSERT_OK_FD(map_fd, "create_task_storage_map"))
+		return;
+
+	task_fd = sys_pidfd_open(getpid(), 0);
+	if (!ASSERT_OK_FD(task_fd, "sys_pidfd_open"))
+		goto out;
+
+	buf = calloc(1, max_size);
+	if (!ASSERT_OK_PTR(buf, "calloc"))
+		goto out;
+	err = bpf_map_update_elem(map_fd, &task_fd, buf, 0);
+	ASSERT_OK(err, "bpf_map_update_elem");
+
+out:
+	free(buf);
+	if (task_fd >= 0)
+		close(task_fd);
+	close(map_fd);
+}
+
 void test_task_local_storage(void)
 {
 	if (test__start_subtest("sys_enter_exit"))
@@ -521,5 +606,7 @@ void test_task_local_storage(void)
 		test_uptr_map_failure("empty_uptr_map", EINVAL);
 	if (test__start_subtest("uptr_map_failure_kstruct"))
 		test_uptr_map_failure("kstruct_uptr_map", EINVAL);
+	if (test__start_subtest("local_storage_max_value_size"))
+		test_local_storage_max_value_size();
 	RUN_TESTS(uptr_failure);
 }

-- 
2.56.0

Re: [PATCH bpf-next 2/2] selftests/bpf: Test local storage max value size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/bpf: non-standard multi-line comment above find_max_value_size()
--

commit 1da8998ccd74b6171959f2da642b888d3db302bd
Author: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

selftests/bpf: Test local storage max value size

This commit adds a selftest in task_local_storage.c that uses binary search
to find the largest value size accepted for a task storage map, verifies the
size is at least 4096 bytes, and checks that an element of that size can be
stored.

> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> index 5b2b56cc3a4f3..c7e9fa6743b93 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> @@ -495,6 +495,91 @@ static void test_uptr_map_failure(const char *map_name, int expected_errno)

[ ... ]

> +/* Return the largest value_size the kernel accepts for a local storage
> + * map, or -1 on an unexpected error.
> + */
> +static int find_max_value_size(void)

[Severity: Low]
This isn't a bug, but in BPF selftests, should the multi-line comment above
find_max_value_size() in tools/testing/selftests/bpf/prog_tests/task_local_storage.c
place the opening /* on its own line with the comment text starting on the
next line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-local-storage-max-v1-0-c836731bd184@gmail.com?part=2

Re: [PATCH bpf-next 1/2] bpf: Limit local storage value size to max kmalloc_nolock() allocation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: non-standard multi-line comment style in local storage files
--

commit bbb69a62fc0fe47ec15aa691029d15c86b6ea57a
Author: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>

bpf: Limit local storage value size to max kmalloc_nolock() allocation

This commit updates BPF_LOCAL_STORAGE_MAX_VALUE_SIZE to be bounded by
KMALLOC_MAX_CACHE_SIZE so that local storage maps using kmalloc_nolock()
reject oversized values with E2BIG at map creation time. It also introduces
BPF_CGROUP_STORAGE_MAX_VALUE_SIZE to preserve the previous limit for
BPF_MAP_TYPE_CGROUP_STORAGE, which allocates elements with kmalloc().

Link: https://lore.kernel.org/bpf/CAADnVQKeFF--bgnZZSU12UY0muuwYA=7EdzLyOi837oZs+bXTA@mail.gmail.com/

> diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
> index 6c643ac510799..e1ce75b0fb0ac 100644
> --- a/include/linux/bpf_local_storage.h
> +++ b/include/linux/bpf_local_storage.h
> @@ -117,13 +117,13 @@ bpf_local_storage_unpin_owner(struct bpf_local_storage *local_storage)
>  	last = refcount_dec_and_test(&local_storage->owner_refcnt);
>  }
>  
> -/* U16_MAX is much more than enough for sk local storage
> - * considering a tcp_sock is ~2k.
> +/* Elements come from kmalloc_nolock() where the max size
> + * is KMALLOC_MAX_CACHE_SIZE.
>   */
> -#define BPF_LOCAL_STORAGE_MAX_VALUE_SIZE				       \
> +#define BPF_LOCAL_STORAGE_MAX_VALUE_SIZE                                       \
>  	min_t(u32,                                                             \
> -	      (KMALLOC_MAX_SIZE - MAX_BPF_STACK -                              \
> -	       sizeof(struct bpf_local_storage_elem)),                         \
> +	      (KMALLOC_MAX_CACHE_SIZE -                                        \
> +	       offsetof(struct bpf_local_storage_elem, sdata.data)),           \
>  	      (U16_MAX - sizeof(struct bpf_local_storage_elem)))
>  
>  #define SELEM(_SDATA)                                                          \
> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index 83cd527a2542b..22a9841d4fd55 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -18,6 +18,15 @@
>  #define LOCAL_STORAGE_CREATE_FLAG_MASK					\
>  	(BPF_F_NUMA_NODE | BPF_F_ACCESS_MASK)
>  
> +/* Cgroup storage allocates with kmalloc() where the max size is
> + * larger than kmalloc_nolock() used by other local storages.
> + */
> +#define BPF_CGROUP_STORAGE_MAX_VALUE_SIZE                                      \
> +	min_t(u32,                                                             \
> +	      (KMALLOC_MAX_SIZE - MAX_BPF_STACK -                              \
> +	       sizeof(struct bpf_local_storage_elem)),                         \
> +	      (U16_MAX - sizeof(struct bpf_local_storage_elem)))
> +
>  struct bpf_cgroup_storage_map {
>  	struct bpf_map map;
>  
> @@ -296,12 +305,12 @@ static int cgroup_storage_get_next_key(struct bpf_map *_map, void *key,
>  
>  static struct bpf_map *cgroup_storage_map_alloc(union bpf_attr *attr)
>  {
> -	__u32 max_value_size = BPF_LOCAL_STORAGE_MAX_VALUE_SIZE;
> +	__u32 max_value_size = BPF_CGROUP_STORAGE_MAX_VALUE_SIZE;
>  	int numa_node = bpf_map_attr_numa_node(attr);
>  	struct bpf_cgroup_storage_map *map;
>  
>  	/* percpu is bound by PCPU_MIN_UNIT_SIZE, non-percu
> -	 * is the same as other local storages.
> +	 * uses the cgroup storage limit above
>  	 */

[Severity: Low]
This isn't a bug, but in BPF code, multi-line comments place the opening /*
on its own line with the comment text starting on the following line. Could
the multi-line comments above BPF_LOCAL_STORAGE_MAX_VALUE_SIZE in
include/linux/bpf_local_storage.h, as well as above
BPF_CGROUP_STORAGE_MAX_VALUE_SIZE and in cgroup_storage_map_alloc() in
kernel/bpf/local_storage.c, be formatted to follow this style?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-local-storage-max-v1-0-c836731bd184@gmail.com?part=1