From: sdf@google.com
To: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
Akihiro HARAI <jharai0815@gmail.com>,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>, Hao Luo <haoluo@google.com>
Subject: Re: [PATCH] x86: Include asm/ptrace.h in syscall_wrapper header
Date: Tue, 18 Oct 2022 11:23:21 -0700 [thread overview]
Message-ID: <Y07vGVPIf913gND8@google.com> (raw)
In-Reply-To: <20221018122708.823792-1-jolsa@kernel.org>
On 10/18, Jiri Olsa wrote:
> From: Jiri Olsa <olsajiri@gmail.com>
> With just the forward declaration of the 'struct pt_regs' in
> syscall_wrapper.h, the syscall stub functions:
> __[x64|ia32]_sys_*(struct pt_regs *regs)
> will have different definition of 'regs' argument in BTF data
> based on which object file they are defined in.
> If the syscall's object includes 'struct pt_regs' definition,
> the BTF argument data will point to 'struct pt_regs' record,
> like:
> [226] STRUCT 'pt_regs' size=168 vlen=21
> 'r15' type_id=1 bits_offset=0
> 'r14' type_id=1 bits_offset=64
> 'r13' type_id=1 bits_offset=128
> ...
> If not, it will point to fwd declaration record:
> [15439] FWD 'pt_regs' fwd_kind=struct
> and make bpf tracing program hooking on those functions unable
> to access fields from 'struct pt_regs'.
Is the core issue here is that we can't / don't resolve FWD declarations
at load time (or dedup time)? I'm assuming 'struct pt_regs' is still
exposed somewhere in BTF via some other obj file, it's just those
syscall definitions that have FWD decl?
Applying this patch seems fine for now, but also seems fragile. Should
we instead/also teach verifier/dedup/whatever to resolve those FWD
declarations?
> Including asm/ptrace.h directly in syscall_wrapper.h to make
> sure all syscalls see 'struct pt_regs' definition and resulted
> BTF for '__*_sys_*(struct pt_regs *regs)' functions point to
> actual struct, not just forward declaration.
> Reported-by: Akihiro HARAI <jharai0815@gmail.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> arch/x86/include/asm/syscall_wrapper.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/arch/x86/include/asm/syscall_wrapper.h
> b/arch/x86/include/asm/syscall_wrapper.h
> index 59358d1bf880..fd2669b1cb2d 100644
> --- a/arch/x86/include/asm/syscall_wrapper.h
> +++ b/arch/x86/include/asm/syscall_wrapper.h
> @@ -6,7 +6,7 @@
> #ifndef _ASM_X86_SYSCALL_WRAPPER_H
> #define _ASM_X86_SYSCALL_WRAPPER_H
> -struct pt_regs;
> +#include <asm/ptrace.h>
> extern long __x64_sys_ni_syscall(const struct pt_regs *regs);
> extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
> --
> 2.37.3
next prev parent reply other threads:[~2022-10-18 18:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-18 12:27 [PATCH] x86: Include asm/ptrace.h in syscall_wrapper header Jiri Olsa
2022-10-18 18:23 ` sdf [this message]
2022-10-19 9:30 ` Jiri Olsa
2022-10-21 21:49 ` Andrii Nakryiko
2022-10-24 15:26 ` Lorenz Bauer
2022-10-24 17:10 ` [tip: x86/urgent] x86/syscall: " tip-bot2 for Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y07vGVPIf913gND8@google.com \
--to=sdf@google.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bp@alien8.de \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dave.hansen@linux.intel.com \
--cc=haoluo@google.com \
--cc=jharai0815@gmail.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=tglx@linutronix.de \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox