On Wed, Feb 9, 2022 at 4:37 PM Delyan Kratunov <delyank@xxxxxx> wrote:
>
> Multiple test cases already fail if you add a type whose size is
> different between userspace and bpf. That said, let's also add an
> explicit test that ensures mis-sized reads/writes do not actually
> happen. This test case fails before this patch series and passes after:
>
> test_skeleton:FAIL:writes and reads match size unexpected writes
> and reads match size: actual 3735928559 != expected 8030895855
> test_skeleton:FAIL:skeleton uses underlying type unexpected
> skeleton uses underlying type: actual 8 != expected 4
>
> Signed-off-by: Delyan Kratunov <delyank@xxxxxx>
> ---
> tools/testing/selftests/bpf/prog_tests/skeleton.c | 6 ++++++
> tools/testing/selftests/bpf/progs/test_skeleton.c | 8 ++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> index 9894e1b39211..bc07da929566 100644
> --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
> +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> @@ -97,6 +97,9 @@ void test_skeleton(void)
>
> skel->data_read_mostly->read_mostly_var = 123;
>
> + /* validate apparent 64-bit value is actually 32-bit */
> + skel->data->intest64 = (typeof(skel->data->intest64)) 0xdeadbeefdeadbeefULL;
> +
> err = test_skeleton__attach(skel);
> if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
> goto cleanup;
> @@ -126,6 +129,9 @@ void test_skeleton(void)
> ASSERT_OK_PTR(elf_bytes, "elf_bytes");
> ASSERT_GE(elf_bytes_sz, 0, "elf_bytes_sz");
>
> + ASSERT_EQ(skel->data->outtest64, skel->data->intest64, "writes and reads match size");
> + ASSERT_EQ(sizeof(skel->data->intest64), sizeof(u32), "skeleton uses underlying type");
> +
> cleanup:
> test_skeleton__destroy(skel);
> }
> diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c
> index 1b1187d2967b..fd1f4910cf42 100644
> --- a/tools/testing/selftests/bpf/progs/test_skeleton.c
> +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
> @@ -16,6 +16,13 @@ struct s {
> int in1 = -1;
> long long in2 = -1;
>
> +/* declare the int64_t type to actually be 32-bit to ensure the skeleton
> + * uses actual sizes and doesn't just copy the type name
> + */
> +typedef __s32 int64_t;
> +int64_t intest64 = -1;
> +int64_t outtest64 = -1;
This will be so confusing... But when you drop __s32 special handling
you can just use __s32 directly, right?
> +
> /* .bss section */
> char in3 = '\0';
> long long in4 __attribute__((aligned(64))) = 0;
> @@ -62,6 +69,7 @@ int handler(const void *ctx)
> out4 = in4;
> out5 = in5;
> out6 = in.in6;
> + outtest64 = intest64;
>
> bpf_syscall = CONFIG_BPF_SYSCALL;
> kern_ver = LINUX_KERNEL_VERSION;
> --
> 2.34.1
