| 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
| ||
|
Message-ID: <CAP-5=fXzcR6QKKhFG5PS1pFYi5JsO4AOUoH_-Bv0O+11YzEkkw@mail.gmail.com>
Date: Thu, 5 Oct 2023 14:24:29 -0700
From: Ian Rogers <irogers@...gle.com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Jiri Olsa <jolsa@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Liam Howlett <liam.howlett@...cle.com>,
Namhyung Kim <namhyung@...nel.org>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> On 5/10/23 18:48, Ian Rogers wrote:
> > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@...gle.com> 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@...gle.com>
> >> ---
> >
> > 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! This has been down my list of things to do for ages,
> but using get_unaligned_le16() etc seems nicer. I sent a patch
> set for that.
Thanks Adrian! Your patch set looks good and I think after Arnaldo's
comment is addressed we should go with it.
Ian
> >
> > 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
> >>
>
Powered by blists - more mailing lists