linux-kernel - Re: [PATCH RFC bpf-next 1/4] bpf: add struct largest member size in func model

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: <6b6472c3-0718-4e60-9972-c166d51962a3@huaweicloud.com>
Date: Thu, 24 Apr 2025 20:00:30 +0800
From: Xu Kuohai <xukuohai@...weicloud.com>
To: Alexis Lothoré <alexis.lothore@...tlin.com>,
 Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Puranjay Mohan <puranjay@...nel.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Florent Revest <revest@...omium.org>,
 Bastien Curutchet <bastien.curutchet@...tlin.com>, ebpf@...uxfoundation.org,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, bpf@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kselftest@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH RFC bpf-next 1/4] bpf: add struct largest member size in
 func model

On 4/24/2025 3:24 AM, Alexis Lothoré wrote:
> Hi Andrii,
> 
> On Wed Apr 23, 2025 at 7:15 PM CEST, Andrii Nakryiko wrote:
>> On Thu, Apr 17, 2025 at 12:14 AM Alexis Lothoré
>> <alexis.lothore@...tlin.com> wrote:
>>>
>>> Hi Andrii,
>>>
>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
>>>> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
>>>> <alexis.lothore@...tlin.com> wrote:
> 
> [...]
> 
>>> Indeed I initially checked whether I could return directly some alignment
>>> info from btf, but it then involves the alignment computation in the btf
>>> module. Since there could be minor differences between architectures about
>>> alignment requirements, I though it would be better to in fact keep alignment
>>> computation out of the btf module. For example, I see that 128 bits values
>>> are aligned on 16 bytes on ARM64, while being aligned on 8 bytes on S390.
>>>
>>> And since for ARM64, all needed alignments are somehow derived from size
>>> (it is either directly size for fundamental types, or alignment of the
>>> largest member for structs, which is then size of largest member),
>>> returning the size seems to be enough to allow the JIT side to compute
>>> alignments.
>>
>> If you mean the size of "primitive" field and/or array element
>> (applied recursively for all embedded structs/unions) then yes, that's
>> close enough. But saying just "largest struct member" is wrong,
>> because for
>>
>> struct blah {
>>      struct {
>>          int whatever[128];
>>      } heya;
>> };
>>
>>
>> blah.heya has a large size, but alignment is still just 4 bytes.
> 
> Indeed, that's another case making my proposal fail :)
> 
>> I'd suggest looking at btf__align_of() in libbpf (tools/lib/bpf/btf.c)
>> to see how we calculate alignment there. It seems to work decently
>> enough. It won't cover any arch-specific extra rules like double
>> needing 16-byte alignment (I vaguely remember something like that for
>> some architectures, but I might be misremembering), or anything
>> similar. It also won't detect (I don't think it's possible without
>> DWARF) artificially increased alignment with attribute((aligned(N))).
> 
> Thanks for the pointer, I'll take a look at it. The more we discuss this
> series, the less member size sounds relevant for what I'm trying to achieve
> here.
> 
> Following Xu's comments, I have been thinking about how I could detect the
> custom alignments and packing on structures, and I was wondering if I could
> somehow benefit from __attribute__ encoding in BTF info ([1]). But
> following your hint, I also see some btf_is_struct_packed() in
> tools/lib/bpf/btf_dump.c that could help. I'll dig this further and see if
> I can manage to make something work with all of this.
>

With DWARF info, we might not need to detect the structure alignment anymore,
since the DW_AT_location attribute tells us where the structure parameter is
located on the stack, and DW_AT_byte_size gives us the size of the structure.

> Thanks,
> 
> Alexis
> 
> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
>