All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] net: fix misaligned member access
@ 2018-02-09 19:03 Marc-André Lureau
  2018-02-09 19:50 ` Philippe Mathieu-Daudé
  2018-03-02 17:22 ` Peter Maydell
  0 siblings, 2 replies; 5+ messages in thread
From: Marc-André Lureau @ 2018-02-09 19:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc-André Lureau, Dmitry Fleytman, Jason Wang

Fixes the following ASAN warnings:

/home/elmarco/src/qemu/hw/net/net_tx_pkt.c:201:27: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
0x631000028846: note: pointer points here
 01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
             ^
/home/elmarco/src/qemu/hw/net/net_tx_pkt.c:208:63: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
0x631000028846: note: pointer points here
 01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
             ^
/home/elmarco/src/qemu/hw/net/net_tx_pkt.c:210:13: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
0x631000028846: note: pointer points here
 01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/net/eth.h   | 4 +++-
 hw/net/net_tx_pkt.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/net/eth.h b/include/net/eth.h
index 09054a506d..e6dc8a7ba0 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -194,7 +194,9 @@ struct tcp_hdr {
 #define PKT_GET_IP_HDR(p)         \
     ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
 #define IP_HDR_GET_LEN(p)         \
-    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
+    ((ldub_p(p + offsetof(struct ip_header, ip_ver_len)) & 0x0F) << 2)
+#define IP_HDR_GET_P(p)                                           \
+    (ldub_p(p + offsetof(struct ip_header, ip_p)))
 #define PKT_GET_IP_HDR_LEN(p)     \
     (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
 #define PKT_GET_IP6_HDR(p)        \
diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index e29c881bc2..162f802dd7 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -205,7 +205,7 @@ static bool net_tx_pkt_parse_headers(struct NetTxPkt *pkt)
             return false;
         }
 
-        pkt->l4proto = ((struct ip_header *) l3_hdr->iov_base)->ip_p;
+        pkt->l4proto = IP_HDR_GET_P(l3_hdr->iov_base);
 
         if (IP_HDR_GET_LEN(l3_hdr->iov_base) != sizeof(struct ip_header)) {
             /* copy optional IPv4 header data if any*/
-- 
2.16.1.73.g5832b7e9f2

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: fix misaligned member access
  2018-02-09 19:03 [Qemu-devel] [PATCH] net: fix misaligned member access Marc-André Lureau
@ 2018-02-09 19:50 ` Philippe Mathieu-Daudé
  2018-02-11 12:04   ` Marc-André Lureau
  2018-03-02 17:22 ` Peter Maydell
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-09 19:50 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel; +Cc: Jason Wang, Dmitry Fleytman

Hi Marc-André,

On 02/09/2018 04:03 PM, Marc-André Lureau wrote:
> Fixes the following ASAN warnings:
> 
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:201:27: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>              ^
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:208:63: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>              ^
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:210:13: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/net/eth.h   | 4 +++-
>  hw/net/net_tx_pkt.c | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 09054a506d..e6dc8a7ba0 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -194,7 +194,9 @@ struct tcp_hdr {
>  #define PKT_GET_IP_HDR(p)         \
>      ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>  #define IP_HDR_GET_LEN(p)         \
> -    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
> +    ((ldub_p(p + offsetof(struct ip_header, ip_ver_len)) & 0x0F) << 2)

I like the idea of using the ldst API and offsetof() macro,
however I think it would be cleaner to use inlined static functions.

> +#define IP_HDR_GET_P(p)                                           \
> +    (ldub_p(p + offsetof(struct ip_header, ip_p)))
>  #define PKT_GET_IP_HDR_LEN(p)     \
>      (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
>  #define PKT_GET_IP6_HDR(p)        \
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index e29c881bc2..162f802dd7 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -205,7 +205,7 @@ static bool net_tx_pkt_parse_headers(struct NetTxPkt *pkt)
>              return false;
>          }
>  
> -        pkt->l4proto = ((struct ip_header *) l3_hdr->iov_base)->ip_p;
> +        pkt->l4proto = IP_HDR_GET_P(l3_hdr->iov_base);
>  
>          if (IP_HDR_GET_LEN(l3_hdr->iov_base) != sizeof(struct ip_header)) {
>              /* copy optional IPv4 header data if any*/
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: fix misaligned member access
  2018-02-09 19:50 ` Philippe Mathieu-Daudé
@ 2018-02-11 12:04   ` Marc-André Lureau
  0 siblings, 0 replies; 5+ messages in thread
From: Marc-André Lureau @ 2018-02-11 12:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: QEMU, Jason Wang, Dmitry Fleytman

Hi

On Fri, Feb 9, 2018 at 8:50 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Marc-André,
>
> On 02/09/2018 04:03 PM, Marc-André Lureau wrote:
>> Fixes the following ASAN warnings:
>>
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:201:27: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>              ^
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:208:63: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>              ^
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:210:13: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  include/net/eth.h   | 4 +++-
>>  hw/net/net_tx_pkt.c | 2 +-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/eth.h b/include/net/eth.h
>> index 09054a506d..e6dc8a7ba0 100644
>> --- a/include/net/eth.h
>> +++ b/include/net/eth.h
>> @@ -194,7 +194,9 @@ struct tcp_hdr {
>>  #define PKT_GET_IP_HDR(p)         \
>>      ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>>  #define IP_HDR_GET_LEN(p)         \
>> -    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
>> +    ((ldub_p(p + offsetof(struct ip_header, ip_ver_len)) & 0x0F) << 2)
>
> I like the idea of using the ldst API and offsetof() macro,
> however I think it would be cleaner to use inlined static functions.

Could be a different patch. I don't see much reason to mix this with
the alignment fix.

Then you need to review the rest of the define too.

Thanks

>
>> +#define IP_HDR_GET_P(p)                                           \
>> +    (ldub_p(p + offsetof(struct ip_header, ip_p)))
>>  #define PKT_GET_IP_HDR_LEN(p)     \
>>      (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
>>  #define PKT_GET_IP6_HDR(p)        \
>> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
>> index e29c881bc2..162f802dd7 100644
>> --- a/hw/net/net_tx_pkt.c
>> +++ b/hw/net/net_tx_pkt.c
>> @@ -205,7 +205,7 @@ static bool net_tx_pkt_parse_headers(struct NetTxPkt *pkt)
>>              return false;
>>          }
>>
>> -        pkt->l4proto = ((struct ip_header *) l3_hdr->iov_base)->ip_p;
>> +        pkt->l4proto = IP_HDR_GET_P(l3_hdr->iov_base);
>>
>>          if (IP_HDR_GET_LEN(l3_hdr->iov_base) != sizeof(struct ip_header)) {
>>              /* copy optional IPv4 header data if any*/
>>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: fix misaligned member access
  2018-02-09 19:03 [Qemu-devel] [PATCH] net: fix misaligned member access Marc-André Lureau
  2018-02-09 19:50 ` Philippe Mathieu-Daudé
@ 2018-03-02 17:22 ` Peter Maydell
  2018-03-05  9:47   ` Peter Maydell
  1 sibling, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2018-03-02 17:22 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: QEMU Developers, Jason Wang, Dmitry Fleytman

On 9 February 2018 at 19:03, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> Fixes the following ASAN warnings:
>
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:201:27: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>              ^
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:208:63: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>              ^
> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:210:13: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
> 0x631000028846: note: pointer points here
>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/net/eth.h   | 4 +++-
>  hw/net/net_tx_pkt.c | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 09054a506d..e6dc8a7ba0 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -194,7 +194,9 @@ struct tcp_hdr {
>  #define PKT_GET_IP_HDR(p)         \
>      ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>  #define IP_HDR_GET_LEN(p)         \
> -    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
> +    ((ldub_p(p + offsetof(struct ip_header, ip_ver_len)) & 0x0F) << 2)
> +#define IP_HDR_GET_P(p)                                           \
> +    (ldub_p(p + offsetof(struct ip_header, ip_p)))
>  #define PKT_GET_IP_HDR_LEN(p)     \
>      (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
>  #define PKT_GET_IP6_HDR(p)        \
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index e29c881bc2..162f802dd7 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -205,7 +205,7 @@ static bool net_tx_pkt_parse_headers(struct NetTxPkt *pkt)
>              return false;
>          }
>
> -        pkt->l4proto = ((struct ip_header *) l3_hdr->iov_base)->ip_p;
> +        pkt->l4proto = IP_HDR_GET_P(l3_hdr->iov_base);
>
>          if (IP_HDR_GET_LEN(l3_hdr->iov_base) != sizeof(struct ip_header)) {
>              /* copy optional IPv4 header data if any*/
> --
> 2.16.1.73.g5832b7e9f2

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

and I'm going to apply this to master, because I'm fed up of the warnings
in my build system logs.

It looks like all these macros need to be fixed, though, not just these two.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: fix misaligned member access
  2018-03-02 17:22 ` Peter Maydell
@ 2018-03-05  9:47   ` Peter Maydell
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2018-03-05  9:47 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: QEMU Developers, Jason Wang, Dmitry Fleytman

On 2 March 2018 at 17:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 February 2018 at 19:03, Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>> Fixes the following ASAN warnings:
>>
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:201:27: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>              ^
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:208:63: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>              ^
>> /home/elmarco/src/qemu/hw/net/net_tx_pkt.c:210:13: runtime error: member access within misaligned address 0x631000028846 for type 'struct ip_header', which requires 4 byte alignment
>> 0x631000028846: note: pointer points here
>>  01 00 00 00 45 00  01 a9 01 00 00 00 40 11  78 45 00 00 00 00 ff ff  ff ff 00 00 00 00 00 00  00 00
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  include/net/eth.h   | 4 +++-
>>  hw/net/net_tx_pkt.c | 2 +-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/eth.h b/include/net/eth.h
>> index 09054a506d..e6dc8a7ba0 100644
>> --- a/include/net/eth.h
>> +++ b/include/net/eth.h
>> @@ -194,7 +194,9 @@ struct tcp_hdr {
>>  #define PKT_GET_IP_HDR(p)         \
>>      ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>>  #define IP_HDR_GET_LEN(p)         \
>> -    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
>> +    ((ldub_p(p + offsetof(struct ip_header, ip_ver_len)) & 0x0F) << 2)
>> +#define IP_HDR_GET_P(p)                                           \
>> +    (ldub_p(p + offsetof(struct ip_header, ip_p)))
>>  #define PKT_GET_IP_HDR_LEN(p)     \
>>      (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
>>  #define PKT_GET_IP6_HDR(p)        \
>> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
>> index e29c881bc2..162f802dd7 100644
>> --- a/hw/net/net_tx_pkt.c
>> +++ b/hw/net/net_tx_pkt.c
>> @@ -205,7 +205,7 @@ static bool net_tx_pkt_parse_headers(struct NetTxPkt *pkt)
>>              return false;
>>          }
>>
>> -        pkt->l4proto = ((struct ip_header *) l3_hdr->iov_base)->ip_p;
>> +        pkt->l4proto = IP_HDR_GET_P(l3_hdr->iov_base);
>>
>>          if (IP_HDR_GET_LEN(l3_hdr->iov_base) != sizeof(struct ip_header)) {
>>              /* copy optional IPv4 header data if any*/
>> --
>> 2.16.1.73.g5832b7e9f2
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> and I'm going to apply this to master, because I'm fed up of the warnings
> in my build system logs.

Now applied to master, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-05  9:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 19:03 [Qemu-devel] [PATCH] net: fix misaligned member access Marc-André Lureau
2018-02-09 19:50 ` Philippe Mathieu-Daudé
2018-02-11 12:04   ` Marc-André Lureau
2018-03-02 17:22 ` Peter Maydell
2018-03-05  9:47   ` Peter Maydell

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.