| 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
| ||
|
Date: Sun, 3 Dec 2017 09:51:10 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Hendrik Brueckner <brueckner@...ux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin Schwidefsky <schwidefsky@...ibm.com>,
Will Deacon <will.deacon@....com>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
linux-s390@...r.kernel.org, netdev@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 1/5] bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type
On Fri, Dec 01, 2017 at 03:19:04PM +0100, Hendrik Brueckner wrote:
> Commit 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT
> program type") introduced the bpf_perf_event_data structure which
> exports the pt_regs structure. This is OK for multiple architectures
> but fail for s390 and arm64 which do not export pt_regs. Programs
> using them, for example, the bpf selftest fail to compile on these
> architectures.
>
> For s390, exporting the pt_regs is not an option because s390 wants
> to allow changes to it. For arm64, there is a user_pt_regs structure
> that covers parts of the pt_regs structure for use by user space.
>
> To solve the broken uapi for s390 and arm64, introduce an abstract
> type for pt_regs and add an asm/bpf_perf_event.h file that concretes
> the type. An asm-generic header file covers the architectures that
> export pt_regs today.
>
> The arch-specific enablement for s390 and arm64 follows in separate
> commits.
>
> Reported-by: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> Fixes: 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type")
> Signed-off-by: Hendrik Brueckner <brueckner@...ux.vnet.ibm.com>
> Reviewed-and-tested-by: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> ---
> include/linux/perf_event.h | 6 +++++-
> include/uapi/asm-generic/bpf_perf_event.h | 9 +++++++++
> include/uapi/linux/bpf_perf_event.h | 5 ++---
> kernel/events/core.c | 2 +-
> 4 files changed, 17 insertions(+), 5 deletions(-)
> create mode 100644 include/uapi/asm-generic/bpf_perf_event.h
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 2c9c87d..7546822 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -15,6 +15,7 @@
> #define _LINUX_PERF_EVENT_H
>
> #include <uapi/linux/perf_event.h>
> +#include <uapi/linux/bpf_perf_event.h>
>
> /*
> * Kernel-internal data types and definitions:
> @@ -787,7 +788,7 @@ struct perf_output_handle {
> };
>
> struct bpf_perf_event_data_kern {
> - struct pt_regs *regs;
> + bpf_user_pt_regs_t *regs;
> struct perf_sample_data *data;
> struct perf_event *event;
> };
> @@ -1177,6 +1178,9 @@ extern void perf_tp_event(u16 event_type, u64 count, void *record,
> (user_mode(regs) ? PERF_RECORD_MISC_USER : PERF_RECORD_MISC_KERNEL)
> # define perf_instruction_pointer(regs) instruction_pointer(regs)
> #endif
> +#ifndef perf_arch_bpf_user_pt_regs
> +# define perf_arch_bpf_user_pt_regs(regs) regs
> +#endif
>
> static inline bool has_branch_stack(struct perf_event *event)
> {
> diff --git a/include/uapi/asm-generic/bpf_perf_event.h b/include/uapi/asm-generic/bpf_perf_event.h
> new file mode 100644
> index 0000000..53815d2
> --- /dev/null
> +++ b/include/uapi/asm-generic/bpf_perf_event.h
> @@ -0,0 +1,9 @@
> +#ifndef _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
> +#define _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
> +
> +#include <linux/ptrace.h>
> +
> +/* Export kernel pt_regs structure */
> +typedef struct pt_regs bpf_user_pt_regs_t;
> +
> +#endif /* _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__ */
> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h
> index af549d4..8f95303 100644
> --- a/include/uapi/linux/bpf_perf_event.h
> +++ b/include/uapi/linux/bpf_perf_event.h
> @@ -8,11 +8,10 @@
> #ifndef _UAPI__LINUX_BPF_PERF_EVENT_H__
> #define _UAPI__LINUX_BPF_PERF_EVENT_H__
>
> -#include <linux/types.h>
> -#include <linux/ptrace.h>
> +#include <asm/bpf_perf_event.h>
>
> struct bpf_perf_event_data {
> - struct pt_regs regs;
> + bpf_user_pt_regs_t regs;
> __u64 sample_period;
> };
Thank you for working on this problem.
The fix looks great to me.
While applying it I noticed few nits:
Applying: selftests/bpf: sync kernel headers and introduce arch support in Makefile
/w/bpf/.git/rebase-apply/patch:253: trailing whitespace.
freg_t fprs[NUM_FPRS];
/w/bpf/.git/rebase-apply/patch:262: trailing whitespace.
typedef struct
/w/bpf/.git/rebase-apply/patch:439: trailing whitespace.
} lowcore;
/w/bpf/.git/rebase-apply/patch:490: trailing whitespace.
} ptprot_area;
warning: 4 lines add whitespace errors.
Could you please fix those and resubmit ?
With that fixed feel free to add my
Acked-by: Alexei Starovoitov <ast@...nel.org>
to the patches.
I've tested it on arm64 and don't see any issues.
When resubmitting could you please reduce cc-list, since this set
went into spam folder for me and I noticed it only in patchworks.
Thanks
Powered by blists - more mailing lists