Re: Fix umb(4) on big-endian machines

2017-05-03 Thread Stefan Sperling
On Wed, May 03, 2017 at 10:12:04AM +0200, Gerhard Roth wrote:
> Hi,
> 
> all MBIM values are in litte-endian encoding but somewhere in the fine
> print it reads that "the addresses will be in network byte order".
> 
> So applying letoh32() on addresses is just plain wrong. On little-endian
> machines, we didn't notice since letoh32() is a no-op there. But one
> big-endian ones, the driver used IP addresses in the wrong byte order.
> 
> Cheers,
> 
> Gerhard
> 

ok

I don't have a big-endian machine with umb(4) to test with, though.

> 
> 
> Index: sys/dev/usb/if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 if_umb.c
> --- sys/dev/usb/if_umb.c  18 Apr 2017 13:27:55 -  1.11
> +++ sys/dev/usb/if_umb.c  2 May 2017 14:41:32 -
> @@ -1646,7 +1646,6 @@ umb_decode_ip_configuration(struct umb_s
>  
>   /* Only pick the first one */
>   memcpy(, data + off, sizeof (ipv4elem));
> - ipv4elem.addr = letoh32(ipv4elem.addr);
>   ipv4elem.prefixlen = letoh32(ipv4elem.prefixlen);
>  
>   memset(, 0, sizeof (ifra));
> @@ -1660,8 +1659,7 @@ umb_decode_ip_configuration(struct umb_s
>   sin->sin_len = sizeof (ifra.ifra_dstaddr);
>   if (avail & MBIM_IPCONF_HAS_GWINFO) {
>   off = letoh32(ic->ipv4_gwoffs);
> - sin->sin_addr.s_addr =
> - letoh32(*((uint32_t *)(data + off)));
> + sin->sin_addr.s_addr = *((uint32_t *)(data + off));
>   }
>  
>   sin = (struct sockaddr_in *)_mask;
> @@ -1690,7 +1688,7 @@ umb_decode_ip_configuration(struct umb_s
>   while (n-- > 0) {
>   if (off + sizeof (uint32_t) > len)
>   break;
> - val = letoh32(*((uint32_t *)(data + off)));
> + val = *((uint32_t *)(data + off));
>   if (i < UMB_MAX_DNSSRV)
>   sc->sc_info.ipv4dns[i++] = val;
>   off += sizeof (uint32_t);
> 



Fix umb(4) on big-endian machines

2017-05-03 Thread Gerhard Roth
Hi,

all MBIM values are in litte-endian encoding but somewhere in the fine
print it reads that "the addresses will be in network byte order".

So applying letoh32() on addresses is just plain wrong. On little-endian
machines, we didn't notice since letoh32() is a no-op there. But one
big-endian ones, the driver used IP addresses in the wrong byte order.

Cheers,

Gerhard



Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 if_umb.c
--- sys/dev/usb/if_umb.c18 Apr 2017 13:27:55 -  1.11
+++ sys/dev/usb/if_umb.c2 May 2017 14:41:32 -
@@ -1646,7 +1646,6 @@ umb_decode_ip_configuration(struct umb_s
 
/* Only pick the first one */
memcpy(, data + off, sizeof (ipv4elem));
-   ipv4elem.addr = letoh32(ipv4elem.addr);
ipv4elem.prefixlen = letoh32(ipv4elem.prefixlen);
 
memset(, 0, sizeof (ifra));
@@ -1660,8 +1659,7 @@ umb_decode_ip_configuration(struct umb_s
sin->sin_len = sizeof (ifra.ifra_dstaddr);
if (avail & MBIM_IPCONF_HAS_GWINFO) {
off = letoh32(ic->ipv4_gwoffs);
-   sin->sin_addr.s_addr =
-   letoh32(*((uint32_t *)(data + off)));
+   sin->sin_addr.s_addr = *((uint32_t *)(data + off));
}
 
sin = (struct sockaddr_in *)_mask;
@@ -1690,7 +1688,7 @@ umb_decode_ip_configuration(struct umb_s
while (n-- > 0) {
if (off + sizeof (uint32_t) > len)
break;
-   val = letoh32(*((uint32_t *)(data + off)));
+   val = *((uint32_t *)(data + off));
if (i < UMB_MAX_DNSSRV)
sc->sc_info.ipv4dns[i++] = val;
off += sizeof (uint32_t);