From 7d4f8524f57ba0ac77e47171bebfc0370df667b1 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 1 Sep 2026 09:08:47 +0300 Subject: utils: centralize unaligned integer accessors --- accel-pppd/ctrl/l2tp/packet.c | 68 +++++--------------------------------- accel-pppd/ctrl/l2tp/packet_test.c | 2 +- accel-pppd/ctrl/pppoe/pppoe.c | 26 +++------------ accel-pppd/ctrl/pppoe/tr101.c | 37 +++++++++------------ 4 files changed, 29 insertions(+), 104 deletions(-) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/l2tp/packet.c b/accel-pppd/ctrl/l2tp/packet.c index 0a4113a0..f134666d 100644 --- a/accel-pppd/ctrl/l2tp/packet.c +++ b/accel-pppd/ctrl/l2tp/packet.c @@ -111,58 +111,6 @@ void l2tp_packet_free(struct l2tp_packet_t *pack) mempool_free(pack); } -/* - * AVPs are not aligned in any way inside the packet buffer: their offset - * depends on the length of every preceding AVP, which is peer chosen. - * Always go through memcpy() to read multi-byte fields out of them, both to - * stay portable on strict alignment architectures and to avoid tripping - * -fsanitize=alignment. - */ -static uint16_t unaligned_ntohs(const void *ptr) -{ - uint16_t val; - - memcpy(&val, ptr, sizeof(val)); - - return ntohs(val); -} - -static uint32_t unaligned_ntohl(const void *ptr) -{ - uint32_t val; - - memcpy(&val, ptr, sizeof(val)); - - return ntohl(val); -} - -static uint64_t unaligned_be64toh(const void *ptr) -{ - uint64_t val; - - memcpy(&val, ptr, sizeof(val)); - - return be64toh(val); -} - -static void unaligned_htons(void *ptr, uint16_t val) -{ - val = htons(val); - memcpy(ptr, &val, sizeof(val)); -} - -static void unaligned_htonl(void *ptr, uint32_t val) -{ - val = htonl(val); - memcpy(ptr, &val, sizeof(val)); -} - -static void unaligned_htobe64(void *ptr, uint64_t val) -{ - val = htobe64(val); - memcpy(ptr, &val, sizeof(val)); -} - static void memxor(uint8_t *dst, const uint8_t *src, size_t sz) { size_t indx; @@ -223,7 +171,7 @@ static int decode_avp(struct l2tp_avp_t *avp, const struct l2tp_attr_t *RV, } memxor(p1, avp->val, MD5_DIGEST_LENGTH); - orig_attr_len = unaligned_ntohs(p1); + orig_attr_len = u_read_be16(p1); if (orig_attr_len <= MD5_DIGEST_LENGTH - sizeof(uint16_t)) { /* Enough bytes decoded already, no need to decode padding */ @@ -271,7 +219,7 @@ out: trustworthy as the peer's knowledge of the shared secret. Bound it against the room actually available in the received AVP before letting it drive any read of the attribute value */ - orig_attr_len = unaligned_ntohs(avp->val); + orig_attr_len = u_read_be16(avp->val); if (orig_attr_len > attr_len - sizeof(uint16_t)) { log_warn("l2tp: incorrect hidden avp received (type %hu):" " deciphered attribute length too big (ciphered" @@ -502,17 +450,17 @@ int l2tp_recv(int fd, struct l2tp_packet_t **p, struct in_pktinfo *pkt_info, case ATTR_TYPE_INT16: if (orig_avp_len != sizeof(*avp) + 2) goto out_err_len; - attr->val.uint16 = unaligned_ntohs(orig_avp_val); + attr->val.uint16 = u_read_be16(orig_avp_val); break; case ATTR_TYPE_INT32: if (orig_avp_len != sizeof(*avp) + 4) goto out_err_len; - attr->val.uint32 = unaligned_ntohl(orig_avp_val); + attr->val.uint32 = u_read_be32(orig_avp_val); break; case ATTR_TYPE_INT64: if (orig_avp_len != sizeof(*avp) + 8) goto out_err_len; - attr->val.uint64 = unaligned_be64toh(orig_avp_val); + attr->val.uint64 = u_read_be64(orig_avp_val); break; case ATTR_TYPE_OCTETS: attr->val.octets = _malloc(attr->length); @@ -589,13 +537,13 @@ int l2tp_packet_send(int sock, struct l2tp_packet_t *pack) else switch (attr->attr->type) { case ATTR_TYPE_INT16: - unaligned_htons(avp->val, attr->val.int16); + u_write_be16(avp->val, attr->val.int16); break; case ATTR_TYPE_INT32: - unaligned_htonl(avp->val, attr->val.int32); + u_write_be32(avp->val, attr->val.int32); break; case ATTR_TYPE_INT64: - unaligned_htobe64(avp->val, attr->val.uint64); + u_write_be64(avp->val, attr->val.uint64); break; case ATTR_TYPE_STRING: case ATTR_TYPE_OCTETS: diff --git a/accel-pppd/ctrl/l2tp/packet_test.c b/accel-pppd/ctrl/l2tp/packet_test.c index 9f962407..a6c9a182 100644 --- a/accel-pppd/ctrl/l2tp/packet_test.c +++ b/accel-pppd/ctrl/l2tp/packet_test.c @@ -4,7 +4,7 @@ * Not part of the cmake build. Compile and run with: * gcc -O1 -g -Wall -fno-strict-aliasing -D_GNU_SOURCE \ * -fsanitize=address,undefined -fno-sanitize-recover=all \ - * -I accel-pppd/include -I accel-pppd/ctrl/l2tp \ + * -I accel-pppd -I accel-pppd/include -I accel-pppd/ctrl/l2tp \ * -o /tmp/l2tp_packet_test \ * accel-pppd/ctrl/l2tp/packet_test.c accel-pppd/ctrl/l2tp/packet.c \ * -lcrypto && /tmp/l2tp_packet_test diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 00f962af..bd92cbf8 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -554,22 +554,6 @@ static void print_tag_octets(struct pppoe_tag *tag) log_info2("%02x", (uint8_t)tag->tag_data[i]); } -static uint16_t pppoe_read_u16(const void *ptr) -{ - uint16_t value; - - memcpy(&value, ptr, sizeof(value)); - return ntohs(value); -} - -static uint32_t pppoe_read_u32(const void *ptr) -{ - uint32_t value; - - memcpy(&value, ptr, sizeof(value)); - return ntohl(value); -} - static void print_tag_u16(struct pppoe_tag *tag) { if (ntohs(tag->tag_len) != sizeof(uint16_t)) { @@ -577,7 +561,7 @@ static void print_tag_u16(struct pppoe_tag *tag) return; } - log_info2("%i", pppoe_read_u16(tag->tag_data)); + log_info2("%i", u_read_be16(tag->tag_data)); } static void print_packet(const char *ifname, const char *op, uint8_t *pack) @@ -654,7 +638,7 @@ static void print_packet(const char *ifname, const char *op, uint8_t *pack) if (ntohs(tag->tag_len) < 4) log_info2(" "); else - log_info2(" ", pppoe_read_u32(tag->tag_data)); + log_info2(" ", u_read_be32(tag->tag_data)); break; case TAG_RELAY_SESSION_ID: log_info2(" tag_len) == 2) - ppp_max_payload = pppoe_read_u16(tag->tag_data); + ppp_max_payload = u_read_be16(tag->tag_data); break; } } @@ -1248,14 +1232,14 @@ static void pppoe_recv_PADR(struct pppoe_serv_t *serv, uint8_t *pack, int size) case TAG_VENDOR_SPECIFIC: if (ntohs(tag->tag_len) < 4) continue; - vendor_id = pppoe_read_u32(tag->tag_data); + vendor_id = u_read_be32(tag->tag_data); if (vendor_id == VENDOR_ADSL_FORUM) if (conf_tr101) tr101_tag = tag; break; case TAG_PPP_MAX_PAYLOAD: if (ntohs(tag->tag_len) == 2) - ppp_max_payload = pppoe_read_u16(tag->tag_data); + ppp_max_payload = u_read_be16(tag->tag_data); break; } } diff --git a/accel-pppd/ctrl/pppoe/tr101.c b/accel-pppd/ctrl/pppoe/tr101.c index e7aa96dc..06aeff86 100644 --- a/accel-pppd/ctrl/pppoe/tr101.c +++ b/accel-pppd/ctrl/pppoe/tr101.c @@ -8,6 +8,7 @@ #include "log.h" #include "radius.h" #include "memdebug.h" +#include "utils.h" #include "pppoe.h" @@ -30,14 +31,6 @@ #define ACCESS_LOOP_ENCAP 0x90 #define IFW_SESSION 0xFE -static uint32_t tr101_read_u32(const void *ptr) -{ - uint32_t value; - - memcpy(&value, ptr, sizeof(value)); - return ntohl(value); -} - static int tr101_send_request(struct pppoe_tag *tr101, struct rad_packet_t *pack, int type) { uint8_t *ptr = (uint8_t *)tr101->tag_data + 4; @@ -83,85 +76,85 @@ static int tr101_send_request(struct pppoe_tag *tr101, struct rad_packet_t *pack case OPT_ACTUAL_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Upstream", u_read_be32(ptr))) return -1; break; case OPT_ACTUAL_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Downstream", u_read_be32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream", u_read_be32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream", u_read_be32(ptr))) return -1; break; case OPT_ATT_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Upstream", u_read_be32(ptr))) return -1; break; case OPT_ATT_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Downstream", u_read_be32(ptr))) return -1; break; case OPT_MAX_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Upstream", u_read_be32(ptr))) return -1; break; case OPT_MAX_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Downstream", u_read_be32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_UP_LP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream-Low-Power", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream-Low-Power", u_read_be32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_DOWN_LP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream-Low-Power", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream-Low-Power", u_read_be32(ptr))) return -1; break; case OPT_MAX_INTERL_DELAY_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Upstream", u_read_be32(ptr))) return -1; break; case OPT_ACTUAL_INTERL_DELAY_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Upstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Upstream", u_read_be32(ptr))) return -1; break; case OPT_MAX_INTER_DELAY_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Downstream", u_read_be32(ptr))) return -1; break; case OPT_ACTUAL_INTER_DELAY_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Downstream", tr101_read_u32(ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Downstream", u_read_be32(ptr))) return -1; break; case ACCESS_LOOP_ENCAP: -- cgit v1.2.3