* [PATCH bpf-next v2 1/5] selftests/bpf: Fix task_local_data data allocation size
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
@ 2026-03-31 21:35 ` Amery Hung
2026-04-01 1:48 ` sun jian
2026-03-31 21:35 ` [PATCH bpf-next v2 2/5] selftests/bpf: Simplify task_local_data memory allocation Amery Hung
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2026-03-31 21:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, yatsenko,
ameryhung, kernel-team
Currently, when allocating memory for data, size of tld_data_u->start
is not taken into account. This may cause OOB access. Fixed it by adding
the non-flexible array part of tld_data_u.
Besides, explicitly align tld_data_u->data to 8 bytes in case some
fields are added before data in the future. It could break the
assumption that every data field is 8 byte aligned and
sizeof(tld_data_u) will no longer be equal to
offsetof(struct tld_data_u, data), which we use interchangeably.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../selftests/bpf/prog_tests/task_local_data.h | 12 +++++++-----
.../selftests/bpf/progs/task_local_data.bpf.h | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
index 7819f318b2fb..a52d8b549425 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -90,7 +90,7 @@ typedef struct {
struct tld_metadata {
char name[TLD_NAME_LEN];
- _Atomic __u16 size;
+ _Atomic __u16 size; /* size of tld_data_u->data */
};
struct tld_meta_u {
@@ -101,7 +101,7 @@ struct tld_meta_u {
struct tld_data_u {
__u64 start; /* offset of tld_data_u->data in a page */
- char data[];
+ char data[] __attribute__((aligned(8)));
};
struct tld_map_value {
@@ -158,6 +158,7 @@ static int __tld_init_data_p(int map_fd)
struct tld_data_u *data;
void *data_alloc = NULL;
int err, tid_fd = -1;
+ size_t size;
tid_fd = syscall(SYS_pidfd_open, sys_gettid(), O_EXCL);
if (tid_fd < 0) {
@@ -173,9 +174,10 @@ static int __tld_init_data_p(int map_fd)
* tld_meta_p->size = TLD_DYN_DATA_SIZE +
* total size of TLDs defined via TLD_DEFINE_KEY()
*/
- data_alloc = (use_aligned_alloc || tld_meta_p->size * 2 >= TLD_PAGE_SIZE) ?
- aligned_alloc(TLD_PAGE_SIZE, tld_meta_p->size) :
- malloc(tld_meta_p->size * 2);
+ size = tld_meta_p->size + sizeof(struct tld_data_u);
+ data_alloc = (use_aligned_alloc || size * 2 >= TLD_PAGE_SIZE) ?
+ aligned_alloc(TLD_PAGE_SIZE, size) :
+ malloc(size * 2);
if (!data_alloc) {
err = -ENOMEM;
goto out;
diff --git a/tools/testing/selftests/bpf/progs/task_local_data.bpf.h b/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
index 8b6f7af43648..1f396711f487 100644
--- a/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
+++ b/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
@@ -87,7 +87,7 @@ struct tld_meta_u {
struct tld_data_u {
__u64 start; /* offset of tld_data_u->data in a page */
- char data[__PAGE_SIZE - sizeof(__u64)];
+ char data[__PAGE_SIZE - sizeof(__u64)] __attribute__((aligned(8)));
};
struct tld_map_value {
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH bpf-next v2 1/5] selftests/bpf: Fix task_local_data data allocation size
2026-03-31 21:35 ` [PATCH bpf-next v2 1/5] selftests/bpf: Fix task_local_data data allocation size Amery Hung
@ 2026-04-01 1:48 ` sun jian
0 siblings, 0 replies; 12+ messages in thread
From: sun jian @ 2026-04-01 1:48 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
On Wed, Apr 1, 2026 at 5:41 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> Currently, when allocating memory for data, size of tld_data_u->start
> is not taken into account. This may cause OOB access. Fixed it by adding
> the non-flexible array part of tld_data_u.
>
> Besides, explicitly align tld_data_u->data to 8 bytes in case some
> fields are added before data in the future. It could break the
> assumption that every data field is 8 byte aligned and
> sizeof(tld_data_u) will no longer be equal to
> offsetof(struct tld_data_u, data), which we use interchangeably.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> .../selftests/bpf/prog_tests/task_local_data.h | 12 +++++++-----
> .../selftests/bpf/progs/task_local_data.bpf.h | 2 +-
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> index 7819f318b2fb..a52d8b549425 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> @@ -90,7 +90,7 @@ typedef struct {
>
> struct tld_metadata {
> char name[TLD_NAME_LEN];
> - _Atomic __u16 size;
> + _Atomic __u16 size; /* size of tld_data_u->data */
> };
>
> struct tld_meta_u {
> @@ -101,7 +101,7 @@ struct tld_meta_u {
>
> struct tld_data_u {
> __u64 start; /* offset of tld_data_u->data in a page */
> - char data[];
> + char data[] __attribute__((aligned(8)));
> };
>
> struct tld_map_value {
> @@ -158,6 +158,7 @@ static int __tld_init_data_p(int map_fd)
> struct tld_data_u *data;
> void *data_alloc = NULL;
> int err, tid_fd = -1;
> + size_t size;
>
> tid_fd = syscall(SYS_pidfd_open, sys_gettid(), O_EXCL);
> if (tid_fd < 0) {
> @@ -173,9 +174,10 @@ static int __tld_init_data_p(int map_fd)
> * tld_meta_p->size = TLD_DYN_DATA_SIZE +
> * total size of TLDs defined via TLD_DEFINE_KEY()
> */
> - data_alloc = (use_aligned_alloc || tld_meta_p->size * 2 >= TLD_PAGE_SIZE) ?
> - aligned_alloc(TLD_PAGE_SIZE, tld_meta_p->size) :
> - malloc(tld_meta_p->size * 2);
> + size = tld_meta_p->size + sizeof(struct tld_data_u);
> + data_alloc = (use_aligned_alloc || size * 2 >= TLD_PAGE_SIZE) ?
> + aligned_alloc(TLD_PAGE_SIZE, size) :
> + malloc(size * 2);
> if (!data_alloc) {
> err = -ENOMEM;
> goto out;
> diff --git a/tools/testing/selftests/bpf/progs/task_local_data.bpf.h b/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
> index 8b6f7af43648..1f396711f487 100644
> --- a/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
> +++ b/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
> @@ -87,7 +87,7 @@ struct tld_meta_u {
>
> struct tld_data_u {
> __u64 start; /* offset of tld_data_u->data in a page */
> - char data[__PAGE_SIZE - sizeof(__u64)];
> + char data[__PAGE_SIZE - sizeof(__u64)] __attribute__((aligned(8)));
> };
>
> struct tld_map_value {
> --
> 2.52.0
>
>
Acked-by: Sun Jian <sun.jian.kdev@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v2 2/5] selftests/bpf: Simplify task_local_data memory allocation
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
2026-03-31 21:35 ` [PATCH bpf-next v2 1/5] selftests/bpf: Fix task_local_data data allocation size Amery Hung
@ 2026-03-31 21:35 ` Amery Hung
2026-04-01 1:46 ` sun jian
2026-03-31 21:35 ` [PATCH bpf-next v2 3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first Amery Hung
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2026-03-31 21:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, yatsenko,
ameryhung, kernel-team
Simplify data allocation by always using aligned_alloc() and passing
size_pot, size rounded up to the closest power of two to alignment.
Currently, aligned_alloc(page_size, size) is only intended to be used
with memory allocators that can fulfill the request without rounding
size up to page_size to conserve memory. This is enabled by defining
TLD_DATA_USE_ALIGNED_ALLOC. The reason to align to page_size is due to
the limitation of UPTR where only a page can be pinned to the kernel.
Otherwise, malloc(size * 2) is used to allocate memory for data.
However, we don't need to call aligned_alloc(page_size, size) to get
a contiguous memory of size bytes within a page. aligned_alloc(size_pot,
...) will also do the trick. Therefore, just use aligned_alloc(size_pot,
...) universally.
As for the size argument, create a new option,
TLD_DONT_ROUND_UP_DATA_SIZE, to specify not rounding up the size.
This preserves the current TLD_DATA_USE_ALIGNED_ALLOC behavior, allowing
memory allocators with low overhead aligned_alloc() to not waste memory.
To enable this, users need to make sure it is not an undefined behavior
for the memory allocator to have size not being an integral multiple of
alignment.
Compared to the current implementation, !TLD_DATA_USE_ALIGNED_ALLOC
used to always waste size-byte of memory due to malloc(size * 2).
Now the worst case becomes size - 1 and the best case is 0 when the size
is already a power of two.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/task_local_data.h | 60 +++++++------------
1 file changed, 22 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
index a52d8b549425..366a6739c086 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -50,16 +50,13 @@
* TLD_MAX_DATA_CNT.
*
*
- * TLD_DATA_USE_ALIGNED_ALLOC - Always use aligned_alloc() instead of malloc()
+ * TLD_DONT_ROUND_UP_DATA_SIZE - Don't round up memory size allocated for data if
+ * the memory allocator has low overhead aligned_alloc() implementation.
*
- * When allocating the memory for storing TLDs, we need to make sure there is a memory
- * region of the X bytes within a page. This is due to the limit posed by UPTR: memory
- * pinned to the kernel cannot exceed a page nor can it cross the page boundary. The
- * library normally calls malloc(2*X) given X bytes of total TLDs, and only uses
- * aligned_alloc(PAGE_SIZE, X) when X >= PAGE_SIZE / 2. This is to reduce memory wastage
- * as not all memory allocator can use the exact amount of memory requested to fulfill
- * aligned_alloc(). For example, some may round the size up to the alignment. Enable the
- * option to always use aligned_alloc() if the implementation has low memory overhead.
+ * For some memory allocators, when calling aligned_alloc(alignment, size), size
+ * does not need to be an integral multiple of alignment and it can be fulfilled
+ * without using round_up(size, alignment) bytes of memory. Enable this option to
+ * reduce memory usage.
*/
#define TLD_PAGE_SIZE getpagesize()
@@ -68,6 +65,8 @@
#define TLD_ROUND_MASK(x, y) ((__typeof__(x))((y) - 1))
#define TLD_ROUND_UP(x, y) ((((x) - 1) | TLD_ROUND_MASK(x, y)) + 1)
+#define TLD_ROUND_UP_POWER_OF_TWO(x) (1UL << (sizeof(x) * 8 - __builtin_clzl(x - 1)))
+
#define TLD_READ_ONCE(x) (*(volatile typeof(x) *)&(x))
#ifndef TLD_DYN_DATA_SIZE
@@ -111,7 +110,6 @@ struct tld_map_value {
struct tld_meta_u * _Atomic tld_meta_p __attribute__((weak));
__thread struct tld_data_u *tld_data_p __attribute__((weak));
-__thread void *tld_data_alloc_p __attribute__((weak));
#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
pthread_key_t tld_pthread_key __attribute__((weak));
@@ -153,12 +151,10 @@ static int __tld_init_meta_p(void)
static int __tld_init_data_p(int map_fd)
{
- bool use_aligned_alloc = false;
struct tld_map_value map_val;
struct tld_data_u *data;
- void *data_alloc = NULL;
int err, tid_fd = -1;
- size_t size;
+ size_t size, size_pot;
tid_fd = syscall(SYS_pidfd_open, sys_gettid(), O_EXCL);
if (tid_fd < 0) {
@@ -166,48 +162,37 @@ static int __tld_init_data_p(int map_fd)
goto out;
}
-#ifdef TLD_DATA_USE_ALIGNED_ALLOC
- use_aligned_alloc = true;
-#endif
-
/*
* tld_meta_p->size = TLD_DYN_DATA_SIZE +
* total size of TLDs defined via TLD_DEFINE_KEY()
*/
size = tld_meta_p->size + sizeof(struct tld_data_u);
- data_alloc = (use_aligned_alloc || size * 2 >= TLD_PAGE_SIZE) ?
- aligned_alloc(TLD_PAGE_SIZE, size) :
- malloc(size * 2);
- if (!data_alloc) {
+ size_pot = TLD_ROUND_UP_POWER_OF_TWO(size);
+#ifdef TLD_DONT_ROUND_UP_DATA_SIZE
+ data = (struct tld_data_u *)aligned_alloc(size_pot, size);
+#else
+ data = (struct tld_data_u *)aligned_alloc(size_pot, size_pot);
+#endif
+ if (!data) {
err = -ENOMEM;
goto out;
}
/*
* Always pass a page-aligned address to UPTR since the size of tld_map_value::data
- * is a page in BTF. If data_alloc spans across two pages, use the page that contains large
- * enough memory.
+ * is a page in BTF.
*/
- if (TLD_PAGE_SIZE - (~TLD_PAGE_MASK & (intptr_t)data_alloc) >= tld_meta_p->size) {
- map_val.data = (void *)(TLD_PAGE_MASK & (intptr_t)data_alloc);
- data = data_alloc;
- data->start = (~TLD_PAGE_MASK & (intptr_t)data_alloc) +
- offsetof(struct tld_data_u, data);
- } else {
- map_val.data = (void *)(TLD_ROUND_UP((intptr_t)data_alloc, TLD_PAGE_SIZE));
- data = (void *)(TLD_ROUND_UP((intptr_t)data_alloc, TLD_PAGE_SIZE));
- data->start = offsetof(struct tld_data_u, data);
- }
+ map_val.data = (void *)(TLD_PAGE_MASK & (intptr_t)data);
+ data->start = (~TLD_PAGE_MASK & (intptr_t)data) + sizeof(struct tld_data_u);
map_val.meta = TLD_READ_ONCE(tld_meta_p);
err = bpf_map_update_elem(map_fd, &tid_fd, &map_val, 0);
if (err) {
- free(data_alloc);
+ free(data);
goto out;
}
tld_data_p = data;
- tld_data_alloc_p = data_alloc;
#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
pthread_setspecific(tld_pthread_key, (void *)1);
#endif
@@ -375,9 +360,8 @@ static void *tld_get_data(int map_fd, tld_key_t key)
__attribute__((unused))
static void tld_free(void)
{
- if (tld_data_alloc_p) {
- free(tld_data_alloc_p);
- tld_data_alloc_p = NULL;
+ if (tld_data_p) {
+ free(tld_data_p);
tld_data_p = NULL;
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH bpf-next v2 2/5] selftests/bpf: Simplify task_local_data memory allocation
2026-03-31 21:35 ` [PATCH bpf-next v2 2/5] selftests/bpf: Simplify task_local_data memory allocation Amery Hung
@ 2026-04-01 1:46 ` sun jian
0 siblings, 0 replies; 12+ messages in thread
From: sun jian @ 2026-04-01 1:46 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
On Wed, Apr 1, 2026 at 5:41 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> Simplify data allocation by always using aligned_alloc() and passing
> size_pot, size rounded up to the closest power of two to alignment.
>
> Currently, aligned_alloc(page_size, size) is only intended to be used
> with memory allocators that can fulfill the request without rounding
> size up to page_size to conserve memory. This is enabled by defining
> TLD_DATA_USE_ALIGNED_ALLOC. The reason to align to page_size is due to
> the limitation of UPTR where only a page can be pinned to the kernel.
> Otherwise, malloc(size * 2) is used to allocate memory for data.
>
> However, we don't need to call aligned_alloc(page_size, size) to get
> a contiguous memory of size bytes within a page. aligned_alloc(size_pot,
> ...) will also do the trick. Therefore, just use aligned_alloc(size_pot,
> ...) universally.
>
> As for the size argument, create a new option,
> TLD_DONT_ROUND_UP_DATA_SIZE, to specify not rounding up the size.
The default path looks right to me.
I'd suggest dropping TLD_DONT_ROUND_UP_DATA_SIZE, since it seems to
rely on allocator-specific, non-portable aligned_alloc() behavior.
Sun Jian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v2 3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
2026-03-31 21:35 ` [PATCH bpf-next v2 1/5] selftests/bpf: Fix task_local_data data allocation size Amery Hung
2026-03-31 21:35 ` [PATCH bpf-next v2 2/5] selftests/bpf: Simplify task_local_data memory allocation Amery Hung
@ 2026-03-31 21:35 ` Amery Hung
2026-04-01 2:16 ` sun jian
2026-03-31 21:35 ` [PATCH bpf-next v2 4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header Amery Hung
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2026-03-31 21:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, yatsenko,
ameryhung, kernel-team
Without specifying constructor priority of the hidden constructor
function defined by TLD_DEFINE_KEY, __tld_create_key(..., dyn_data =
false) may run after tld_get_data() called from other constructors.
Threads calling tld_get_data() before __tld_create_key(..., dyn_data
= false) will not allocate enough memory for all TLDs and later result
in OOB access. Therefore, set it to the lowest value available to
users. Note that lower means higher priority and 0-100 is reserved to
the compiler.
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
tools/testing/selftests/bpf/prog_tests/task_local_data.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
index 366a6739c086..e242c455ddae 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -278,7 +278,7 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
#define TLD_DEFINE_KEY(key, name, size) \
tld_key_t key; \
\
-__attribute__((constructor)) \
+__attribute__((constructor(101))) \
void __tld_define_key_##key(void) \
{ \
key = __tld_create_key(name, size, false); \
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH bpf-next v2 3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first
2026-03-31 21:35 ` [PATCH bpf-next v2 3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first Amery Hung
@ 2026-04-01 2:16 ` sun jian
0 siblings, 0 replies; 12+ messages in thread
From: sun jian @ 2026-04-01 2:16 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
On Wed, Apr 1, 2026 at 5:43 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> Without specifying constructor priority of the hidden constructor
> function defined by TLD_DEFINE_KEY, __tld_create_key(..., dyn_data =
> false) may run after tld_get_data() called from other constructors.
> Threads calling tld_get_data() before __tld_create_key(..., dyn_data
> = false) will not allocate enough memory for all TLDs and later result
> in OOB access. Therefore, set it to the lowest value available to
> users. Note that lower means higher priority and 0-100 is reserved to
> the compiler.
>
> Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> tools/testing/selftests/bpf/prog_tests/task_local_data.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> index 366a6739c086..e242c455ddae 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> @@ -278,7 +278,7 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
> #define TLD_DEFINE_KEY(key, name, size) \
> tld_key_t key; \
> \
> -__attribute__((constructor)) \
> +__attribute__((constructor(101))) \
> void __tld_define_key_##key(void) \
> { \
> key = __tld_create_key(name, size, false); \
> --
> 2.52.0
>
>
Acked-by: Sun Jian <sun.jian.kdev@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v2 4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
` (2 preceding siblings ...)
2026-03-31 21:35 ` [PATCH bpf-next v2 3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first Amery Hung
@ 2026-03-31 21:35 ` Amery Hung
2026-04-01 4:22 ` sun jian
2026-03-31 21:35 ` [PATCH bpf-next v2 5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak Amery Hung
2026-04-02 22:20 ` [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement patchwork-bot+netdevbpf
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2026-03-31 21:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, yatsenko,
ameryhung, kernel-team
TLD_READ_ONCE() is redundant as the only reference passed to it is
defined as _Atomic. The load is guaranteed to be atomic in C11 standard
(6.2.6.1). Drop the macro.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
tools/testing/selftests/bpf/prog_tests/task_local_data.h | 8 +++-----
.../selftests/bpf/prog_tests/test_task_local_data.c | 2 +-
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
index e242c455ddae..91f3486439bf 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -67,8 +67,6 @@
#define TLD_ROUND_UP_POWER_OF_TWO(x) (1UL << (sizeof(x) * 8 - __builtin_clzl(x - 1)))
-#define TLD_READ_ONCE(x) (*(volatile typeof(x) *)&(x))
-
#ifndef TLD_DYN_DATA_SIZE
#define TLD_DYN_DATA_SIZE 64
#endif
@@ -184,7 +182,7 @@ static int __tld_init_data_p(int map_fd)
*/
map_val.data = (void *)(TLD_PAGE_MASK & (intptr_t)data);
data->start = (~TLD_PAGE_MASK & (intptr_t)data) + sizeof(struct tld_data_u);
- map_val.meta = TLD_READ_ONCE(tld_meta_p);
+ map_val.meta = tld_meta_p;
err = bpf_map_update_elem(map_fd, &tid_fd, &map_val, 0);
if (err) {
@@ -207,7 +205,7 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
int err, i, sz, off = 0;
__u16 cnt;
- if (!TLD_READ_ONCE(tld_meta_p)) {
+ if (!tld_meta_p) {
err = __tld_init_meta_p();
if (err)
return (tld_key_t){(__s16)err};
@@ -338,7 +336,7 @@ static inline int tld_key_err_or_zero(tld_key_t key)
__attribute__((unused))
static void *tld_get_data(int map_fd, tld_key_t key)
{
- if (!TLD_READ_ONCE(tld_meta_p))
+ if (!tld_meta_p)
return NULL;
/* tld_data_p is allocated on the first invocation of tld_get_data() */
diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c b/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
index 9556ad3d986f..e219ff506b56 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
@@ -26,7 +26,7 @@ TLD_DEFINE_KEY(value0_key, "value0", sizeof(int));
*/
static void reset_tld(void)
{
- if (TLD_READ_ONCE(tld_meta_p)) {
+ if (tld_meta_p) {
/* Remove TLDs created by tld_create_key() */
tld_meta_p->cnt = 1;
tld_meta_p->size = TLD_DYN_DATA_SIZE;
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH bpf-next v2 4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header
2026-03-31 21:35 ` [PATCH bpf-next v2 4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header Amery Hung
@ 2026-04-01 4:22 ` sun jian
0 siblings, 0 replies; 12+ messages in thread
From: sun jian @ 2026-04-01 4:22 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
On Wed, Apr 1, 2026 at 5:48 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> TLD_READ_ONCE() is redundant as the only reference passed to it is
> defined as _Atomic. The load is guaranteed to be atomic in C11 standard
> (6.2.6.1). Drop the macro.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> tools/testing/selftests/bpf/prog_tests/task_local_data.h | 8 +++-----
> .../selftests/bpf/prog_tests/test_task_local_data.c | 2 +-
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> index e242c455ddae..91f3486439bf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> @@ -67,8 +67,6 @@
>
> #define TLD_ROUND_UP_POWER_OF_TWO(x) (1UL << (sizeof(x) * 8 - __builtin_clzl(x - 1)))
>
> -#define TLD_READ_ONCE(x) (*(volatile typeof(x) *)&(x))
> -
> #ifndef TLD_DYN_DATA_SIZE
> #define TLD_DYN_DATA_SIZE 64
> #endif
> @@ -184,7 +182,7 @@ static int __tld_init_data_p(int map_fd)
> */
> map_val.data = (void *)(TLD_PAGE_MASK & (intptr_t)data);
> data->start = (~TLD_PAGE_MASK & (intptr_t)data) + sizeof(struct tld_data_u);
> - map_val.meta = TLD_READ_ONCE(tld_meta_p);
> + map_val.meta = tld_meta_p;
>
> err = bpf_map_update_elem(map_fd, &tid_fd, &map_val, 0);
> if (err) {
> @@ -207,7 +205,7 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
> int err, i, sz, off = 0;
> __u16 cnt;
>
> - if (!TLD_READ_ONCE(tld_meta_p)) {
> + if (!tld_meta_p) {
> err = __tld_init_meta_p();
> if (err)
> return (tld_key_t){(__s16)err};
> @@ -338,7 +336,7 @@ static inline int tld_key_err_or_zero(tld_key_t key)
> __attribute__((unused))
> static void *tld_get_data(int map_fd, tld_key_t key)
> {
> - if (!TLD_READ_ONCE(tld_meta_p))
> + if (!tld_meta_p)
> return NULL;
>
> /* tld_data_p is allocated on the first invocation of tld_get_data() */
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c b/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
> index 9556ad3d986f..e219ff506b56 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_task_local_data.c
> @@ -26,7 +26,7 @@ TLD_DEFINE_KEY(value0_key, "value0", sizeof(int));
> */
> static void reset_tld(void)
> {
> - if (TLD_READ_ONCE(tld_meta_p)) {
> + if (tld_meta_p) {
> /* Remove TLDs created by tld_create_key() */
> tld_meta_p->cnt = 1;
> tld_meta_p->size = TLD_DYN_DATA_SIZE;
> --
> 2.52.0
>
>
Acked-by: Sun Jian <sun.jian.kdev@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v2 5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
` (3 preceding siblings ...)
2026-03-31 21:35 ` [PATCH bpf-next v2 4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header Amery Hung
@ 2026-03-31 21:35 ` Amery Hung
2026-04-01 4:11 ` sun jian
2026-04-02 22:20 ` [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement patchwork-bot+netdevbpf
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2026-03-31 21:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, yatsenko,
ameryhung, kernel-team
If TLD_FREE_DATA_ON_THREAD_EXIT is not enabled in a translation unit
that calls __tld_create_key() first, another translation unit that
enables it will not get the auto cleanup feature as pthread key is only
created once when allocation metadata. Fix it by always try to create
the pthread key when __tld_create_key() is called.
Also improve the documentation:
- Discourage user from using different options in different translation
units
- Specify calling tld_free() before thread exit as undefined behavior
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/task_local_data.h | 25 +++++++++++++------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
index 91f3486439bf..1e5c67c78ffb 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -22,14 +22,17 @@
/*
* OPTIONS
*
- * Define the option before including the header
+ * Define the option before including the header. Using different options in
+ * different translation units is strongly discouraged.
*
* TLD_FREE_DATA_ON_THREAD_EXIT - Frees memory on thread exit automatically
*
* Thread-specific memory for storing TLD is allocated lazily on the first call to
* tld_get_data(). The thread that calls it must also call tld_free() on thread exit
* to prevent memory leak. Pthread will be included if the option is defined. A pthread
- * key will be registered with a destructor that calls tld_free().
+ * key will be registered with a destructor that calls tld_free(). Enabled only when
+ * the option is defined and TLD_DEFINE_KEY/tld_create_key() is called in the same
+ * translation unit.
*
*
* TLD_DYN_DATA_SIZE - The maximum size of memory allocated for TLDs created dynamically
@@ -47,7 +50,7 @@
* TLD_NAME_LEN - The maximum length of the name of a TLD (default: 62)
*
* Setting TLD_NAME_LEN will affect the maximum number of TLDs a process can store,
- * TLD_MAX_DATA_CNT.
+ * TLD_MAX_DATA_CNT. Must be consistent with task_local_data.bpf.h.
*
*
* TLD_DONT_ROUND_UP_DATA_SIZE - Don't round up memory size allocated for data if
@@ -110,6 +113,7 @@ struct tld_meta_u * _Atomic tld_meta_p __attribute__((weak));
__thread struct tld_data_u *tld_data_p __attribute__((weak));
#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
+bool _Atomic tld_pthread_key_init __attribute__((weak));
pthread_key_t tld_pthread_key __attribute__((weak));
static void tld_free(void);
@@ -140,9 +144,6 @@ static int __tld_init_meta_p(void)
goto out;
}
-#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
- pthread_key_create(&tld_pthread_key, __tld_thread_exit_handler);
-#endif
out:
return err;
}
@@ -203,6 +204,7 @@ static int __tld_init_data_p(int map_fd)
static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
{
int err, i, sz, off = 0;
+ bool uninit = false;
__u16 cnt;
if (!tld_meta_p) {
@@ -211,6 +213,14 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
return (tld_key_t){(__s16)err};
}
+#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
+ if (atomic_compare_exchange_strong(&tld_pthread_key_init, &uninit, true)) {
+ err = pthread_key_create(&tld_pthread_key, __tld_thread_exit_handler);
+ if (err)
+ return (tld_key_t){(__s16)err};
+ }
+#endif
+
for (i = 0; i < (int)TLD_MAX_DATA_CNT; i++) {
retry:
cnt = atomic_load(&tld_meta_p->cnt);
@@ -353,7 +363,8 @@ static void *tld_get_data(int map_fd, tld_key_t key)
*
* Users must call tld_free() on thread exit to prevent memory leak. Alternatively,
* define TLD_FREE_DATA_ON_THREAD_EXIT and a thread exit handler will be registered
- * to free the memory automatically.
+ * to free the memory automatically. Calling tld_free() before thread exit is
+ * undefined behavior, which may lead to null-pointer dereference.
*/
__attribute__((unused))
static void tld_free(void)
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH bpf-next v2 5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak
2026-03-31 21:35 ` [PATCH bpf-next v2 5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak Amery Hung
@ 2026-04-01 4:11 ` sun jian
0 siblings, 0 replies; 12+ messages in thread
From: sun jian @ 2026-04-01 4:11 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
On Wed, Apr 1, 2026 at 5:42 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> If TLD_FREE_DATA_ON_THREAD_EXIT is not enabled in a translation unit
> that calls __tld_create_key() first, another translation unit that
> enables it will not get the auto cleanup feature as pthread key is only
> created once when allocation metadata. Fix it by always try to create
> the pthread key when __tld_create_key() is called.
>
> Also improve the documentation:
> - Discourage user from using different options in different translation
> units
> - Specify calling tld_free() before thread exit as undefined behavior
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> .../bpf/prog_tests/task_local_data.h | 25 +++++++++++++------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> index 91f3486439bf..1e5c67c78ffb 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> @@ -22,14 +22,17 @@
> /*
> * OPTIONS
> *
> - * Define the option before including the header
> + * Define the option before including the header. Using different options in
> + * different translation units is strongly discouraged.
> *
> * TLD_FREE_DATA_ON_THREAD_EXIT - Frees memory on thread exit automatically
> *
> * Thread-specific memory for storing TLD is allocated lazily on the first call to
> * tld_get_data(). The thread that calls it must also call tld_free() on thread exit
> * to prevent memory leak. Pthread will be included if the option is defined. A pthread
> - * key will be registered with a destructor that calls tld_free().
> + * key will be registered with a destructor that calls tld_free(). Enabled only when
> + * the option is defined and TLD_DEFINE_KEY/tld_create_key() is called in the same
> + * translation unit.
> *
> *
> * TLD_DYN_DATA_SIZE - The maximum size of memory allocated for TLDs created dynamically
> @@ -47,7 +50,7 @@
> * TLD_NAME_LEN - The maximum length of the name of a TLD (default: 62)
> *
> * Setting TLD_NAME_LEN will affect the maximum number of TLDs a process can store,
> - * TLD_MAX_DATA_CNT.
> + * TLD_MAX_DATA_CNT. Must be consistent with task_local_data.bpf.h.
> *
> *
> * TLD_DONT_ROUND_UP_DATA_SIZE - Don't round up memory size allocated for data if
> @@ -110,6 +113,7 @@ struct tld_meta_u * _Atomic tld_meta_p __attribute__((weak));
> __thread struct tld_data_u *tld_data_p __attribute__((weak));
>
> #ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> +bool _Atomic tld_pthread_key_init __attribute__((weak));
> pthread_key_t tld_pthread_key __attribute__((weak));
>
> static void tld_free(void);
> @@ -140,9 +144,6 @@ static int __tld_init_meta_p(void)
> goto out;
> }
>
> -#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> - pthread_key_create(&tld_pthread_key, __tld_thread_exit_handler);
> -#endif
> out:
> return err;
> }
> @@ -203,6 +204,7 @@ static int __tld_init_data_p(int map_fd)
> static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
> {
> int err, i, sz, off = 0;
> + bool uninit = false;
> __u16 cnt;
>
> if (!tld_meta_p) {
> @@ -211,6 +213,14 @@ static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
> return (tld_key_t){(__s16)err};
> }
>
> +#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> + if (atomic_compare_exchange_strong(&tld_pthread_key_init, &uninit, true)) {
> + err = pthread_key_create(&tld_pthread_key, __tld_thread_exit_handler);
> + if (err)
tld_pthread_key_init would still be true, so later callers would not
retry key creation.
sun jian
> + return (tld_key_t){(__s16)err};
> + }
> +#endif
> +
> for (i = 0; i < (int)TLD_MAX_DATA_CNT; i++) {
> retry:
> cnt = atomic_load(&tld_meta_p->cnt);
> @@ -353,7 +363,8 @@ static void *tld_get_data(int map_fd, tld_key_t key)
> *
> * Users must call tld_free() on thread exit to prevent memory leak. Alternatively,
> * define TLD_FREE_DATA_ON_THREAD_EXIT and a thread exit handler will be registered
> - * to free the memory automatically.
> + * to free the memory automatically. Calling tld_free() before thread exit is
> + * undefined behavior, which may lead to null-pointer dereference.
> */
> __attribute__((unused))
> static void tld_free(void)
> --
> 2.52.0
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement
2026-03-31 21:35 [PATCH bpf-next v2 0/5] Task local data bug fixes and improvement Amery Hung
` (4 preceding siblings ...)
2026-03-31 21:35 ` [PATCH bpf-next v2 5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak Amery Hung
@ 2026-04-02 22:20 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-04-02 22:20 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
yatsenko, kernel-team
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 31 Mar 2026 14:35:50 -0700 you wrote:
> Hi,
>
> This patchset fixed three task local data bugs, improved the
> memory allocation code, and dropped unnecessary TLD_READ_ONCE. Please
> find the detail in each patch's commit msg.
>
> One thing worth mentioning is that Patch 3 allows us to renable task
> local data selftests as the library now always calls aligned_alloc()
> with size matching alignment under default configuration.
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/5] selftests/bpf: Fix task_local_data data allocation size
https://git.kernel.org/bpf/bpf-next/c/7c8ca532a741
- [bpf-next,v2,2/5] selftests/bpf: Simplify task_local_data memory allocation
https://git.kernel.org/bpf/bpf-next/c/bb6d9f5cf1d4
- [bpf-next,v2,3/5] selftests/bpf: Make sure TLD_DEFINE_KEY runs first
https://git.kernel.org/bpf/bpf-next/c/80aa8e9c64d0
- [bpf-next,v2,4/5] selftests/bpf: Remove TLD_READ_ONCE() in the user space header
https://git.kernel.org/bpf/bpf-next/c/0b481a6915ed
- [bpf-next,v2,5/5] selftests/bpf: Improve task local data documentation and fix potential memory leak
https://git.kernel.org/bpf/bpf-next/c/63f5156a9c3e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread