netdev - Re: [PATCH bpf-next v8 09/10] tools/bpf: add a test for bpf_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1bffa8bc-f652-c52c-ed5a-3eaa32887551@fb.com>
Date:   Sat, 28 Apr 2018 13:02:53 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Y Song <ys114321@...il.com>
CC:     Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        netdev <netdev@...r.kernel.org>, <kernel-team@...com>
Subject: Re: [PATCH bpf-next v8 09/10] tools/bpf: add a test for bpf_get_stack
 with raw tracepoint prog



On 4/28/18 12:06 PM, Alexei Starovoitov wrote:
> On Sat, Apr 28, 2018 at 11:17:30AM -0700, Y Song wrote:
>> On Sat, Apr 28, 2018 at 9:56 AM, Alexei Starovoitov
>> <alexei.starovoitov@...il.com> wrote:
>>> On Sat, Apr 28, 2018 at 12:02:04AM -0700, Yonghong Song wrote:
>>>> The test attached a raw_tracepoint program to sched/sched_switch.
>>>> It tested to get stack for user space, kernel space and user
>>>> space with build_id request. It also tested to get user
>>>> and kernel stack into the same buffer with back-to-back
>>>> bpf_get_stack helper calls.
>>>>
>>>> Whenever the kernel stack is available, the user space
>>>> application will check to ensure that the kernel function
>>>> for raw_tracepoint ___bpf_prog_run is part of the stack.
>>>>
>>>> Signed-off-by: Yonghong Song <yhs@...com>
>>> ...
>>>> +static int get_stack_print_output(void *data, int size)
>>>> +{
>>>> +     bool good_kern_stack = false, good_user_stack = false;
>>>> +     const char *expected_func = "___bpf_prog_run";
>>>
>>> so the test works with interpreter only?
>>> I guess that's ok for now, but needs to fixed for
>>> configs with CONFIG_BPF_JIT_ALWAYS_ON=y
>>
>> I did not test CONFIG_BPF_JIT_ALWAYS_ON=y.
>> I can have a followup patch for this if the patch set does not need respin.
> 
> I was thinking to apply the set and do the fix in the follow up,
> but testing it with jit_enable=1 I don't see it's failing,
> so something is wrong with the test.

Yes, it is because the return value test

if (CHECK(err < 0, "perf_event_poller", "err %d errno %d\n", err,
...

the "err < 0" is not right as all the return values are nonnegative.


> Also get_stack_raw_tp_action() keeps spawning new 'dd' in the background
> which is not killed after test stops.
> Please fix both issues in respin.

I will fix both and resend the patch.