From b347189ca62c553b60c53c3b6978ebd354c1c88b Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Wed, 20 Dec 2017 01:07:48 -0800 Subject: [PATCH] Add EXTRACT_ macros/functions for IPv4 addresses, get rid of structure wrappers. Add EXTRACT_IPV4_TO_HOST_ORDER() and EXTRACT_IPV4_TO_NETWORK_ORDER(); the former extracts a possibly-unaligned IPv4 address, in network byte order, returning a uint32_t in host byte order, and the latter extracts a possibly-unaligned IPv4 address, in network byte order, returning a uint32_t in *network* byte order. Some APIs take an address in network byte order, and some operations are more easily done in host byte order, so both are useful. Remove the structure wrappers around nd_ipv4 and nd_ipv6; that makes it easier to pass variables of those types to functions/macros that take a byte pointer as an argument (because they might be used either with pointers to structure members or raw buffer pointers), and the structure probably wouldn't do much to prevent people from using EXTRACT_BE_U_4() when they really want to extract the value in *network* byte order; using the above EXTRACT_IPV4_ calls should do more to encourage that. --- cpack.c | 4 +++- extract.h | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ netdissect.h | 40 ++++++++++++++++++++------------------- print-decnet.c | 2 +- print-ip.c | 11 ++++------- 5 files changed, 80 insertions(+), 28 deletions(-) diff --git a/cpack.c b/cpack.c index 7628a5c3..26d79643 100644 --- a/cpack.c +++ b/cpack.c @@ -35,9 +35,11 @@ #include #include -#include "cpack.h" +#include "netdissect.h" #include "extract.h" +#include "cpack.h" + const uint8_t * cpack_next_boundary(const uint8_t *buf, const uint8_t *p, size_t alignment) { diff --git a/extract.h b/extract.h index 28c828eb..8f2ed8d0 100644 --- a/extract.h +++ b/extract.h @@ -19,6 +19,8 @@ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. */ +#include + /* * For 8-bit values; needed to fetch a one-byte value. Byte order * isn't relevant, and alignment isn't an issue. @@ -121,6 +123,16 @@ EXTRACT_BE_S_8(const void *p) ((uint64_t)ntohl(*((const uint32_t *)(p) + 1))) << 0)); } + +/* + * Extract an IPv4 address, which is in network byte order, and not + * necessarily aligned, and provide the result in host byte order. + */ +static inline uint32_t UNALIGNED_OK +EXTRACT_IPV4_TO_HOST_ORDER(const void *p) +{ + return ((uint32_t)ntohl(*(const uint32_t *)(p))); +} #elif defined(__GNUC__) && defined(HAVE___ATTRIBUTE__) && \ (defined(__alpha) || defined(__alpha__) || \ defined(__mips) || defined(__mips__)) @@ -228,6 +240,16 @@ EXTRACT_BE_S_8(const void *p) return ((int64_t)(((uint64_t)ntohl(((const unaligned_uint32_t *)(p) + 0)->val)) << 32 | ((uint64_t)ntohl(((const unaligned_uint32_t *)(p) + 1)->val)) << 0)); } + +/* + * Extract an IPv4 address, which is in network byte order, and not + * necessarily aligned, and provide the result in host byte order. + */ +UNALIGNED_OK static inline uint32_t +EXTRACT_IPV4_TO_HOST_ORDER(const void *p) +{ + return ((uint32_t)ntohl(((const unaligned_uint32_t *)(p))->val)); +} #else /* * This architecture doesn't natively support unaligned loads, and either @@ -271,8 +293,37 @@ EXTRACT_BE_S_8(const void *p) ((uint64_t)(*((const uint8_t *)(p) + 5)) << 16) | \ ((uint64_t)(*((const uint8_t *)(p) + 6)) << 8) | \ ((uint64_t)(*((const uint8_t *)(p) + 7)) << 0))) + +/* + * Extract an IPv4 address, which is in network byte order, and not + * necessarily aligned, and provide the result in host byte order. + */ +#define EXTRACT_IPV4_TO_HOST_ORDER(p) \ + ((uint32_t)(((uint32_t)(*((const uint8_t *)(p) + 0)) << 24) | \ + ((uint32_t)(*((const uint8_t *)(p) + 1)) << 16) | \ + ((uint32_t)(*((const uint8_t *)(p) + 2)) << 8) | \ + ((uint32_t)(*((const uint8_t *)(p) + 3)) << 0))) #endif /* unaligned access checks */ +/* + * Extract an IPv4 address, which is in network byte order, and which + * is not necessarily aligned on a 4-byte boundary, and provide the + * result in network byte order. + * + * This works the same way regardless of the host's byte order. + */ +static inline uint32_t +EXTRACT_IPV4_TO_NETWORK_ORDER(const void *p) +{ + uint32_t addr; + + UNALIGNED_MEMCPY(&addr, p, sizeof(uint32_t)); + return addr; +} + +/* + * Non-power-of-2 sizes. + */ #define EXTRACT_BE_U_3(p) \ ((uint32_t)(((uint32_t)(*((const uint8_t *)(p) + 0)) << 16) | \ ((uint32_t)(*((const uint8_t *)(p) + 1)) << 8) | \ diff --git a/netdissect.h b/netdissect.h index 46f9670a..47507c31 100644 --- a/netdissect.h +++ b/netdissect.h @@ -67,31 +67,33 @@ typedef unsigned char nd_int32_t[4]; * * It's defined as an array of octets, so that it's not guaranteed to * be aligned on its "natural" boundary (in some packet formats, it - * *isn't* so aligned), and it's defined as a structure in the hopes - * that this makes it harder to naively use EXTRACT_BE_U_4() to extract - * the value - in many cases you just want to use UNALIGNED_MEMCPY() to - * copy its value, so that it remains in network byte order. + * *isn't* so aligned). We have separate EXTRACT_ calls for them; + * sometimes you want the host-byte-order value, other times you want + * the network-byte-order value. * - * (Among other things, we don't want somebody thinking "IPv4 addresses, - * they're in network byte order, so we want EXTRACT_BE_U_4(), right?" - * and then handing the result to system APIs that expect network-order - * IPv4 addresses, such as inet_ntop(), on their little-endian PCs, getting - * the wrong behavior, and concluding "oh, it must be in *little*-endian - * order" and "fixing" it to use EXTRACT_LE_U_4(). Yes, people do this; - * that's why Wireshark has tvb_get_ipv4(), to extract an IPv4 address from - * a packet data buffer; it was introduced in reaction to somebody who - * *had* done that.) + * Don't use EXTRACT_BE_U_4() on them, use EXTRACT_IPV4_TO_HOST_ORDER() + * if you want them in host byte order and EXTRACT_IPV4_TO_NETWORK_ORDER() + * if you want them in network byte order (which you want with system APIs + * that expect network-order IPv4 addresses, such as inet_ntop()). + * + * If, on your little-endian machine (e.g., an "IBM-compatible PC", no matter + * what the OS, or an Intel Mac, no matter what the OS), you get the wrong + * answer, and you've used EXTRACT_BE_U_4(), do *N*O*T* "fix" this by using + * EXTRACT_LE_U_4(), fix it by using EXTRACT_IPV4_TO_NETWORK_ORDER(), + * otherwise you're breaking the result on big-endian machines (e.g., + * most PowerPC/Power ISA machines, System/390 and z/Architecture, SPARC, + * etc.). + * + * Yes, people do this; that's why Wireshark has tvb_get_ipv4(), to extract + * an IPv4 address from a packet data buffer; it was introduced in reaction + * to somebody who *had* done that. */ -typedef struct { - unsigned char bytes[4]; -} nd_ipv4; +typedef unsigned char nd_ipv4[4]; /* * Use this for IPv6 addresses and netmasks. */ -typedef struct { - unsigned char bytes[16]; -} nd_ipv6; +typedef unsigned char nd_ipv6[16]; /* * Use this for MAC addresses. diff --git a/print-decnet.c b/print-decnet.c index b86bd889..4ac74e11 100644 --- a/print-decnet.c +++ b/print-decnet.c @@ -38,8 +38,8 @@ struct rtentry; #include #include -#include "extract.h" #include "netdissect.h" +#include "extract.h" #include "addrtoname.h" static const char tstr[] = "[|decnet]"; diff --git a/print-ip.c b/print-ip.c index 1e9f1ed9..4f916bc6 100644 --- a/print-ip.c +++ b/print-ip.c @@ -98,7 +98,6 @@ ip_finddst(netdissect_options *ndo, int length; int len; const u_char *cp; - uint32_t retval; cp = (const u_char *)(ip + 1); length = (IP_HL(ip) << 2) - sizeof(struct ip); @@ -125,13 +124,11 @@ ip_finddst(netdissect_options *ndo, case IPOPT_LSRR: if (len < 7) break; - UNALIGNED_MEMCPY(&retval, cp + len - 4, 4); - return retval; + return (EXTRACT_IPV4_TO_NETWORK_ORDER(cp + len - 4)); } } trunc: - UNALIGNED_MEMCPY(&retval, &ip->ip_dst, sizeof(uint32_t)); - return retval; + return (EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_dst)); } /* @@ -155,9 +152,9 @@ nextproto4_cksum(netdissect_options *ndo, ph.len = htons((uint16_t)len); ph.mbz = 0; ph.proto = next_proto; - UNALIGNED_MEMCPY(&ph.src, &ip->ip_src, sizeof(uint32_t)); + ph.src = EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_src); if (IP_HL(ip) == 5) - UNALIGNED_MEMCPY(&ph.dst, &ip->ip_dst, sizeof(uint32_t)); + ph.dst = EXTRACT_IPV4_TO_NETWORK_ORDER(ip->ip_dst); else ph.dst = ip_finddst(ndo, ip);