linux-kernel - [PATCH net-next 7/8] net: qualcomm: rmnet: avoid unnecessary byte-swapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210611190529.3085813-8-elder@linaro.org>
Date:   Fri, 11 Jun 2021 14:05:28 -0500
From:   Alex Elder <elder@...aro.org>
To:     subashab@...eaurora.org, stranche@...eaurora.org,
        davem@...emloft.net, kuba@...nel.org
Cc:     bjorn.andersson@...aro.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH net-next 7/8] net: qualcomm: rmnet: avoid unnecessary byte-swapping

Internet checksums are used for IPv4 header checksum, as well as TCP
segment and UDP datagram checksums.  Such a checksum represents the
negated sum of adjacent pairs of bytes, using ones' complement
arithmetic.

One property of the Internet checkum is byte order independence [1].
Specifically, the sum of byte-swapped pairs is equal to the result
of byte swapping the sum of those same pairs when not byte-swapped.

So for example if a, b, c, d, y, and z are hexadecimal digits, and
PLUS represents ones' complement addition:
    If:		ab PLUS cd = yz
    Then:	ba PLUS dc = zy

For this reason, there is no need to swap the order of bytes in the
checksum value held in a message header, nor the one in the QMAPv4
trailer, in order to operate on them.

In other words, we can determine whether the hardware-computed
checksum matches the one in the message header without any byte
swaps.

(This patch leaves in place all existing type casts.)

[1] https://tools.ietf.org/html/rfc1071

Signed-off-by: Alex Elder <elder@...aro.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 4f93355e9a93a..39f198d7595bd 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -78,15 +78,15 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
 	 * trailer checksum.  Therefore the checksum in the trailer is
 	 * just the checksum computed over the IP payload.
 	 */
-	ip_payload_csum = (__force __sum16)~ntohs(csum_trailer->csum_value);
+	ip_payload_csum = (__force __sum16)~csum_trailer->csum_value;
 
 	pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
 					 ntohs(ip4h->tot_len) - ip4h->ihl * 4,
 					 ip4h->protocol, 0);
-	addend = (__force __be16)ntohs((__force __be16)pseudo_csum);
+	addend = (__force __be16)pseudo_csum;
 	pseudo_csum = csum16_add(ip_payload_csum, addend);
 
-	addend = (__force __be16)ntohs((__force __be16)*csum_field);
+	addend = (__force __be16)*csum_field;
 	csum_temp = ~csum16_sub(pseudo_csum, addend);
 	csum_value_final = (__force u16)csum_temp;
 
@@ -105,7 +105,7 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
 		}
 	}
 
-	if (csum_value_final == ntohs((__force __be16)*csum_field)) {
+	if (csum_value_final == (__force u16)*csum_field) {
 		priv->stats.csum_ok++;
 		return 0;
 	} else {
-- 
2.27.0