Re: [Qemu-devel] [PATCH] slirp: Use lduw_be_p in slirp_input

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/17/19 12:50 AM, Samuel Thibault wrote:
> Hello,
> 
> Richard Henderson, le mer. 26 déc. 2018 14:42:54 +1100, a ecrit:
>> The pointer may be unaligned, so we must use our routines for that.
>> At the same time, we might as well use the big-endian version
>> instead of ntohs.
>>
>> This fixes sparc64 host SIGBUS during pxe boot.
> 
> I'm not at ease with applying this, when Marc-André is trying to make
> slirp an external library...  I'd rather apply the change below, could
> somebody review it?
> 
> Samuel
> 
> 
> slirp: Avoid unaligned 16bit memory access
> 
> pkt parameter may be unaligned, so we must access it byte-wise.
> 
> This fixes sparc64 host SIGBUS during pxe boot.
> 
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index ab2fc4eb8b..0e41d5aedf 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -851,7 +851,7 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
>      if (pkt_len < ETH_HLEN)
>          return;
>  
> -    proto = ntohs(*(uint16_t *)(pkt + 12));
> +    proto = (((uint16_t) pkt[12]) << 8) + pkt[13];
>      switch(proto) {
>      case ETH_P_ARP:
>          arp_input(slirp, pkt, pkt_len);

What about using memcpy?

-- >8 --
@@ -846,12 +846,13 @@ static void arp_input(Slirp *slirp, const uint8_t
*pkt, int pkt_len)
 void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
 {
     struct mbuf *m;
-    int proto;
+    uint16_t proto;

     if (pkt_len < ETH_HLEN)
         return;

-    proto = ntohs(*(uint16_t *)(pkt + 12));
+    memcpy(&proto, pkt + 12, sizeof(proto)); /* Avoid unaligned 16bit
access */
+    proto = ntohs(proto);
     switch(proto) {
     case ETH_P_ARP:
         arp_input(slirp, pkt, pkt_len);
---