From: Willy Tarreau Date: Tue, 19 Sep 2017 12:59:52 +0000 (+0200) Subject: MINOR: net_helper: add functions to read from vectors X-Git-Tag: v1.8-dev3~78 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d5370e1d6c2007f07d1bf743aa6b8034a8196fda;p=thirdparty%2Fhaproxy.git MINOR: net_helper: add functions to read from vectors This patch adds the ability to read from a wrapping memory area (ie: buffers). The new functions are called "readv_". The original ones were renamed to start with "read_" to make the difference more obvious between the read method and the returned type. It's worth noting that the memory barrier in readv_bytes() is critical, as otherwise gcc decides that it doesn't need the resulting data, but even worse, removes the length checks in readv_u64() and happily performs an out-of-bounds unaligned read using read_u64()! Such "optimizations" are a bit borderline, especially when they impact security like this... --- diff --git a/include/common/net_helper.h b/include/common/net_helper.h index 78975144cb..7d5636df78 100644 --- a/include/common/net_helper.h +++ b/include/common/net_helper.h @@ -3,6 +3,7 @@ * This file contains miscellaneous network helper functions. * * Copyright (C) 2017 Olivier Houchard + * Copyright (C) 2017 Willy Tarreau * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -26,50 +27,151 @@ #ifndef _COMMON_NET_HELPER_H #define _COMMON_NET_HELPER_H +#include #include -/* Functions to read various integer that may be unaligned */ +/* Functions to read various integers that may be unaligned */ -/* Read a uint16_t */ -static inline uint16_t readu16(const void *p) +/* Read a uint16_t in native host order */ +static inline uint16_t read_u16(const void *p) { const union { uint16_t u16; } __attribute__((packed))*u = p; return u->u16; } -/* Read a int16_t */ -static inline int16_t readi16(const void *p) +/* Read a uint32_t in native host order */ +static inline uint32_t read_u32(const void *p) { - const union { int16_t i16; } __attribute__((packed))*u = p; - return u->i16; + const union { uint32_t u32; } __attribute__((packed))*u = p; + return u->u32; } -/* Read a uint16_t, and convert from network order to host order */ -static inline uint16_t readn16(const void *p) +/* Read a possibly wrapping number of bytes into destination . The + * first segment is composed of bytes at p1. The remaining byte(s), if any, + * are read from . may be zero and may also be larger than . The + * caller is always responsible for providing enough bytes. Note: the function + * is purposely *not* marked inline to let the compiler decide what to do with + * it, because it's around 34 bytes long, placed on critical path but rarely + * called, and uses uses a lot of arguments if not inlined. The compiler will + * thus decide what's best to do with it depending on the context. + */ +static void readv_bytes(void *dst, const size_t bytes, const void *p1, size_t s1, const void *p2) { - const union { uint16_t u16; } __attribute__((packed))*u = p; - return ntohs(u->u16); + size_t idx; + + p2 -= s1; + for (idx = 0; idx < bytes; idx++) { + if (idx == s1) + p1 = p2; + ((uint8_t *)dst)[idx] = ((const uint8_t *)p1)[idx]; + } + /* this memory barrier is critical otherwise gcc may over-optimize this + * code, completely removing it as well as any surrounding boundary + * check (4.7.1..6.4.0)! + */ + __asm__ volatile("" ::: "memory"); } -/* Read a uint32_t */ -static inline uint32_t readu32(const void *p) +/* Read a possibly wrapping uint16_t in native host order. The first segment is + * composed of bytes at p1. The remaining byte(s), if any, are read from + * . may be zero and may be larger than the type. The caller is always + * responsible for providing enough bytes. + */ +static inline uint16_t readv_u16(const void *p1, size_t s1, const void *p2) { - const union { uint32_t u32; } __attribute__((packed))*u = p; - return u->u32; + if (unlikely(s1 == 1)) { + volatile uint16_t u16; + + ((uint8_t *)&u16)[0] = *(uint8_t *)p1; + ((uint8_t *)&u16)[1] = *(uint8_t *)p2; + return u16; + } + else { + const union { uint16_t u16; } __attribute__((packed)) *u; + + u = (s1 == 0) ? p2 : p1; + return u->u16; + } +} + +/* Read a possibly wrapping uint32_t in native host order. The first segment is + * composed of bytes at p1. The remaining byte(s), if any, are read from + * . may be zero and may be larger than the type. The caller is always + * responsible for providing enough bytes. + */ +static inline uint32_t readv_u32(const void *p1, size_t s1, const void *p2) +{ + uint32_t u32; + + if (!unlikely(s1 < sizeof(u32))) + u32 = read_u32(p1); + else + readv_bytes(&u32, sizeof(u32), p1, s1, p2); + return u32; +} + +/* Signed integer versions : return the same data but signed */ + +/* Read an int16_t in native host order */ +static inline int16_t read_i16(const void *p) +{ + return read_u16(p); } -/* Read a int32_t */ -static inline int16_t readi32(const void *p) +/* Read an int32_t in native host order */ +static inline int32_t read_i32(const void *p) { - const union { int32_t i32; } __attribute__((packed))*u = p; - return u->i32; + return read_u32(p); +} + +/* Read a possibly wrapping int16_t in native host order */ +static inline int16_t readv_i16(const void *p1, size_t s1, const void *p2) +{ + return readv_u16(p1, s1, p2); +} + +/* Read a possibly wrapping int32_t in native host order */ +static inline int32_t readv_i32(const void *p1, size_t s1, const void *p2) +{ + return readv_u32(p1, s1, p2); +} + +/* Read a uint16_t, and convert from network order to host order */ +static inline uint16_t read_n16(const void *p) +{ + return ntohs(read_u16(p)); } /* Read a uint32_t, and convert from network order to host order */ -static inline uint32_t readn32(const void *p) +static inline uint32_t read_n32(const void *p) { - const union { uint32_t u32; } __attribute__((packed))*u = p; - return ntohl(u->u32); + return ntohl(read_u32(p)); +} + +/* Read a possibly wrapping uint16_t in network order. The first segment is + * composed of bytes at p1. The remaining byte(s), if any, are read from + * . may be zero and may be larger than the type. The caller is always + * responsible for providing enough bytes. + */ +static inline uint16_t readv_n16(const void *p1, size_t s1, const void *p2) +{ + if (unlikely(s1 < 2)) { + if (s1 == 0) + p1 = p2++; + } + else + p2 = p1 + 1; + return (*(uint8_t *)p1 << 8) + *(uint8_t *)p2; +} + +/* Read a possibly wrapping uint32_t in network order. The first segment is + * composed of bytes at p1. The remaining byte(s), if any, are read from + * . may be zero and may be larger than the type. The caller is always + * responsible for providing enough bytes. + */ +static inline uint32_t readv_n32(const void *p1, size_t s1, const void *p2) +{ + return ntohl(readv_u32(p1, s1, p2)); } #endif /* COMMON_NET_HELPER_H */ diff --git a/src/dns.c b/src/dns.c index 35c2d755fd..926e189a5d 100644 --- a/src/dns.c +++ b/src/dns.c @@ -1333,11 +1333,11 @@ int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, struct free_dns_answer_item(dns_answer_record); return DNS_RESP_INVALID; } - dns_answer_record->priority = readn16(reader); + dns_answer_record->priority = read_n16(reader); reader += sizeof(uint16_t); - dns_answer_record->weight = readn16(reader); + dns_answer_record->weight = read_n16(reader); reader += sizeof(uint16_t); - dns_answer_record->port = readn16(reader); + dns_answer_record->port = read_n16(reader); reader += sizeof(uint16_t); offset = 0; len = dns_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset);