[PATCH bpf-next v1 2/3] selftests/bpf: Fix tld_get_data() returning garbage data

[Amery Hung <ameryhung@gmail.com>]

BPF side tld_get_data() currently may return garbage when tld_data_u is
not aligned to page_size. This can happen when small amount of memory
is allocated for tld_data_u. The misalignment is supposed to be allowed
and the BPF side will use tld_data_u->start to reference the tld_data_u
in a page. However, since "start" is within tld_data_u, there is no way
to know the correct "start" in the first place. As a result, BPF
programs will see garbage data. The selftest did not catch this since
it tries to allocate the maximum amount of data possible (i.e., a page)
such that tld_data_u->start is always correct.

Fix it by moving tld_data_u->start to tld_data_map->start. The original
field is now renamed as unused instead of removing it because BPF side
tld_get_data() views off = 0 returned from tld_fetch_key() as
uninitialized.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 .../testing/selftests/bpf/prog_tests/task_local_data.h | 10 ++++++++--
 .../testing/selftests/bpf/progs/task_local_data.bpf.h  |  5 +++--
 2 files changed, 11 insertions(+), 4 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 489f07045c9f..8ae4fb2027f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_local_data.h
+++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
@@ -99,14 +99,20 @@ struct tld_meta_u {
 	struct tld_metadata metadata[];
 };
 
+/*
+ * The unused field ensures map_val.start > 0. On the BPF side, __tld_fetch_key()
+ * calculates off by summing map_val.start and tld_key_t.off and treats off == 0
+ * as key not cached.
+ */
 struct tld_data_u {
-	__u64 start; /* offset of tld_data_u->data in a page */
+	__u64 unused;
 	char data[] __attribute__((aligned(8)));
 };
 
 struct tld_map_value {
 	void *data;
 	struct tld_meta_u *meta;
+	__u16 start; /* offset of tld_data_u->data in a page */
 };
 
 struct tld_meta_u * _Atomic tld_meta_p __attribute__((weak));
@@ -182,7 +188,7 @@ static int __tld_init_data_p(int map_fd)
 	 * is a page in BTF.
 	 */
 	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.start = (~TLD_PAGE_MASK & (intptr_t)data) + sizeof(struct tld_data_u);
 	map_val.meta = tld_meta_p;
 
 	err = bpf_map_update_elem(map_fd, &tid_fd, &map_val, 0);
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 1f396711f487..0df8a12fd61e 100644
--- a/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
+++ b/tools/testing/selftests/bpf/progs/task_local_data.bpf.h
@@ -86,13 +86,14 @@ struct tld_meta_u {
 };
 
 struct tld_data_u {
-	__u64 start; /* offset of tld_data_u->data in a page */
+	__u64 unused;
 	char data[__PAGE_SIZE - sizeof(__u64)] __attribute__((aligned(8)));
 };
 
 struct tld_map_value {
 	struct tld_data_u __uptr *data;
 	struct tld_meta_u __uptr *meta;
+	__u16 start; /* offset of tld_data_u->data in a page */
 };
 
 typedef struct tld_uptr_dummy {
@@ -176,7 +177,7 @@ static int __tld_fetch_key(struct tld_object *tld_obj, const char *name, int i_s
 	if (!tld_obj->data_map || !tld_obj->data_map->data || !tld_obj->data_map->meta)
 		return 0;
 
-	start = tld_obj->data_map->data->start;
+	start = tld_obj->data_map->start;
 	cnt = tld_obj->data_map->meta->cnt;
 	metadata = tld_obj->data_map->meta->metadata;
 
-- 
2.52.0