In eth_strip_vlan_ex() we take a pointer to the eth_header h_proto
field into a local uint16_t* variable, and then later in the function
we dereference that pointer.  This isn't safe, because the eth_header
struct may not be aligned, and if we mark the struct as QEMU_PACKED
then gcc will complain about taking the address of a field in a
packed struct.

Instead, make the local variable be a void* and use the appropriate
functions for accessing 16 bits of possibly unaligned data through
it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 net/eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 3f680cc033..12ec316e24 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -274,7 +274,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, int index,
                   uint16_t *payload_offset, uint16_t *tci)
 {
     struct vlan_header vlan_hdr;
-    uint16_t *new_ehdr_proto;
+    void *new_ehdr_proto;
     size_t new_ehdr_size;
     size_t copied;
 
@@ -298,7 +298,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, int index,
         return 0;
     }
 
-    if (copied < new_ehdr_size || be16_to_cpu(*new_ehdr_proto) != vet) {
+    if (copied < new_ehdr_size || lduw_be_p(new_ehdr_proto) != vet) {
         return 0;
     }
 
@@ -308,7 +308,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, int index,
         return 0;
     }
 
-    *new_ehdr_proto = vlan_hdr.h_proto;
+    stw_he_p(new_ehdr_proto, vlan_hdr.h_proto);
     *payload_offset = iovoff + new_ehdr_size + sizeof(vlan_hdr);
     *tci = be16_to_cpu(vlan_hdr.h_tci);
 
-- 
2.43.0

On 12/2/26 15:09, Peter Maydell wrote:
> In eth_strip_vlan_ex() we take a pointer to the eth_header h_proto
> field into a local uint16_t* variable, and then later in the function
> we dereference that pointer.  This isn't safe, because the eth_header
> struct may not be aligned, and if we mark the struct as QEMU_PACKED
> then gcc will complain about taking the address of a field in a
> packed struct.
> 
> Instead, make the local variable be a void* and use the appropriate
> functions for accessing 16 bits of possibly unaligned data through
> it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   net/eth.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>