netdev - Re: [PATCH bpf-next] bpf: fix missing * in bpf.h

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b64fa932-5902-f13f-b3b9-f476e389db1b@isovalent.com>
Date:   Fri, 26 Feb 2021 16:50:42 +0000
From:   Quentin Monnet <quentin@...valent.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Hangbin Liu <liuhangbin@...il.com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Joe Stringer <joe@...d.net.nz>
Subject: Re: [PATCH bpf-next] bpf: fix missing * in bpf.h

2021-02-24 10:59 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@...il.com>
> On Wed, Feb 24, 2021 at 7:55 AM Daniel Borkmann <daniel@...earbox.net> wrote:
>>
>> On 2/23/21 3:43 PM, Jesper Dangaard Brouer wrote:
>>> On Tue, 23 Feb 2021 20:45:54 +0800
>>> Hangbin Liu <liuhangbin@...il.com> wrote:
>>>
>>>> Commit 34b2021cc616 ("bpf: Add BPF-helper for MTU checking") lost a *
>>>> in bpf.h. This will make bpf_helpers_doc.py stop building
>>>> bpf_helper_defs.h immediately after bpf_check_mtu, which will affect
>>>> future add functions.
>>>>
>>>> Fixes: 34b2021cc616 ("bpf: Add BPF-helper for MTU checking")
>>>> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
>>>> ---
>>>>   include/uapi/linux/bpf.h       | 2 +-
>>>>   tools/include/uapi/linux/bpf.h | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> Thanks for fixing that!
>>>
>>> Acked-by: Jesper Dangaard Brouer <brouer@...hat.com>
>>
>> Thanks guys, applied!
>>
>>> I though I had already fix that, but I must have missed or reintroduced
>>> this, when I rolling back broken ideas in V13.
>>>
>>> I usually run this command to check the man-page (before submitting):
>>>
>>>   ./scripts/bpf_helpers_doc.py | rst2man | man -l -
>>
>> [+ Andrii] maybe this could be included to run as part of CI to catch such
>> things in advance?
> 
> We do something like that as part of bpftool build, so there is no
> reason we can't add this to selftests/bpf/Makefile as well.

Hi, pretty sure this is the case already? [0]

This helps catching RST formatting issues, for example if a description
is using invalid markup, and reported by rst2man. My understanding is
that in the current case, the missing star simply ends the block for the
helpers documentation from the parser point of view, it's not considered
an error.

I see two possible workarounds:

1) Check that the number of helpers found ("len(self.helpers)") is equal
to the number of helpers in the file, but that requires knowing how many
helpers we have in the first place (e.g. parsing "__BPF_FUNC_MAPPER(FN)").

2) Add some ending tag to the documentation block, and make sure we
eventually reach it. This is probably a much simpler solution. I could
work on this (or sync with Joe (+Cc) who is also working on these bits
for documenting the bpf() syscall).

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/Makefile?h=v5.11#n189

Quentin