From 52e8c95321c5291c6b55019047ec5284c14c708a Mon Sep 17 00:00:00 2001 From: Martin Stein <martin.stein@genode-labs.com> Date: Mon, 3 May 2021 15:39:35 +0200 Subject: [PATCH] net: fix packed-conversion compiler warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the update to GCC 10 the compiler used to warn when using the internet checksum functions on packet classes (like in Net::Ipv4_packet::update_checksum): warning: converting a packed ‘Net::[PACKET_CLASS]’ pointer (alignment 1) to a ‘const uint16_t’ {aka ‘const short unsigned int’} pointer (alignment 2) may result in an unaligned pointer value Apparently, the 'packed' attribute normally used on packet classes sets the alignment of the packet class to 1. However, for the purpose of the internet-checksum functions, we can assume that the packet data has no alignment. This is expressed by casting the packet-object pointer to a pointer of the new packed helper struct 'Packed_uint16' that contains only a single uint16_t member before handing it over to the checksum function (instead of casting it to a uint16_t pointer). Ref #4109 --- repos/os/include/net/internet_checksum.h | 40 ++++++++++++++++++----- repos/os/src/lib/net/icmp.cc | 4 +-- repos/os/src/lib/net/internet_checksum.cc | 22 +++++++++---- repos/os/src/lib/net/ipv4.cc | 4 +-- repos/os/src/lib/net/tcp.cc | 2 +- repos/os/src/lib/net/udp.cc | 4 +-- 6 files changed, 53 insertions(+), 23 deletions(-) diff --git a/repos/os/include/net/internet_checksum.h b/repos/os/include/net/internet_checksum.h index cf2d27179e..9ba0701eef 100644 --- a/repos/os/include/net/internet_checksum.h +++ b/repos/os/include/net/internet_checksum.h @@ -20,16 +20,38 @@ namespace Net { - Genode::uint16_t internet_checksum(Genode::uint16_t const *addr, - Genode::size_t size, - Genode::addr_t init_sum = 0); + /** + * This struct helps avoiding the following compiler warning when using + * the internet checksum functions on packet classes (like + * Net::Ipv4_packet): + * + * warning: converting a packed ‘Net::[PACKET_CLASS]’ pointer + * (alignment 1) to a ‘const uint16_t’ {aka ‘const short + * unsigned int’} pointer (alignment 2) may result in an + * unaligned pointer value + * + * Apparently, the 'packed' attribute normally used on packet classes sets + * the alignment of the packet class to 1. However, for the purpose of the + * internet-checksum functions, we can assume that the packet data has no + * alignment. This is expressed by casting the packet-object pointer to a + * Packed_uint16 pointer instead of a uint16_t pointer. + */ + struct Packed_uint16 + { + Genode::uint16_t value; - Genode::uint16_t internet_checksum_pseudo_ip(Genode::uint16_t const *addr, - Genode::size_t size, - Genode::uint16_t size_be, - Ipv4_packet::Protocol ip_prot, - Ipv4_address &ip_src, - Ipv4_address &ip_dst); + } __attribute__((packed)); + + Genode::uint16_t internet_checksum(Packed_uint16 const *addr, + Genode::size_t size, + Genode::addr_t init_sum = 0); + + Genode::uint16_t internet_checksum_pseudo_ip(Packed_uint16 const *addr, + Genode::size_t size, + Genode::uint16_t size_be, + Ipv4_packet::Protocol ip_prot, + Ipv4_address &ip_src, + Ipv4_address &ip_dst); } #endif /* _NET__INTERNET_CHECKSUM_H_ */ diff --git a/repos/os/src/lib/net/icmp.cc b/repos/os/src/lib/net/icmp.cc index e065cab5dd..50d673a906 100644 --- a/repos/os/src/lib/net/icmp.cc +++ b/repos/os/src/lib/net/icmp.cc @@ -30,11 +30,11 @@ void Net::Icmp_packet::print(Output &output) const void Icmp_packet::update_checksum(size_t data_sz) { _checksum = 0; - _checksum = internet_checksum((uint16_t *)this, sizeof(Icmp_packet) + data_sz); + _checksum = internet_checksum((Packed_uint16 *)this, sizeof(Icmp_packet) + data_sz); } bool Icmp_packet::checksum_error(size_t data_sz) const { - return internet_checksum((uint16_t *)this, sizeof(Icmp_packet) + data_sz); + return internet_checksum((Packed_uint16 *)this, sizeof(Icmp_packet) + data_sz); } diff --git a/repos/os/src/lib/net/internet_checksum.cc b/repos/os/src/lib/net/internet_checksum.cc index 567a5ba007..15f2b273e1 100644 --- a/repos/os/src/lib/net/internet_checksum.cc +++ b/repos/os/src/lib/net/internet_checksum.cc @@ -17,19 +17,27 @@ using namespace Net; using namespace Genode; +struct Packed_uint8 +{ + Genode::uint8_t value; -uint16_t Net::internet_checksum(uint16_t const *addr, - size_t size, - addr_t init_sum) +} __attribute__((packed)); + + +uint16_t Net::internet_checksum(Packed_uint16 const *addr, + size_t size, + addr_t init_sum) { /* add up bytes in pairs */ addr_t sum = init_sum; - for (; size > 1; size -= 2) - sum += *addr++; + for (; size > 1; size -= sizeof(Packed_uint16)) { + sum += addr->value; + addr++; + } /* add left-over byte, if any */ if (size > 0) - sum += *(uint8_t *)addr; + sum += ((Packed_uint8 const *)addr)->value; /* fold sum to 16-bit value */ while (addr_t const sum_rsh = sum >> 16) @@ -40,7 +48,7 @@ uint16_t Net::internet_checksum(uint16_t const *addr, } -uint16_t Net::internet_checksum_pseudo_ip(uint16_t const *ip_data, +uint16_t Net::internet_checksum_pseudo_ip(Packed_uint16 const *ip_data, size_t ip_data_sz, uint16_t ip_data_sz_be, Ipv4_packet::Protocol ip_prot, diff --git a/repos/os/src/lib/net/ipv4.cc b/repos/os/src/lib/net/ipv4.cc index 34615641a9..25f9ca555c 100644 --- a/repos/os/src/lib/net/ipv4.cc +++ b/repos/os/src/lib/net/ipv4.cc @@ -138,13 +138,13 @@ Ipv4_address Ipv4_packet::ip_from_string(const char *ip) void Ipv4_packet::update_checksum() { _checksum = 0; - _checksum = internet_checksum((uint16_t *)this, sizeof(Ipv4_packet)); + _checksum = internet_checksum((Packed_uint16 *)this, sizeof(Ipv4_packet)); } bool Ipv4_packet::checksum_error() const { - return internet_checksum((uint16_t *)this, sizeof(Ipv4_packet)); + return internet_checksum((Packed_uint16 *)this, sizeof(Ipv4_packet)); } diff --git a/repos/os/src/lib/net/tcp.cc b/repos/os/src/lib/net/tcp.cc index a9a0c5799c..0457887e73 100644 --- a/repos/os/src/lib/net/tcp.cc +++ b/repos/os/src/lib/net/tcp.cc @@ -43,7 +43,7 @@ void Net::Tcp_packet::update_checksum(Ipv4_address ip_src, size_t tcp_size) { _checksum = 0; - _checksum = internet_checksum_pseudo_ip((uint16_t*)this, tcp_size, + _checksum = internet_checksum_pseudo_ip((Packed_uint16 *)this, tcp_size, host_to_big_endian((uint16_t)tcp_size), Ipv4_packet::Protocol::TCP, ip_src, ip_dst); } diff --git a/repos/os/src/lib/net/udp.cc b/repos/os/src/lib/net/udp.cc index 7f70eebccc..0c99cff1bb 100644 --- a/repos/os/src/lib/net/udp.cc +++ b/repos/os/src/lib/net/udp.cc @@ -36,7 +36,7 @@ void Net::Udp_packet::update_checksum(Ipv4_address ip_src, Ipv4_address ip_dst) { _checksum = 0; - _checksum = internet_checksum_pseudo_ip((uint16_t*)this, length(), _length, + _checksum = internet_checksum_pseudo_ip((Packed_uint16 *)this, length(), _length, Ipv4_packet::Protocol::UDP, ip_src, ip_dst); } @@ -44,6 +44,6 @@ void Net::Udp_packet::update_checksum(Ipv4_address ip_src, bool Net::Udp_packet::checksum_error(Ipv4_address ip_src, Ipv4_address ip_dst) const { - return internet_checksum_pseudo_ip((uint16_t*)this, length(), _length, + return internet_checksum_pseudo_ip((Packed_uint16 *)this, length(), _length, Ipv4_packet::Protocol::UDP, ip_src, ip_dst); }