Hi Joanne,
On Wed, Jul 20, 2022 at 7:49 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> Add an additional test, "data_slice_use_after_release2", for ensuring
> that data slices are correctly invalidated by the verifier after the
> dynptr whose ref obj id they track is released. In particular, this
> tests data slice invalidation for dynptrs located at a non-zero offset
> from the frame pointer.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> .../testing/selftests/bpf/prog_tests/dynptr.c | 3 +-
> .../testing/selftests/bpf/progs/dynptr_fail.c | 32 ++++++++++++++++++-
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> index 3c7aa82b98e2..bcf80b9f7c27 100644
> --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
> +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> @@ -22,7 +22,8 @@ static struct {
> {"add_dynptr_to_map2", "invalid indirect read from stack"},
> {"data_slice_out_of_bounds_ringbuf", "value is outside of the allowed memory range"},
> {"data_slice_out_of_bounds_map_value", "value is outside of the allowed memory range"},
> - {"data_slice_use_after_release", "invalid mem access 'scalar'"},
> + {"data_slice_use_after_release1", "invalid mem access 'scalar'"},
> + {"data_slice_use_after_release2", "invalid mem access 'scalar'"},
> {"data_slice_missing_null_check1", "invalid mem access 'mem_or_null'"},
> {"data_slice_missing_null_check2", "invalid mem access 'mem_or_null'"},
> {"invalid_helper1", "invalid indirect read from stack"},
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> index d811cff73597..d8c4ed3ee146 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> @@ -248,7 +248,7 @@ int data_slice_out_of_bounds_map_value(void *ctx)
>
> /* A data slice can't be used after it has been released */
> SEC("?raw_tp/sys_nanosleep")
> -int data_slice_use_after_release(void *ctx)
> +int data_slice_use_after_release1(void *ctx)
> {
> struct bpf_dynptr ptr;
> struct sample *sample;
> @@ -272,6 +272,36 @@ int data_slice_use_after_release(void *ctx)
> return 0;
> }
>
> +SEC("?raw_tp/sys_nanosleep")
> +int data_slice_use_after_release2(void *ctx)
Could you put comments explaining the reason for failure, like other test cases?
> +{
> + struct bpf_dynptr ptr1, ptr2;
> + struct sample *sample;
> +
> + bpf_ringbuf_reserve_dynptr(&ringbuf, 64, 0, &ptr1);
> + bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(*sample), 0, &ptr2);
> +
> + sample = bpf_dynptr_data(&ptr2, 0, sizeof(*sample));
> + if (!sample)
> + goto done;
> +
> + sample->pid = 23;
> +
> + bpf_ringbuf_submit_dynptr(&ptr2, 0);
> +
> + /* this should fail */
> + sample->pid = 23;
> +
> + bpf_ringbuf_submit_dynptr(&ptr1, 0);
> +
> + return 0;
> +
> +done:
> + bpf_ringbuf_discard_dynptr(&ptr2, 0);
> + bpf_ringbuf_discard_dynptr(&ptr1, 0);
> + return 0;
> +}
> +
Joanne, I haven't been following the effort of dynptr, so I am still
learning dynptr. Is there any use of `ptr1` in this test case?
> /* A data slice must be first checked for NULL */
> SEC("?raw_tp/sys_nanosleep")
> int data_slice_missing_null_check1(void *ctx)
> --
> 2.30.2
>
