netdev - Re: [PATCH bpf-next 1/2] libbpf: Fix accessing first syscall argument on RV64

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

Message-ID: <f14d0337-0743-4490-88ba-f6e24f0e547e@huaweicloud.com>
Date: Sat, 31 Aug 2024 14:16:23 +0800
From: Pu Lehui <pulehui@...weicloud.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: bpf@...r.kernel.org, linux-riscv@...ts.infradead.org,
 netdev@...r.kernel.org, Björn Töpel <bjorn@...nel.org>,
 Ilya Leoshkevich <iii@...ux.ibm.com>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, 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>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Puranjay Mohan <puranjay@...nel.org>,
 Palmer Dabbelt <palmer@...belt.com>, Pu Lehui <pulehui@...wei.com>
Subject: Re: [PATCH bpf-next 1/2] libbpf: Fix accessing first syscall argument
 on RV64



On 2024/8/31 3:34, Andrii Nakryiko wrote:
> On Thu, Aug 29, 2024 at 6:32 AM Pu Lehui <pulehui@...weicloud.com> wrote:
>>
[SNIP]
>>
>> -#define __PT_PARM1_SYSCALL_REG __PT_PARM1_REG
>> +#define __PT_PARM1_SYSCALL_REG orig_a0
>>   #define __PT_PARM2_SYSCALL_REG __PT_PARM2_REG
>>   #define __PT_PARM3_SYSCALL_REG __PT_PARM3_REG
>>   #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
>>   #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
>>   #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
>> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
>> +#define PT_REGS_PARM1_CORE_SYSCALL(x) \
>> +       BPF_CORE_READ((const struct pt_regs___riscv *)(x), __PT_PARM1_SYSCALL_REG)
> 
> I feel like what we did for s390x is a bit suboptimal, so let's (try
> to) improve that and then do the same for RV64.
> 
> What I mean is that PT_REGS_PARMn_SYSCALL macros are used to read
> pt_regs coming directly from context, right? In that case we don't
> need to pay the price of BPF_CORE_READ(), we can just access memory
> directly (but we still need CO-RE relocation!).
> 
> So I think what we should do is
> 
> 1) mark pt_regs___riscv {} with __attribute__((preserve_access_index))
> so that normal field accesses are CO-RE-relocated
> 2) change PT_REGS_PARM1_SYSCALL(x) to be `((const
> struct_pt_regs___riscv *)(x))->orig_a0`, which will directly read
> memory
> 3) keep PT_REGS_PARM1_CORE_SYSCALL() as is
> 
> 
> But having written all the above, I'm not sure whether we allow CO-RE
> relocated direct context accesses (verifier might complain about
> modifying ctx register offset or something). So can you please check
> it either on s390 or RV64 and let me know? I'm not marking it as
> "Changes Requested" for that reason, because that might not work and
> we'll have to do BPF_CORE_READ().

Hi Andrii, thanks for your suggestion, it's really cool. I check that 
work for RV64, and send a new version:

https://lore.kernel.org/bpf/20240831041934.1629216-1-pulehui@huaweicloud.com

> 
>>
>>   #define __PT_RET_REG ra
>>   #define __PT_FP_REG s0
>> --
>> 2.34.1
>>