All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
@ 2023-10-02 22:19 Ian Rogers
  2023-10-05 15:48 ` Ian Rogers
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-10-02 22:19 UTC (permalink / raw
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Miguel Ojeda, Liam Howlett,
	linux-perf-users, linux-kernel

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>
---
 .../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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-02 22:19 [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues Ian Rogers
@ 2023-10-05 15:48 ` Ian Rogers
  2023-10-05 19:04   ` Adrian Hunter
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-10-05 15:48 UTC (permalink / raw
  To: Adrian Hunter
  Cc: Alexander Shishkin, Ingo Molnar, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Jiri Olsa, Miguel Ojeda, Liam Howlett,
	Namhyung Kim, linux-kernel, linux-perf-users

On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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,
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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-05 15:48 ` Ian Rogers
@ 2023-10-05 19:04   ` Adrian Hunter
  2023-10-05 21:24     ` Ian Rogers
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2023-10-05 19:04 UTC (permalink / raw
  To: Ian Rogers
  Cc: Alexander Shishkin, Ingo Molnar, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Jiri Olsa, Miguel Ojeda, Liam Howlett,
	Namhyung Kim, linux-kernel, linux-perf-users

On 5/10/23 18:48, Ian Rogers wrote:
> On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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,
> 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
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-05 19:04   ` Adrian Hunter
@ 2023-10-05 21:24     ` Ian Rogers
  2023-10-09  5:29       ` Namhyung Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-10-05 21:24 UTC (permalink / raw
  To: Adrian Hunter
  Cc: Alexander Shishkin, Ingo Molnar, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Jiri Olsa, Miguel Ojeda, Liam Howlett,
	Namhyung Kim, linux-kernel, linux-perf-users

On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 5/10/23 18:48, Ian Rogers wrote:
> > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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
> >>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-05 21:24     ` Ian Rogers
@ 2023-10-09  5:29       ` Namhyung Kim
  2023-10-09 15:31         ` Ian Rogers
  0 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2023-10-09  5:29 UTC (permalink / raw
  To: Ian Rogers
  Cc: Adrian Hunter, Alexander Shishkin, Ingo Molnar,
	Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, Miguel Ojeda,
	Liam Howlett, linux-kernel, linux-perf-users

On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > On 5/10/23 18:48, Ian Rogers wrote:
> > > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.

I think it can be done as a later step as long as the interface is the
same.  Can I add your Ack's to the Adrian's patchset?

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-09  5:29       ` Namhyung Kim
@ 2023-10-09 15:31         ` Ian Rogers
  2023-10-11  5:56           ` Namhyung Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-10-09 15:31 UTC (permalink / raw
  To: Namhyung Kim
  Cc: Adrian Hunter, Alexander Shishkin, Ingo Molnar,
	Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, Miguel Ojeda,
	Liam Howlett, linux-kernel, linux-perf-users

On Sun, Oct 8, 2023 at 10:29 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> > >
> > > On 5/10/23 18:48, Ian Rogers wrote:
> > > > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.
>
> I think it can be done as a later step as long as the interface is the
> same.  Can I add your Ack's to the Adrian's patchset?

I think addressing Arnaldo's comment:
https://lore.kernel.org/lkml/ZR8QnasisGEsaaDR@kernel.org/
will need some changes to the patch series, and so I was waiting to
see the outcome of that.

Thanks,
Ian

> Thanks,
> Namhyung

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-09 15:31         ` Ian Rogers
@ 2023-10-11  5:56           ` Namhyung Kim
  2023-10-11  6:50             ` Ian Rogers
  0 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2023-10-11  5:56 UTC (permalink / raw
  To: Ian Rogers
  Cc: Adrian Hunter, Alexander Shishkin, Ingo Molnar,
	Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, Miguel Ojeda,
	Liam Howlett, linux-kernel, linux-perf-users

On Mon, Oct 9, 2023 at 8:31 AM Ian Rogers <irogers@google.com> wrote:
>
> On Sun, Oct 8, 2023 at 10:29 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> > > >
> > > > On 5/10/23 18:48, Ian Rogers wrote:
> > > > > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.
> >
> > I think it can be done as a later step as long as the interface is the
> > same.  Can I add your Ack's to the Adrian's patchset?
>
> I think addressing Arnaldo's comment:
> https://lore.kernel.org/lkml/ZR8QnasisGEsaaDR@kernel.org/
> will need some changes to the patch series, and so I was waiting to
> see the outcome of that.

It seems it's done without further changes.  Can I get your Ack's now?

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-11  5:56           ` Namhyung Kim
@ 2023-10-11  6:50             ` Ian Rogers
  2023-10-12 12:27               ` Adrian Hunter
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-10-11  6:50 UTC (permalink / raw
  To: Namhyung Kim
  Cc: Adrian Hunter, Alexander Shishkin, Ingo Molnar,
	Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, Miguel Ojeda,
	Liam Howlett, linux-kernel, linux-perf-users

On Tue, Oct 10, 2023 at 10:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Mon, Oct 9, 2023 at 8:31 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Sun, Oct 8, 2023 at 10:29 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> > > > >
> > > > > On 5/10/23 18:48, Ian Rogers wrote:
> > > > > > On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.
> > >
> > > I think it can be done as a later step as long as the interface is the
> > > same.  Can I add your Ack's to the Adrian's patchset?
> >
> > I think addressing Arnaldo's comment:
> > https://lore.kernel.org/lkml/ZR8QnasisGEsaaDR@kernel.org/
> > will need some changes to the patch series, and so I was waiting to
> > see the outcome of that.
>
> It seems it's done without further changes.  Can I get your Ack's now?

With the unaligned.h patch on its own, I think patch 1 of 5 needs
dropping. For the rest I'm happy to acked-by.

Thanks,
Ian

> Thanks,
> Namhyung

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-11  6:50             ` Ian Rogers
@ 2023-10-12 12:27               ` Adrian Hunter
  2023-10-13  4:23                 ` Namhyung Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2023-10-12 12:27 UTC (permalink / raw
  To: Ian Rogers, Namhyung Kim
  Cc: Alexander Shishkin, Ingo Molnar, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Jiri Olsa, Miguel Ojeda, Liam Howlett,
	linux-kernel, linux-perf-users

On 11/10/23 09:50, Ian Rogers wrote:
> On Tue, Oct 10, 2023 at 10:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> On Mon, Oct 9, 2023 at 8:31 AM Ian Rogers <irogers@google.com> wrote:
>>>
>>> On Sun, Oct 8, 2023 at 10:29 PM Namhyung Kim <namhyung@kernel.org> wrote:
>>>>
>>>> On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
>>>>>
>>>>> On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>>
>>>>>> On 5/10/23 18:48, Ian Rogers wrote:
>>>>>>> On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.
>>>>
>>>> I think it can be done as a later step as long as the interface is the
>>>> same.  Can I add your Ack's to the Adrian's patchset?
>>>
>>> I think addressing Arnaldo's comment:
>>> https://lore.kernel.org/lkml/ZR8QnasisGEsaaDR@kernel.org/
>>> will need some changes to the patch series, and so I was waiting to
>>> see the outcome of that.
>>
>> It seems it's done without further changes.  Can I get your Ack's now?
> 
> With the unaligned.h patch on its own, I think patch 1 of 5 needs
> dropping. For the rest I'm happy to acked-by.

The new patch is on top of the others, so patch 1 is still needed.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues
  2023-10-12 12:27               ` Adrian Hunter
@ 2023-10-13  4:23                 ` Namhyung Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Namhyung Kim @ 2023-10-13  4:23 UTC (permalink / raw
  To: Adrian Hunter
  Cc: Ian Rogers, Alexander Shishkin, Ingo Molnar,
	Arnaldo Carvalho de Melo, Peter Zijlstra, Jiri Olsa, Miguel Ojeda,
	Liam Howlett, linux-kernel, linux-perf-users

On Thu, Oct 12, 2023 at 5:27 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 11/10/23 09:50, Ian Rogers wrote:
> > On Tue, Oct 10, 2023 at 10:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >>
> >> On Mon, Oct 9, 2023 at 8:31 AM Ian Rogers <irogers@google.com> wrote:
> >>>
> >>> On Sun, Oct 8, 2023 at 10:29 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >>>>
> >>>> On Thu, Oct 5, 2023 at 2:24 PM Ian Rogers <irogers@google.com> wrote:
> >>>>>
> >>>>> On Thu, Oct 5, 2023 at 12:06 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>>>>
> >>>>>> On 5/10/23 18:48, Ian Rogers wrote:
> >>>>>>> On Mon, Oct 2, 2023 at 3:19 PM Ian Rogers <irogers@google.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@google.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.
> >>>>
> >>>> I think it can be done as a later step as long as the interface is the
> >>>> same.  Can I add your Ack's to the Adrian's patchset?
> >>>
> >>> I think addressing Arnaldo's comment:
> >>> https://lore.kernel.org/lkml/ZR8QnasisGEsaaDR@kernel.org/
> >>> will need some changes to the patch series, and so I was waiting to
> >>> see the outcome of that.
> >>
> >> It seems it's done without further changes.  Can I get your Ack's now?
> >
> > With the unaligned.h patch on its own, I think patch 1 of 5 needs
> > dropping. For the rest I'm happy to acked-by.
>
> The new patch is on top of the others, so patch 1 is still needed.

I think I can squash it to the patch 1 if needed.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-13  4:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-02 22:19 [PATCH v1] perf intel-pt: pkt-decoder: Fix alignment issues Ian Rogers
2023-10-05 15:48 ` Ian Rogers
2023-10-05 19:04   ` Adrian Hunter
2023-10-05 21:24     ` Ian Rogers
2023-10-09  5:29       ` Namhyung Kim
2023-10-09 15:31         ` Ian Rogers
2023-10-11  5:56           ` Namhyung Kim
2023-10-11  6:50             ` Ian Rogers
2023-10-12 12:27               ` Adrian Hunter
2023-10-13  4:23                 ` Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.