Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20231002221909.2958708-1-irogers@google.com>
In-Reply-To: <20231002221909.2958708-1-irogers@google.com>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 5 Oct 2023 08:48:45 -0700
Message-ID: <CAP-5=fX-VOeCkOJY5xeW67x0+A0tGAHM4VYBBz46L-g2eRCR+w@mail.gmail.com>
Subject: Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
To:     Adrian Hunter <adrian.hunter@intel.com>
Cc:     Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Jiri Olsa <jolsa@kernel.org>, Miguel Ojeda <ojeda@kernel.org>,
        Liam Howlett <liam.howlett@oracle.com>,
        Namhyung Kim <namhyung@kernel.org>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Oct 2, 2023 at 3:19=E2=80=AFPM Ian Rogers <irogers@google.com> wrot=
e:
>
> 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@google.com>
> ---

This is a relatively small change that fixes building with
-fsanitize=3Dalignment -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/to=
ols/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 *b=
uf, size_t len,
>         if (len < 8)
>                 return INTEL_PT_NEED_MORE_BYTES;
>
> -       payload =3D le64_to_cpu(*(uint64_t *)buf);
> +       memcpy_le64(&payload, buf, sizeof(payload));
>
>         for (count =3D 47; count; count--) {
>                 if (payload & BIT63)
> @@ -220,6 +220,8 @@ static int intel_pt_get_3byte(const unsigned char *bu=
f, 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 =3D (buf[1] >> 5) & 0x3;
>         packet->type =3D 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 =3D le32_to_cpu(*(uint32_t *)(buf + 2));
> +               memcpy(&tmp, buf + 2, sizeof(tmp));
> +               packet->payload =3D le32_to_cpu(tmp);
>                 return 6;
>         case 1:
>                 if (len < 10)
>                         return INTEL_PT_NEED_MORE_BYTES;
> -               packet->payload =3D le64_to_cpu(*(uint64_t *)(buf + 2));
> +               memcpy_le64(&packet->payload, buf + 2, sizeof(packet->pay=
load));
>                 return 10;
>         default:
>                 return INTEL_PT_BAD_PACKET;
> @@ -258,7 +261,7 @@ static int intel_pt_get_mwait(const unsigned char *bu=
f, size_t len,
>         if (len < 10)
>                 return INTEL_PT_NEED_MORE_BYTES;
>         packet->type =3D INTEL_PT_MWAIT;
> -       packet->payload =3D 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 typ=
e, unsigned int byte,
>                            struct intel_pt_pkt *packet)
>  {
>         int ip_len;
> +       uint16_t tmp16;
> +       uint32_t tmp32;
>
>         packet->count =3D byte >> 5;
>
> @@ -465,13 +470,15 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type t=
ype, unsigned int byte,
>                 if (len < 3)
>                         return INTEL_PT_NEED_MORE_BYTES;
>                 ip_len =3D 2;
> -               packet->payload =3D le16_to_cpu(*(uint16_t *)(buf + 1));
> +               memcpy(&tmp16, buf + 1, sizeof(tmp16));
> +               packet->payload =3D le16_to_cpu(tmp16);
>                 break;
>         case 2:
>                 if (len < 5)
>                         return INTEL_PT_NEED_MORE_BYTES;
>                 ip_len =3D 4;
> -               packet->payload =3D le32_to_cpu(*(uint32_t *)(buf + 1));
> +               memcpy(&tmp32, buf + 1, sizeof(tmp32));
> +               packet->payload =3D le32_to_cpu(tmp32);
>                 break;
>         case 3:
>         case 4:
> @@ -484,7 +491,7 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type typ=
e, unsigned int byte,
>                 if (len < 9)
>                         return INTEL_PT_NEED_MORE_BYTES;
>                 ip_len =3D 8;
> -               packet->payload =3D le64_to_cpu(*(uint64_t *)(buf + 1));
> +               memcpy_le64(&packet->payload, buf + 1, sizeof(packet->pay=
load));
>                 break;
>         default:
>                 return INTEL_PT_BAD_PACKET;
> --
> 2.42.0.582.g8ccd20d70d-goog
>