On Tue, Aug 13, 2024 at 2:24 PM Amery Hung <amery.hung@xxxxxxxxxxxxx> wrote:
>
> From: Dave Marchevsky <davemarchevsky@xxxxxx>
>
> Test stashing both referenced kptr and local kptr into local kptrs. Then,
> test unstashing them.
>
> Acked-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
> Acked-by: Hou Tao <houtao1@xxxxxxxxxx>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> Signed-off-by: Amery Hung <amery.hung@xxxxxxxxxxxxx>
> ---
> .../selftests/bpf/progs/local_kptr_stash.c | 30 +++++++++++++++++--
> .../selftests/bpf/progs/task_kfunc_success.c | 26 +++++++++++++++-
> 2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/local_kptr_stash.c b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> index 75043ffc5dad..b092a72b2c9d 100644
> --- a/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> +++ b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> @@ -8,9 +8,12 @@
> #include "../bpf_experimental.h"
> #include "../bpf_testmod/bpf_testmod_kfunc.h"
>
> +struct plain_local;
> +
> struct node_data {
> long key;
> long data;
> + struct plain_local __kptr * stashed_in_local_kptr;
> struct bpf_rb_node node;
> };
Everything looks correct and I applied the set.
The selftest sort-of covers the case where stashed_in_local_kptr
is being freed by rb_root recursive freeing,
but it doesn't really check for memory leaks.
It only checks that nothing will crash.
Please follow up with an improvement to selftest that
actually makes sure that recursive freeing of stashed kptr
correctly calls bpf_obj_free_fields->__bpf_obj_drop_impl.
The patches seem to do the right thing in terms of storing
correct btf/records in the right places,
but this is tricky, so extra tests are warranted.
