| 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
| ||
|
Message-ID: <m21ptcmdnw.fsf@gmail.com>
Date: Mon, 28 Apr 2025 09:52:35 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: Alexis Lothoré <alexis.lothore@...tlin.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>, "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>, "Xu Kuohai"
<xukuohai@...weicloud.com>, "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 3/4] bpf/selftests: add tests to validate
proper arguments alignment on ARM64
Alexis Lothoré <alexis.lothore@...tlin.com> writes:
[...]
>> The function listened to is defined as accepting 'struct bpf_testmod_struct_arg_7',
>> at the same time this function uses 'struct bpf_testmod_struct_arg_5'.
>
> That's not an accidental mistake, those are in fact the same definition.
> bpf_testmod_struct_arg_7 is the kernel side definition in bpf_testmod.c:
>
> struct bpf_testmod_struct_arg_7 {
> __int128 a;
> };
>
> and struct bpf_testmode_struct_arg_5 is the one defined in the bpf test
> program:
>
> struct bpf_testmod_struct_arg_5 {
> __int128 a;
> };
Apologies, but I'm still confused:
- I apply this series on top of:
224ee86639f5 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf after rc4")
- line 12 of tracing_struct_many_args.c has the following definition:
struct bpf_testmod_struct_arg_5 {
char a;
short b;
int c;
long d;
};
- line 135 of the same file has the following definition:
SEC("fentry/bpf_testmod_test_struct_arg_11")
int BPF_PROG2(test_struct_many_args_9, struct bpf_testmod_struct_arg_5, a,
struct bpf_testmod_struct_arg_5, b,
struct bpf_testmod_struct_arg_5, c,
struct bpf_testmod_struct_arg_5, d, int, e,
struct bpf_testmod_struct_arg_5, f)
- line 70 of tools/testing/selftests/bpf/test_kmods/bpf_testmod.c:
struct bpf_testmod_struct_arg_7 {
__int128 a;
};
- line 152 of the same file:
noinline int bpf_testmod_test_struct_arg_11(struct bpf_testmod_struct_arg_7 a,
struct bpf_testmod_struct_arg_7 b,
struct bpf_testmod_struct_arg_7 c,
struct bpf_testmod_struct_arg_7 d,
short e,
struct bpf_testmod_struct_arg_7 f)
Do I use a wrong base to apply the series?
[...]
>> Nevertheless, the assertion persists even with correct types.
>
> So I digged a bit further to better share my observations here. This is the
> function stack when entering the trampoline after having triggered the
> target function execution:
>
> (gdb) x/64b $rbp+0x18
> 0xffffc9000015fd60: 41 0 0 0 0 0 0 0
> 0xffffc9000015fd68: 0 0 0 0 0 0 0 0
> 0xffffc9000015fd70: 42 0 0 0 0 0 0 0
> 0xffffc9000015fd78: 35 0 0 0 0 0 0 0
> 0xffffc9000015fd80: 43 0 0 0 0 0 0 0
> 0xffffc9000015fd88: 0 0 0 0 0 0 0 0
>
> We see the arguments that did not fit in registers, so d, e and f.
>
> This is the ebpf context generated by the trampoline for the fentry
> program, from the content of the stack above + the registers:
>
> (gdb) x/128b $rbp-60
> 0xffffc9000015fce8: 38 0 0 0 0 0 0 0
> 0xffffc9000015fcf0: 0 0 0 0 0 0 0 0
> 0xffffc9000015fcf8: 39 0 0 0 0 0 0 0
> 0xffffc9000015fd00: 0 0 0 0 0 0 0 0
> 0xffffc9000015fd08: 40 0 0 0 0 0 0 0
> 0xffffc9000015fd10: 0 0 0 0 0 0 0 0
> 0xffffc9000015fd18: 41 0 0 0 0 0 0 0
> 0xffffc9000015fd20: 0 0 0 0 0 0 0 0
> 0xffffc9000015fd28: 42 0 0 0 0 0 0 0
> 0xffffc9000015fd30: 35 0 0 0 0 0 0 0
> 0xffffc9000015fd38: 43 0 0 0 0 0 0 0
> 0xffffc9000015fd40: 37 0 0 0 0 0 0 0
>
> So IIUC, this is wrong because the "e" variable in the bpf program being
> an int (and about to receive value 42), it occupies only 1 "tracing context
> 8-byte slot", so the value 43 (representing the content for variable f),
> should be right after it, at 0xffffc9000015fd30. What we have instead is a
> hole, very likely because we copied silently the alignment from the
> original function call (and I guess this 35 value is a remnant from the
> previous test, which uses values from 27 to 37)
Interesting, thank you for the print outs.
> Regardless of this issue, based on discussion from last week, I think I'll
> go for the implementation suggested by Alexei: handling the nominal cases,
> and detecting and blocking the non trivial cases (eg: structs passed on
> stack). It sounds reasonable as there seems to be no exisiting kernel
> function currently able to trigger those very specific cases, so it could
> be added later if this changes.
Yes, this makes sense.
Powered by blists - more mailing lists