Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues

From: Ian Rogers
Date: Thu Oct 05 2023 - 12:15:34 EST


On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> The byte aligned buffer is cast to large types and dereferenced
> causing misaligned pointer warnings from undefined behavior sanitizer.
> Fix the alignment issues with memcpy which may require the
> introduction of temporaries.
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---

This is a relatively small change that fixes building with
-fsanitize=alignment -fsanitize-undefined-trap-on-error. Adrian, as
this is Intel-PT could you take a look?

Thanks,
Ian

> .../intel-pt-decoder/intel-pt-pkt-decoder.c | 21 ++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
> index af9710622a1f..28659874d84e 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
> @@ -83,7 +83,7 @@ static int intel_pt_get_long_tnt(const unsigned char *buf, size_t len,
> if (len < 8)
> return INTEL_PT_NEED_MORE_BYTES;
>
> - payload = le64_to_cpu(*(uint64_t *)buf);
> + memcpy_le64(&payload, buf, sizeof(payload));
>
> for (count = 47; count; count--) {
> if (payload & BIT63)
> @@ -220,6 +220,8 @@ static int intel_pt_get_3byte(const unsigned char *buf, size_t len,
> static int intel_pt_get_ptwrite(const unsigned char *buf, size_t len,
> struct intel_pt_pkt *packet)
> {
> + uint32_t tmp;
> +
> packet->count = (buf[1] >> 5) & 0x3;
> packet->type = buf[1] & BIT(7) ? INTEL_PT_PTWRITE_IP :
> INTEL_PT_PTWRITE;
> @@ -228,12 +230,13 @@ static int intel_pt_get_ptwrite(const unsigned char *buf, size_t len,
> case 0:
> if (len < 6)
> return INTEL_PT_NEED_MORE_BYTES;
> - packet->payload = le32_to_cpu(*(uint32_t *)(buf + 2));
> + memcpy(&tmp, buf + 2, sizeof(tmp));
> + packet->payload = le32_to_cpu(tmp);
> return 6;
> case 1:
> if (len < 10)
> return INTEL_PT_NEED_MORE_BYTES;
> - packet->payload = le64_to_cpu(*(uint64_t *)(buf + 2));
> + memcpy_le64(&packet->payload, buf + 2, sizeof(packet->payload));
> return 10;
> default:
> return INTEL_PT_BAD_PACKET;
> @@ -258,7 +261,7 @@ static int intel_pt_get_mwait(const unsigned char *buf, size_t len,
> if (len < 10)
> return INTEL_PT_NEED_MORE_BYTES;
> packet->type = INTEL_PT_MWAIT;
> - packet->payload = le64_to_cpu(*(uint64_t *)(buf + 2));
> + memcpy_le64(&packet->payload, buf + 2, sizeof(packet->payload));
> return 10;
> }
>
> @@ -454,6 +457,8 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type type, unsigned int byte,
> struct intel_pt_pkt *packet)
> {
> int ip_len;
> + uint16_t tmp16;
> + uint32_t tmp32;
>
> packet->count = byte >> 5;
>
> @@ -465,13 +470,15 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type type, unsigned int byte,
> if (len < 3)
> return INTEL_PT_NEED_MORE_BYTES;
> ip_len = 2;
> - packet->payload = le16_to_cpu(*(uint16_t *)(buf + 1));
> + memcpy(&tmp16, buf + 1, sizeof(tmp16));
> + packet->payload = le16_to_cpu(tmp16);
> break;
> case 2:
> if (len < 5)
> return INTEL_PT_NEED_MORE_BYTES;
> ip_len = 4;
> - packet->payload = le32_to_cpu(*(uint32_t *)(buf + 1));
> + memcpy(&tmp32, buf + 1, sizeof(tmp32));
> + packet->payload = le32_to_cpu(tmp32);
> break;
> case 3:
> case 4:
> @@ -484,7 +491,7 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type type, unsigned int byte,
> if (len < 9)
> return INTEL_PT_NEED_MORE_BYTES;
> ip_len = 8;
> - packet->payload = le64_to_cpu(*(uint64_t *)(buf + 1));
> + memcpy_le64(&packet->payload, buf + 1, sizeof(packet->payload));
> break;
> default:
> return INTEL_PT_BAD_PACKET;
> --
> 2.42.0.582.g8ccd20d70d-goog
>