From: Alex Elder <elder@linaro.org>
To: subashab@codeaurora.org, stranche@codeaurora.org,
davem@davemloft.net, kuba@kernel.org
Cc: bjorn.andersson@linaro.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH net-next 4/8] net: qualcomm: rmnet: return earlier for bad checksum
Date: Sat, 12 Jun 2021 09:37:32 -0500 [thread overview]
Message-ID: <20210612143736.3498712-5-elder@linaro.org> (raw)
In-Reply-To: <20210612143736.3498712-1-elder@linaro.org>
In rmnet_map_ipv4_dl_csum_trailer(), if the sum of the trailer
checksum and the pseudo checksum is non-zero, checksum validation
has failed. We can return an error as soon as we know that.
We can do the same thing in rmnet_map_ipv6_dl_csum_trailer().
Add some comments that explain where we're headed.
Signed-off-by: Alex Elder <elder@linaro.org>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 36 ++++++++++++++-----
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 51909b8fa8a80..a05124eb8602e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -76,6 +76,17 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
* We verified above that the IP header contributes zero to the
* trailer checksum. Therefore the checksum in the trailer is
* just the checksum computed over the IP payload.
+
+ * If the IP payload arrives intact, adding the pseudo header
+ * checksum to the IP payload checksum will yield 0xffff (negative
+ * zero). This means the trailer checksum and the pseudo checksum
+ * are additive inverses of each other. Put another way, the
+ * message passes the checksum test if the trailer checksum value
+ * is the negated pseudo header checksum.
+ *
+ * Knowing this, we don't even need to examine the transport
+ * header checksum value; it is already accounted for in the
+ * checksum value found in the trailer.
*/
ip_payload_csum = (__force __sum16)~csum_trailer->csum_value;
@@ -84,11 +95,11 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
ip4h->protocol, 0);
pseudo_csum = csum16_add(ip_payload_csum, (__force __be16)pseudo_csum);
- /* The trailer checksum *includes* the checksum in the transport
- * header. Adding that to the pseudo checksum will yield 0xffff
- * ("negative 0") if the message arrived intact.
- */
- WARN_ON((__sum16)~pseudo_csum);
+ /* The cast is required to ensure only the low 16 bits are examined */
+ if ((__sum16)~pseudo_csum) {
+ priv->stats.csum_validation_failed++;
+ return -EINVAL;
+ }
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(!csum_value_final)) {
@@ -143,6 +154,11 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* transport checksum from this, we first subract the contribution
* of the IP header from the trailer checksum. We then add the
* checksum computed over the pseudo header.
+ *
+ * It's sufficient to compare the IP payload checksum with the
+ * negated pseudo checksum to determine whether the packet
+ * checksum was good. (See further explanation in comments
+ * in rmnet_map_ipv4_dl_csum_trailer()).
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
ip6_payload_csum = ~csum16_sub((__force __sum16)csum_trailer->csum_value,
@@ -155,10 +171,12 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
length, ip6h->nexthdr, 0);
pseudo_csum = csum16_add(ip6_payload_csum, (__force __be16)pseudo_csum);
- /* Adding the payload checksum to the pseudo checksum yields 0xffff
- * ("negative 0") if the message arrived intact.
- */
- WARN_ON((__sum16)~pseudo_csum);
+ /* The cast is required to ensure only the low 16 bits are examined */
+ if ((__sum16)~pseudo_csum) {
+ priv->stats.csum_validation_failed++;
+ return -EINVAL;
+ }
+
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(csum_value_final == 0)) {
--
2.27.0
next prev parent reply other threads:[~2021-06-12 14:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
2021-06-12 14:37 ` [PATCH net-next 1/8] net: qualcomm: rmnet: remove some local variables Alex Elder
2021-06-12 14:37 ` [PATCH net-next 2/8] net: qualcomm: rmnet: rearrange some NOTs Alex Elder
2021-06-12 14:37 ` [PATCH net-next 3/8] net: qualcomm: rmnet: show that an intermediate sum is zero Alex Elder
2021-06-12 14:37 ` Alex Elder [this message]
2021-06-12 14:37 ` [PATCH net-next 5/8] net: qualcomm: rmnet: remove unneeded code Alex Elder
2021-06-12 14:37 ` [PATCH net-next 6/8] net: qualcomm: rmnet: trailer value is a checksum Alex Elder
2021-06-12 14:37 ` [PATCH net-next 7/8] net: qualcomm: rmnet: drop some unary NOTs Alex Elder
2021-06-12 14:37 ` [PATCH net-next 8/8] net: qualcomm: rmnet: IPv6 payload length is simple Alex Elder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210612143736.3498712-5-elder@linaro.org \
--to=elder@linaro.org \
--cc=bjorn.andersson@linaro.org \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stranche@codeaurora.org \
--cc=subashab@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.