On 3/2/23 6:29 PM, David Vernet wrote:
> On Thu, Mar 02, 2023 at 03:23:22PM -0800, Alexei Starovoitov wrote:
>> On Thu, Mar 2, 2023 at 3:19 PM Dave Marchevsky <davemarchevsky@xxxxxx> wrote:
>>>
>>> --- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
>>> +++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
>>> @@ -232,8 +232,9 @@ long rbtree_api_first_release_unlock_escape(void *ctx)
>>>
>>> bpf_spin_lock(&glock);
>>> res = bpf_rbtree_first(&groot);
>>> - if (res)
>>> - n = container_of(res, struct node_data, node);
>>> + if (!res)
>>> + return -1;
>>
>> The verifier cannot be ok with this return... I hope...
>
> This is a negative testcase which correctly fails, though the error
> message wasn't what I was expecting to see:
>
> __failure __msg("rbtree_remove node input must be non-owning ref")
>
> Something about the lock still being held seems much more intuitive.
>
It's necessary to call bpf_rbtree_remove w/ lock held. This test expects
to fail because non-owning ref "n" is clobbered after the critical
section where it's returned by bpf_rbtree_first ends.
>>
>>> + n = container_of(res, struct node_data, node);
>>> bpf_spin_unlock(&glock);
