On Thu, Apr 20, 2023 at 12:15 AM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> Add various tests for the added dynptr convenience helpers.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/bpf_kfuncs.h | 6 +
> .../testing/selftests/bpf/prog_tests/dynptr.c | 6 +
> .../testing/selftests/bpf/progs/dynptr_fail.c | 287 +++++++++++++++++
> .../selftests/bpf/progs/dynptr_success.c | 298 ++++++++++++++++++
> 4 files changed, 597 insertions(+)
>
Very nice and thorough set of tests, thanks a lot! I left a comment
below, please follow up with requested improvement.
Applied the patch set to bpf-next. Let's continue discussion about
TRUSTED_ARGS and PTR_TO_BTF_ID for dynptr and do necessary follow ups
separately.
[...]
> +/* Invalidating a dynptr should invalidate any data slices
> + * of its clones
> + */
> +SEC("?raw_tp")
> +__failure __msg("invalid mem access 'scalar'")
> +int clone_invalidate4(void *ctx)
> +{
> + struct bpf_dynptr ptr;
> + struct bpf_dynptr clone;
> + int *data;
> +
> + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
> +
> + bpf_dynptr_clone(&ptr, &clone);
> + data = bpf_dynptr_data(&clone, 0, sizeof(val));
> + if (!data)
you'd need bpf_ringbuf_discard_dynptr() here to make sure that
compiler code generation changes don't suddenly start failing due to
missing dynptr release operation. Please send a small follow up making
this more robust.
> + return 0;
> +
> + bpf_ringbuf_submit_dynptr(&ptr, 0);
> +
> + /* this should fail */
> + *data = 123;
> +
> + return 0;
> +}
> +
> +/* Invalidating a dynptr should invalidate any data slices
> + * of its parent
> + */
> +SEC("?raw_tp")
> +__failure __msg("invalid mem access 'scalar'")
> +int clone_invalidate5(void *ctx)
> +{
> + struct bpf_dynptr ptr;
> + struct bpf_dynptr clone;
> + int *data;
> +
> + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
> + data = bpf_dynptr_data(&ptr, 0, sizeof(val));
> + if (!data)
ditto, we need to make sure we violate only one rule (using
invalidated slices), not the overall dynptr usage pattern
> + return 0;
> +
> + bpf_dynptr_clone(&ptr, &clone);
> +
> + bpf_ringbuf_submit_dynptr(&clone, 0);
> +
> + /* this should fail */
> + *data = 123;
> +
> + return 0;
> +}
> +
[...]
