Message-ID: <9bbfe425-1f2b-4846-a5a3-a31618742e9a@oracle.com>
Date: Tue, 24 Sep 2024 10:29:46 +0100
From: Alan Maguire <alan.maguire@...cle.com>
To: Howard Chu <howardchu95@...il.com>,
        Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc: James Clark <james.clark@...aro.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>, Jiri Olsa <jolsa@...nel.org>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-perf-users <linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH 1/1] perf build: Require at least clang 16.0.6 to build
 BPF skeletons

On 24/09/2024 03:00, Howard Chu wrote:
> Hello,
> 
> I got some good news:
> 
> Not being able to pass the BPF verifier is solely (or bi-solely)
> caused by these two functions:
> 
> SEC("tp/syscalls/sys_enter_rename")
> int sys_enter_rename(struct syscall_enter_args *args)
> 
> SEC("tp/syscalls/sys_enter_renameat2")
> int sys_enter_renameat2(struct syscall_enter_args *args)
> 
> The problem with it is double-string augmentation, but I haven't
> figured out how to solve it. Can you please take a look please,
> Arnaldo? Now I just commented these two BPF functions and perf trace
> runs no problem, built by clang that's older than clang 16, for
> example clang 15.0.7.
>

hi Howard,

I could be wrong - and I don't know this code at all - but I _think_ the
problem is here:

	oldpath_len = augmented_arg__read_str(&augmented_args->arg,
oldpath_arg, sizeof(augmented_args->arg.value));
        augmented_args->arg.size = PERF_ALIGN(oldpath_len + 1, sizeof(u64));
        len += augmented_args->arg.size;

        struct augmented_arg *arg2 = (void *)&augmented_args->arg.value
+ augmented_args->arg.size;

        newpath_len = augmented_arg__read_str(arg2, newpath_arg,
sizeof(augmented_args->arg.value));


Notice that these strings share the augmented_args->arg.value for
storage, but in the second case (reading arg2) we have added
augmented_args->arg.size to the offset where we store the second string.
However at the same time we have also told augmented_arg__read_str()
that it has

sizeof(augmented_args->arg.value)


...to work with as arg_len, where in reality we should have specified

sizeof(augmented_args->arg.value) - augmented_args->arg.size

...and ensured that after doing that subtraction, there is still space
to work with (i.e. the above is > 0).

I _think_ the verifier is likely complaining that we can write past the
end of augmented_args->arg.value , does that sound right?

Fixing the arg_len parameter for cases where we record more than one
augmented string by reducing arg_len available should solve this if so.
Hope this helps,

Alan

