2022-06-14 02:57:19

by Ian Rogers

[permalink] [raw]
Subject: [PATCH] perf bpf: 8 byte align bpil data

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 <[email protected]>
---
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));
}

/* step 5: call syscall again to get required arrays */
--
2.36.1.476.g0c4daa206d-goog


2022-06-27 18:23:12

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

On Mon, Jun 13, 2022 at 6:47 PM Ian Rogers <[email protected]> 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.

Happy Monday, polite ping!

Thanks,
Ian

> Signed-off-by: Ian Rogers <[email protected]>
> ---
> 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));
> }
>
> /* step 5: call syscall again to get required arrays */
> --
> 2.36.1.476.g0c4daa206d-goog
>

2022-06-27 21:29:21

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

On Mon, Jun 13, 2022 at 6:47 PM Ian Rogers <[email protected]> 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.

typo: rounding

>
> Signed-off-by: Ian Rogers <[email protected]>
> ---

Makes sense.

Acked-by: Andrii Nakryiko <[email protected]>


> 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));
> }
>
> /* step 5: call syscall again to get required arrays */
> --
> 2.36.1.476.g0c4daa206d-goog
>

2022-06-28 08:32:21

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

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:

I need to add -w to get the clean build with that, do you see that as well?

$ make EXTRA_CFLAGS='-fsanitize=undefined -w'

>
> $ 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


and I'm also getting another error in:

[root@krava perf]# ./perf record -a sleep 1
util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
/home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
0x00000286f7f2: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
^

are you going to address this one as well?


the reason for this one is that 'data' in struct perf_record_cpu_map_data
is not alligned(8), so that's why I raised the question in my other reply ;-)

I wonder we should mark all tools/lib/perf/include/perf/event.h types
as packed to prevent any compiler padding

thanks,
jirka

2022-06-28 08:42:28

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

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 <[email protected]>
> ---
> 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 *

2022-06-28 15:16:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
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 <[email protected]>
> > ---
> > 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@toolbox 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@toolbox 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

2022-06-28 15:51:30

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

On Tue, Jun 28, 2022 at 1:41 AM <[email protected]> wrote:
>
> 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:
>
> I need to add -w to get the clean build with that, do you see that as well?
>
> $ make EXTRA_CFLAGS='-fsanitize=undefined -w'

I don't recall needing this, but I was stacking fixes which may explain it.

> >
> > $ 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
>
>
> and I'm also getting another error in:
>
> [root@krava perf]# ./perf record -a sleep 1
> util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> /home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
> 0x00000286f7f2: note: pointer points here
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
> ^
>
> are you going to address this one as well?
>
>
> the reason for this one is that 'data' in struct perf_record_cpu_map_data
> is not alligned(8), so that's why I raised the question in my other reply ;-)
>
> I wonder we should mark all tools/lib/perf/include/perf/event.h types
> as packed to prevent any compiler padding

I already sent out a fix and some improvements related to this:
https://lore.kernel.org/lkml/[email protected]/
Could you take a look?

I'm not sure about aligned and packed. I tried to minimize it in the
change above. The issue is that taking the address of a variable in a
packed struct results in an unaligned pointer. To address this in the
fix above I changed the functions to pass pointers to the whole
struct.

Thanks,
Ian

> thanks,
> jirka

2022-06-28 16:02:29

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf bpf: 8 byte align bpil data

On Tue, Jun 28, 2022 at 08:15:04AM -0700, Ian Rogers wrote:
> On Tue, Jun 28, 2022 at 1:41 AM <[email protected]> wrote:
> >
> > 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:
> >
> > I need to add -w to get the clean build with that, do you see that as well?
> >
> > $ make EXTRA_CFLAGS='-fsanitize=undefined -w'
>
> I don't recall needing this, but I was stacking fixes which may explain it.
>
> > >
> > > $ 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
> >
> >
> > and I'm also getting another error in:
> >
> > [root@krava perf]# ./perf record -a sleep 1
> > util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > /home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
> > 0x00000286f7f2: note: pointer points here
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
> > ^
> >
> > are you going to address this one as well?
> >
> >
> > the reason for this one is that 'data' in struct perf_record_cpu_map_data
> > is not alligned(8), so that's why I raised the question in my other reply ;-)
> >
> > I wonder we should mark all tools/lib/perf/include/perf/event.h types
> > as packed to prevent any compiler padding
>
> I already sent out a fix and some improvements related to this:
> https://lore.kernel.org/lkml/[email protected]/
> Could you take a look?

ok, I overlooked that one

>
> I'm not sure about aligned and packed. I tried to minimize it in the
> change above. The issue is that taking the address of a variable in a
> packed struct results in an unaligned pointer. To address this in the
> fix above I changed the functions to pass pointers to the whole
> struct.

ok, will check,

thanks,
jirka