| Message ID | 20220204041955.1958263-3-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: Fix accessing syscall arguments | expand |
On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > user_pt_regs is used by eBPF in order to access userspace registers - > see commit 466698e654e8 ("s390/bpf: correct broken uapi for > BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first > syscall argument from eBPF programs, we need to export orig_gpr2. > > args member is not in use since commit 56e62a737028 ("s390: convert to > generic entry"), so move orig_gpr2 in its place. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > arch/s390/include/asm/ptrace.h | 3 +-- > arch/s390/include/uapi/asm/ptrace.h | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h > index 4ffa8e7f0ed3..0278bacd61be 100644 > --- a/arch/s390/include/asm/ptrace.h > +++ b/arch/s390/include/asm/ptrace.h > @@ -80,12 +80,11 @@ struct pt_regs { > union { > user_pt_regs user_regs; > struct { > - unsigned long args[1]; > + unsigned long orig_gpr2; > psw_t psw; > unsigned long gprs[NUM_GPRS]; > }; > }; > - unsigned long orig_gpr2; Please don't change the physical location of this field, it effectively breaks libbpf's syscall tracing macro on all older kernels. Let's do what you did in the previous revision and just expose the field at its correct offset. That way with up to date UAPI header or vmlinux.h all this will work even on old kernels (even without CO-RE). > union { > struct { > unsigned int int_code; > diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h > index ad64d673b5e6..d0cc737b8151 100644 > --- a/arch/s390/include/uapi/asm/ptrace.h > +++ b/arch/s390/include/uapi/asm/ptrace.h > @@ -292,7 +292,7 @@ typedef struct { > * the in-kernel pt_regs structure to user space. > */ > typedef struct { > - unsigned long args[1]; > + unsigned long orig_gpr2; > psw_t psw; > unsigned long gprs[NUM_GPRS]; > } user_pt_regs; > -- > 2.34.1 >
On Thu, Feb 03, 2022 at 09:19:42PM -0800, Andrii Nakryiko wrote: > On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > > > user_pt_regs is used by eBPF in order to access userspace registers - > > see commit 466698e654e8 ("s390/bpf: correct broken uapi for > > BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first > > syscall argument from eBPF programs, we need to export orig_gpr2. > > > > args member is not in use since commit 56e62a737028 ("s390: convert to > > generic entry"), so move orig_gpr2 in its place. > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > arch/s390/include/asm/ptrace.h | 3 +-- > > arch/s390/include/uapi/asm/ptrace.h | 2 +- > > 2 files changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h > > index 4ffa8e7f0ed3..0278bacd61be 100644 > > --- a/arch/s390/include/asm/ptrace.h > > +++ b/arch/s390/include/asm/ptrace.h > > @@ -80,12 +80,11 @@ struct pt_regs { > > union { > > user_pt_regs user_regs; > > struct { > > - unsigned long args[1]; > > + unsigned long orig_gpr2; > > psw_t psw; > > unsigned long gprs[NUM_GPRS]; > > }; > > }; > > - unsigned long orig_gpr2; > > Please don't change the physical location of this field, it > effectively breaks libbpf's syscall tracing macro on all older > kernels. Let's do what you did in the previous revision and just > expose the field at its correct offset. That way with up to date UAPI > header or vmlinux.h all this will work even on old kernels (even > without CO-RE). That's (unfortunately) a valid argument. So looks like we can't get rid of the args member now. Maybe we can put something else there or simply rename it to "dontuse". Anyway, that's not within the scope of this patch series.
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h index 4ffa8e7f0ed3..0278bacd61be 100644 --- a/arch/s390/include/asm/ptrace.h +++ b/arch/s390/include/asm/ptrace.h @@ -80,12 +80,11 @@ struct pt_regs { union { user_pt_regs user_regs; struct { - unsigned long args[1]; + unsigned long orig_gpr2; psw_t psw; unsigned long gprs[NUM_GPRS]; }; }; - unsigned long orig_gpr2; union { struct { unsigned int int_code; diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h index ad64d673b5e6..d0cc737b8151 100644 --- a/arch/s390/include/uapi/asm/ptrace.h +++ b/arch/s390/include/uapi/asm/ptrace.h @@ -292,7 +292,7 @@ typedef struct { * the in-kernel pt_regs structure to user space. */ typedef struct { - unsigned long args[1]; + unsigned long orig_gpr2; psw_t psw; unsigned long gprs[NUM_GPRS]; } user_pt_regs;
user_pt_regs is used by eBPF in order to access userspace registers - see commit 466698e654e8 ("s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first syscall argument from eBPF programs, we need to export orig_gpr2. args member is not in use since commit 56e62a737028 ("s390: convert to generic entry"), so move orig_gpr2 in its place. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- arch/s390/include/asm/ptrace.h | 3 +-- arch/s390/include/uapi/asm/ptrace.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-)