netdev - Re: [PATCH] bpf: Allow small structs to be type of function argument

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: <4aec5fb8-9f9d-d01b-dd58-f15d50c5e827@fb.com>
Date:   Thu, 18 Jun 2020 22:39:16 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        John Fastabend <john.fastabend@...il.com>
CC:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Jiri Olsa <jolsa@...hat.com>, Andrii Nakryiko <andriin@...com>,
        Jiri Olsa <jolsa@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...hat.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        KP Singh <kpsingh@...omium.org>,
        Masanori Misono <m.misono760@...il.com>
Subject: Re: [PATCH] bpf: Allow small structs to be type of function argument



On 6/18/20 7:04 PM, Alexei Starovoitov wrote:
> On Thu, Jun 18, 2020 at 5:26 PM John Fastabend <john.fastabend@...il.com> wrote:
>>
>>   foo(int a, __int128 b)
>>
>> would put a in r0 and b in r2 and r3 leaving a hole in r1. But that
>> was some old reference manual and  might no longer be the case

This should not happen if clang compilation with -target bpf.
This MAY happen if they compile with 'clang -target riscv' as the IR
could change before coming to bpf backend.

>> in reality. Perhaps just spreading hearsay, but the point is we
>> should say something about what the BPF backend convention
>> is and write it down. We've started to bump into these things
>> lately.
> 
> calling convention for int128 in bpf is _undefined_.
> calling convention for struct by value in bpf is also _undefined_.

Just to clarify a little bit. bpf backend did not do anything
special about int128 and struct type. It is using llvm default.
That is, int128 using two argument registers and struct passed
by address. But I do see some other architectures having their
own ways to handle these parameters like X86, AARCH64, AMDGPU, MIPS.

int128 is not widely used. passing struct as the argument is not
a good practice. So Agree with Alexei is not really worthwhile to
handle them in the place of arguments.

> 
> In many cases the compiler has to have the backend code
> so other parts of the compiler can function.
> I didn't bother explicitly disabling every undefined case.
> Please don't read too much into llvm generated code.
>