On Sat, Aug 17, 2024 at 5:24 PM Jordan Rome <linux@xxxxxxxxxxxxxx> wrote:
>
> This adds tests for both the happy path and
> the error path.
>
> Signed-off-by: Jordan Rome <linux@xxxxxxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 8 ++-
> .../selftests/bpf/prog_tests/read_vsyscall.c | 1 +
> .../selftests/bpf/progs/read_vsyscall.c | 9 ++-
> .../selftests/bpf/progs/test_attach_probe.c | 57 ++++++++++++++++++-
> 4 files changed, 68 insertions(+), 7 deletions(-)
>
Thanks for adding more test cases! See a small nit below, but otherwise LGTM
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
[...]
>
> +static __always_inline bool verify_sleepable_user_copy_str(void)
> +{
> + int ret;
> + char data_long[20];
> + char data_long_pad[20];
> + char data_long_err[20];
> + char data_short[4];
> + char data_short_pad[4];
> +
> + ret = bpf_copy_from_user_str(data_short, sizeof(data_short), user_ptr, 0);
> +
> + if (bpf_strncmp(data_short, 4, "tes\0") != 0 || ret != 4)
> + return false;
> +
> + ret = bpf_copy_from_user_str(data_short_pad, sizeof(data_short_pad), user_ptr, BPF_F_PAD_ZEROS);
> +
> + if (bpf_strncmp(data_short, 4, "tes\0") != 0 || ret != 4)
> + return false;
> +
> + ret = bpf_copy_from_user_str(data_long, sizeof(data_long), user_ptr, 0);
> +
> + if (bpf_strncmp(data_long, 10, "test_data\0") != 0 || ret != 10)
> + return false;
> +
> + ret = bpf_copy_from_user_str(data_long_pad, sizeof(data_long_pad), user_ptr, BPF_F_PAD_ZEROS);
> +
> + if (bpf_strncmp(data_long_pad, 10, "test_data\0") != 0 || ret != 10 || data_long_pad[19] != '\0')
> + return false;
> +
> + ret = bpf_copy_from_user_str(data_long_err, sizeof(data_long_err), (void *)data_long, BPF_F_PAD_ZEROS);
> +
> + if (ret > 0 || data_long_err[9] != '\0')
shouldn't the condition be something along the data_long_err[0] !=
'\0' || data_long_err[19] != '\0' to check that the entire buffer is
zeroed out?
> + return false;
> +
> + ret = bpf_copy_from_user_str(data_long, sizeof(data_long), user_ptr, 2);
> +
> + if (ret != -EINVAL)
> + return false;
> +
> + return true;
> +}
> +
[...]