> perf $ clang -v
> Ubuntu clang version 15.0.7
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> InstalledDir: /bin
> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> Candidate multilib: .;@m64
> Selected multilib: .;@m64
> perf $ ./perf trace -e write --max-events=1
>      0.000 ( 0.008 ms): gmain/2173 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
> count: 8)                         =
> 
> But plot twist, it won't build on clang-14
> 
> perf $ clang -v
> Ubuntu clang version 14.0.6
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> InstalledDir: /bin
> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> Candidate multilib: .;@m64
> Selected multilib: .;@m64
> perf $ ./perf trace -e write --max-events=1
> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
> 0: R1=ctx() R10=fp0
> ; int sys_enter(struct syscall_enter_args *args) @
> augmented_raw_syscalls.bpf.c:509
> 0: (bf) r7 = r1                       ; R1=ctx() R7_w=ctx()
> 
> This time the problem is my code in BTF general collector:
> 
> ; payload_offset += written; @ augmented_raw_syscalls.bpf.c:497
> 
> I'll resolve this.
> 
> Thanks,
> Howard
> 
> On Wed, Sep 18, 2024 at 8:13 PM Howard Chu <howardchu95@...il.com> wrote:
>>
>> Hello,
>>
>> I'm able to reproduce it on Ubuntu 24.
>>
>> perf $ cat /etc/os-release
>> PRETTY_NAME="Ubuntu 24.04.1 LTS"
>> NAME="Ubuntu"
>> VERSION_ID="24.04"
>> VERSION="24.04.1 LTS (Noble Numbat)"
>> VERSION_CODENAME=noble
>> ID=ubuntu
>> ID_LIKE=debian
>> HOME_URL="https://www.ubuntu.com/"
>> SUPPORT_URL="https://help.ubuntu.com/"
>> BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
>> PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
>> UBUNTU_CODENAME=noble
>> LOGO=ubuntu-logo
>>
>> perf $ uname -r
>> 6.11.0-061100-generic
>>
>> perf $ git log
>> commit 4c1af9bf97eb56d069421c3233ce61608458d5c8 (HEAD)
>> Author: Arnaldo Carvalho de Melo <acme@...hat.com>
>> Date:   Tue Sep 10 13:54:23 2024 -0300
>>
>>     perf trace: If a syscall arg is marked as 'const', assume it is
>> coming _from_ userspace
>>
>>
>> perf $ clang -v
>> Ubuntu clang version 15.0.7
>> Target: x86_64-pc-linux-gnu
>> Thread model: posix
>> InstalledDir: /bin
>> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
>> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
>> Candidate multilib: .;@m64
>> Selected multilib: .;@m64
>>
>> perf $ ./perf trace -e write --max-events=1
>> libbpf: prog 'sys_enter_rename': BPF program load failed: Permission denied
>> libbpf: prog 'sys_enter_rename': -- BEGIN PROG LOAD LOG --
>> 0: R1=ctx() R10=fp0
>> ; int sys_enter_rename(struct syscall_enter_args *args) @
>> augmented_raw_syscalls.bpf.c:275
>>
>>
>> With clang-16:
>>
>> perf $ clang -v
>> Ubuntu clang version 16.0.6 (23ubuntu4)
>> Target: x86_64-pc-linux-gnu
>> Thread model: posix
>> InstalledDir: /bin
>> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
>> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
>> Candidate multilib: .;@m64
>> Selected multilib: .;@m64
>>
>> perf $ ./perf trace -e write --max-events=1
>>      0.000 ( 0.021 ms): sudo/4741 write(fd: 8, buf:
>> \27[?25l\27[37m\27[48;5;96m\27[H[trace] , count: 205) =
>>
>>> I'll try to build it and see. But Ubuntu 22 only has clang 11 to 15 so
>>> making 16 the minimum could be an issue.
>>
>> Yes, I think disabling any distro that doesn't come with clang 16
>> (released in 2023) is not a good idea. I'll try to tame the BPF
>> verifier.
>>
>> Thanks,
>> Howard
>>
>> On Tue, Sep 17, 2024 at 10:37 AM Howard Chu <howardchu95@...il.com> wrote:
>>>
>>> Hello James and Arnaldo,
>>>
>>> On Fri, Sep 13, 2024 at 3:42 AM Arnaldo Carvalho de Melo
>>> <arnaldo.melo@...il.com> wrote:
>>>>
>>>>
>>>>
>>>> On Fri, Sep 13, 2024, 7:20 AM James Clark <james.clark@...aro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 12/09/2024 15:50, Arnaldo Carvalho de Melo wrote:
>>>>>> On Thu, Sep 12, 2024 at 03:40:32PM +0100, James Clark wrote:
>>>>>>> On 11/09/2024 13:24, Arnaldo Carvalho de Melo wrote:
>>>>>>>> Howard reported problems using perf features that use BPF:
>>>>>>
>>>>>>>>     perf $ clang -v
>>>>>>>>     Debian clang version 15.0.6
>>>>>>>>     Target: x86_64-pc-linux-gnu
>>>>>>>>     Thread model: posix
>>>>>>>>     InstalledDir: /bin
>>>>>>>>     Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
>>>>>>>>     Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
>>>>>>>>     Candidate multilib: .;@m64
>>>>>>>>     Selected multilib: .;@m64
>>>>>>>>     perf $ ./perf trace -e write --max-events=1
>>>>>>>>     libbpf: prog 'sys_enter_rename': BPF program load failed: Permission denied
>>>>>>>>     libbpf: prog 'sys_enter_rename': -- BEGIN PROG LOAD LOG --
>>>>>>>>     0: R1=ctx() R10=fp0
>>>>>>>>
>>>>>>>> But it works with:
>>>>>>>>
>>>>>>>>     perf $ clang -v
>>>>>>>>     Debian clang version 16.0.6 (15~deb12u1)
>>>>>>>>     Target: x86_64-pc-linux-gnu
>>>>>>>>     Thread model: posix
>>>>>>>>     InstalledDir: /bin
>>>>>>>>     Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
>>>>>>>>     Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
>>>>>>>>     Candidate multilib: .;@m64
>>>>>>>>     Selected multilib: .;@m64
>>>>>>>>     perf $ ./perf trace -e write --max-events=1
>>>>>>>>          0.000 ( 0.009 ms): gmain/1448 write(fd: 4, buf: \1\0\0\0\0\0\0\0, count: 8)                         = 8 (kworker/0:0-eve)
>>>>>>>>     perf $
>>>>>>>>
>>>>>>>> So lets make that the required version, if you happen to have a slightly
>>>>>>>> older version where this work, please report so that we can adjust the
>>>>>>>> minimum required version.
>>>>>>
>>>>>>> I wasn't able to reproduce the issue with either of these versions. But I
>>>>>>> suppose it could be an issue with only 15.0.6.
>>>>>>
>>>>>> Interesting, that complicates things, probably the best way then is to
>>>>>> try to build it, if it fails, mention that 15.0.6 is known to be
>>>>>> problematic and suggest working versions?
>>>>>>
>>>>>> - Arnaldo
>>>>>
>>>>> I still wasn't able to reproduce it with 15.0.6. And I double checked
>>>>> with V=1 that the build was using the right clang. I suppose it could be
>>>>> a build configuration issue, or maybe with a different kernel version?
>>>>
>>>>
>>>> Howard? If not reproducible we can revert it.
>>>>
>>>> - Arnaldo
>>>>
>>>>
>>>>>
>>>>> $  uname --kernel-release
>>>>> 6.8.0-76060800daily20240311-generic
>>>>>
>>>>> $ ../../llvm-project/build/bin/clang -v
>>>>> clang version 15.0.6 (https://github.com/llvm/llvm-project.git
>>>>> 088f33605d8a61ff519c580a71b1dd57d16a03f8)
>>>>> Target: x86_64-unknown-linux-gnu
>>>>> Thread model: posix
>>>>> InstalledDir: /home/james/workspace/linux/linux/../../llvm-project/build/bin
>>>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
>>>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/12
>>>>> Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/12
>>>>> Candidate multilib: .;@m64
>>>>> Selected multilib: .;@m64
>>>>>
>>>>> $ git log
>>>>> commit 003265bb6f028d7bcd7cbd92d6ba2b4e26382796
>>>>> (perf-tools-next/perf-tools-next)
>>>>>
>>>>> $ make O=../build/local/ CLANG=../../llvm-project/build/bin/clang -C \
>>>>>    tools/perf
>>>>>
>>>>> $ perf trace -e write --max-events=1
>>>>>
>>>>>       0.000 ( 0.026 ms): gnome-shell/5454 write(fd: 5, buf:
>>>>>         0x7fffa102d9b0, count: 8)                           = 8
>>>
>>> Thanks for the effort! But this output seems to be unaugmented? I
>>> think you are testing the perf/perf-tools-next branch that's slightly
>>> older (perf mem: Fix the wrong reference in parse_record_events()
>>> 003265bb6f028d7bcd7cbd92d6ba2b4e26382796), and I think that doesn't
>>> have the new perf trace feature yet...
>>>
>>> That's why the perf trace output looks like:
>>>
>>>>> $ perf trace -e write --max-events=1
>>>>>
>>>>>       0.000 ( 0.026 ms): gnome-shell/5454 write(fd: 5, buf:
>>>>>         0x7fffa102d9b0, count: 8)
>>>
>>> With the new buffer augmentation it should look like this:
>>>
>>> perf $ ./perf trace -e write --max-events=1
>>>      0.000 ( 0.011 ms): gmain/1408 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
>>> count: 8)                         =
>>>
>>> The new perf trace can be found on
>>> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
>>> perf-tools-next
>>>
>>> perf $ git log
>>> commit 1de5b5dcb8353f36581c963df2d359a5f151a0be (HEAD ->
>>> struct-aug-arnaldo-2, perf/perf-tools-next)
>>>
>>> Also, if you want to test the clang, you may want to checkout to the
>>> commit before the clang-16 guard (otherwise you can't test different
>>> clang version).
>>>
>>> perf $ git log --oneline
>>> 1de5b5dcb835 (HEAD -> struct-aug-arnaldo-2, perf/perf-tools-next) perf
>>> trace: Mark the 'head' arg in the set_robust_list syscall as coming
>>> from user space
>>> 0c1019e3463b perf trace: Mark the 'rseq' arg in the rseq syscall as
>>> coming from user space
>>> edf3ce0ed38e perf env: Find correct branch counter info on hybrid
>>> 9953807c9e01 perf evlist: Print hint for group
>>> eb9b9a6f5ab3 tools: Drop nonsensical -O6
>>> 89c0a55e550e perf pmu: To info add event_type_desc
>>> f08cc258431d perf evsel: Add accessor for tool_event
>>> 925320737ae2 perf pmus: Fake PMU clean up
>>> d3d5c1a00fcd perf list: Avoid potential out of bounds memory read
>>> 4ae354d73a8e perf help: Fix a typo ("bellow")
>>> 74298dd8acb8 perf ftrace: Detect whether ftrace is enabled on system
>>> 83420d5f5863 perf test shell probe_vfs_getname: Remove extraneous '='
>>> from probe line number regex
>>> 9327f0ecad48 perf build: Require at least clang 16.0.6 to build BPF
>>> skeletons. **********[[[[[[[[[[[[!!!!(This is the clang
>>> guard)]]]]]]]]]]*****
>>> 4c1af9bf97eb perf trace: If a syscall arg is marked as 'const', assume
>>> it is coming _from_ userspace
>>> e37b315c17df perf parse-events: Remove duplicated include in parse-events.c
>>>
>>> My machine is debian 12, and I cannot build perf trace with the new
>>> augmentation feature on clang-13, clang-14, and clang-15.
>>>
>>> perf $ uname -r
>>> 6.11.0-061100rc7-generic
>>>
>>> perf $ git log
>>> commit 4c1af9bf97eb56d069421c3233ce61608458d5c8 (HEAD)
>>>
>>> perf $ make CLANG=/bin/clang-13 && ./perf trace -e write --max-events=1
>>> ...
>>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
>>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
>>> 0: R1=ctx() R10=fp0
>>> ; int sys_enter(struct syscall_enter_args *args) @
>>> augmented_raw_syscalls.bpf.c:509
>>> 0: (bf) r7 = r1                       ; R1=ctx() R7_w=ctx()
>>>
>>>
>>> perf $ make CLANG=/bin/clang-14 && ./perf trace -e write --max-events=1
>>> ...
>>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
>>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
>>> 0: R1=ctx() R10=fp0
>>> ; int sys_enter(struct syscall_enter_args *args) @
>>> augmented_raw_syscalls.bpf.c:509
>>>
>>>
>>> perf $ make CLANG=/bin/clang-15 && ./perf trace -e write --max-events=1
>>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
>>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
>>> 0: R1=ctx() R10=fp0
>>>
>>>
>>> perf $ make CLANG=/bin/clang-16 && ./perf trace -e write --max-events=1
>>>      0.000 ( 0.010 ms): gmain/1408 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
>>> count: 8)                         =
>>>
>>>
>>> Anyway thank you so much, I'll try to reproduce it on Ubuntu, like you
>>> mentioned.
>>>
>>> Thanks,
>>> Howard
>>>
>>>
>>>>>

Powered by blists - more mailing lists