| 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: Tue, 28 Jun 2022 12:11:47 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jiri Olsa <olsajiri@...il.com>
Cc: Ian Rogers <irogers@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Namhyung Kim <namhyung@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Dave Marchevsky <davemarchevsky@...com>,
Quentin Monnet <quentin@...valent.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, bpf@...r.kernel.org,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data
Em Tue, Jun 28, 2022 at 10:14:52AM +0200, Jiri Olsa escreveu:
> On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> > bpil data is accessed assuming 64-bit alignment resulting in undefined
> > behavior as the data is just byte aligned. With an -fsanitize=undefined
> > build the following errors are observed:
> >
> > $ sudo perf record -a sleep 1
> > util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> > 0x55f61084520f: note: pointer points here
> > a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> > ^
> > util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> > 0x55f61084522f: note: pointer points here
> > ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> > ^
> > util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> > 0x55f61084523f: note: pointer points here
> > 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
> >
> > Correct this by rouding up the data sizes and aligning the pointers.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/util/bpf-utils.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> > index e271e05e51bc..80b1d2b3729b 100644
> > --- a/tools/perf/util/bpf-utils.c
> > +++ b/tools/perf/util/bpf-utils.c
> > @@ -149,11 +149,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> > count = bpf_prog_info_read_offset_u32(&info, desc->count_offset);
> > size = bpf_prog_info_read_offset_u32(&info, desc->size_offset);
> >
> > - data_len += count * size;
> > + data_len += roundup(count * size, sizeof(__u64));
> > }
> >
> > /* step 3: allocate continuous memory */
> > - data_len = roundup(data_len, sizeof(__u64));
> > info_linear = malloc(sizeof(struct perf_bpil) + data_len);
> > if (!info_linear)
> > return ERR_PTR(-ENOMEM);
> > @@ -180,7 +179,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> > bpf_prog_info_set_offset_u64(&info_linear->info,
> > desc->array_offset,
> > ptr_to_u64(ptr));
> > - ptr += count * size;
> > + ptr += roundup(count * size, sizeof(__u64));
>
> this one depends on info_linear->data being alligned(8), right?
>
> should we make sure it's allways the case like in the patch
> below, or it's superfluous?
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/bpf-utils.h b/tools/perf/util/bpf-utils.h
> index 86a5055cdfad..1aba76c44116 100644
> --- a/tools/perf/util/bpf-utils.h
> +++ b/tools/perf/util/bpf-utils.h
> @@ -60,7 +60,7 @@ struct perf_bpil {
> /* which arrays are included in data */
> __u64 arrays;
> struct bpf_prog_info info;
> - __u8 data[];
> + __u8 data[] __attribute__((aligned(8)));
> };
>
> struct perf_bpil *
⬢[acme@...lbox perf-urgent]$ pahole -C perf_bpil ~/bin/perf
struct perf_bpil {
__u32 info_len; /* 0 4 */
__u32 data_len; /* 4 4 */
__u64 arrays; /* 8 8 */
struct bpf_prog_info info __attribute__((__aligned__(8))); /* 16 224 */
/* XXX last struct has 4 bytes of padding */
/* --- cacheline 3 boundary (192 bytes) was 48 bytes ago --- */
__u8 data[]; /* 240 0 */
/* size: 240, cachelines: 4, members: 5 */
/* paddings: 1, sum paddings: 4 */
/* forced alignments: 1 */
/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
⬢[acme@...lbox perf-urgent]$
Humm, lotsa explicit alignments already?
Looking at the sources:
struct perf_bpil {
/* size of struct bpf_prog_info, when the tool is compiled */
__u32 info_len;
/* total bytes allocated for data, round up to 8 bytes */
__u32 data_len;
/* which arrays are included in data */
__u64 arrays;
struct bpf_prog_info info;
__u8 data[];
};
Interesting, where is pahole finding those aligned attributes? Ok
'struct bpf_prog_info' in tools/include/uapi/linux/bpf.h has aligned(8)
for the whole struct, so perf_bpil's info gets that.
sp that data right after 'info' is 8 byte alignedas
sizeof(bpf_prog_info) is a multiple of 8 bytes.
So I think I can apply the patch as-is and leave making sure data is
8-byte aligned for later.
Doing that now.
- Arnaldo
Powered by blists - more mailing lists