From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Sam Kumar <samanthakumar@google.com>,
Eric Dumazet <edumazet@google.com>,
Sean Tranchetti <stranche@codeaurora.org>,
"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.19 06/24] net: udp: fix handling of CHECKSUM_COMPLETE packets
Date: Fri, 2 Nov 2018 19:34:39 +0100 [thread overview]
Message-ID: <20181102182840.762357879@linuxfoundation.org> (raw)
In-Reply-To: <20181102182839.725385066@linuxfoundation.org>
4.19-stable review patch. If anyone has any objections, please let me know.
------------------
From: Sean Tranchetti <stranche@codeaurora.org>
[ Upstream commit db4f1be3ca9b0ef7330763d07bf4ace83ad6f913 ]
Current handling of CHECKSUM_COMPLETE packets by the UDP stack is
incorrect for any packet that has an incorrect checksum value.
udp4/6_csum_init() will both make a call to
__skb_checksum_validate_complete() to initialize/validate the csum
field when receiving a CHECKSUM_COMPLETE packet. When this packet
fails validation, skb->csum will be overwritten with the pseudoheader
checksum so the packet can be fully validated by software, but the
skb->ip_summed value will be left as CHECKSUM_COMPLETE so that way
the stack can later warn the user about their hardware spewing bad
checksums. Unfortunately, leaving the SKB in this state can cause
problems later on in the checksum calculation.
Since the the packet is still marked as CHECKSUM_COMPLETE,
udp_csum_pull_header() will SUBTRACT the checksum of the UDP header
from skb->csum instead of adding it, leaving us with a garbage value
in that field. Once we try to copy the packet to userspace in the
udp4/6_recvmsg(), we'll make a call to skb_copy_and_csum_datagram_msg()
to checksum the packet data and add it in the garbage skb->csum value
to perform our final validation check.
Since the value we're validating is not the proper checksum, it's possible
that the folded value could come out to 0, causing us not to drop the
packet. Instead, we believe that the packet was checksummed incorrectly
by hardware since skb->ip_summed is still CHECKSUM_COMPLETE, and we attempt
to warn the user with netdev_rx_csum_fault(skb->dev);
Unfortunately, since this is the UDP path, skb->dev has been overwritten
by skb->dev_scratch and is no longer a valid pointer, so we end up
reading invalid memory.
This patch addresses this problem in two ways:
1) Do not use the dev pointer when calling netdev_rx_csum_fault()
from skb_copy_and_csum_datagram_msg(). Since this gets called
from the UDP path where skb->dev has been overwritten, we have
no way of knowing if the pointer is still valid. Also for the
sake of consistency with the other uses of
netdev_rx_csum_fault(), don't attempt to call it if the
packet was checksummed by software.
2) Add better CHECKSUM_COMPLETE handling to udp4/6_csum_init().
If we receive a packet that's CHECKSUM_COMPLETE that fails
verification (i.e. skb->csum_valid == 0), check who performed
the calculation. It's possible that the checksum was done in
software by the network stack earlier (such as Netfilter's
CONNTRACK module), and if that says the checksum is bad,
we can drop the packet immediately instead of waiting until
we try and copy it to userspace. Otherwise, we need to
mark the SKB as CHECKSUM_NONE, since the skb->csum field
no longer contains the full packet checksum after the
call to __skb_checksum_validate_complete().
Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
Fixes: c84d949057ca ("udp: copy skb->truesize in the first cache line")
Cc: Sam Kumar <samanthakumar@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/core/datagram.c | 5 +++--
net/ipv4/udp.c | 20 ++++++++++++++++++--
net/ipv6/ip6_checksum.c | 20 ++++++++++++++++++--
3 files changed, 39 insertions(+), 6 deletions(-)
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -808,8 +808,9 @@ int skb_copy_and_csum_datagram_msg(struc
return -EINVAL;
}
- if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
- netdev_rx_csum_fault(skb->dev);
+ if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
+ !skb->csum_complete_sw)
+ netdev_rx_csum_fault(NULL);
}
return 0;
fault:
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2120,8 +2120,24 @@ static inline int udp4_csum_init(struct
/* Note, we are only interested in != 0 or == 0, thus the
* force to int.
*/
- return (__force int)skb_checksum_init_zero_check(skb, proto, uh->check,
- inet_compute_pseudo);
+ err = (__force int)skb_checksum_init_zero_check(skb, proto, uh->check,
+ inet_compute_pseudo);
+ if (err)
+ return err;
+
+ if (skb->ip_summed == CHECKSUM_COMPLETE && !skb->csum_valid) {
+ /* If SW calculated the value, we know it's bad */
+ if (skb->csum_complete_sw)
+ return 1;
+
+ /* HW says the value is bad. Let's validate that.
+ * skb->csum is no longer the full packet checksum,
+ * so don't treat it as such.
+ */
+ skb_checksum_complete_unset(skb);
+ }
+
+ return 0;
}
/* wrapper for udp_queue_rcv_skb tacking care of csum conversion and
--- a/net/ipv6/ip6_checksum.c
+++ b/net/ipv6/ip6_checksum.c
@@ -88,8 +88,24 @@ int udp6_csum_init(struct sk_buff *skb,
* Note, we are only interested in != 0 or == 0, thus the
* force to int.
*/
- return (__force int)skb_checksum_init_zero_check(skb, proto, uh->check,
- ip6_compute_pseudo);
+ err = (__force int)skb_checksum_init_zero_check(skb, proto, uh->check,
+ ip6_compute_pseudo);
+ if (err)
+ return err;
+
+ if (skb->ip_summed == CHECKSUM_COMPLETE && !skb->csum_valid) {
+ /* If SW calculated the value, we know it's bad */
+ if (skb->csum_complete_sw)
+ return 1;
+
+ /* HW says the value is bad. Let's validate that.
+ * skb->csum is no longer the full packet checksum,
+ * so don't treat is as such.
+ */
+ skb_checksum_complete_unset(skb);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(udp6_csum_init);
next prev parent reply other threads:[~2018-11-02 18:37 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-02 18:34 [PATCH 4.19 00/24] 4.19.1-stable review Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 01/24] bridge: do not add port to router list when receives query with source 0.0.0.0 Greg Kroah-Hartman
2019-02-17 14:29 ` Sebastian Gottschall
2019-02-17 16:48 ` Greg Kroah-Hartman
2019-02-18 10:18 ` Sebastian Gottschall
2019-02-20 12:48 ` Sebastian Gottschall
2019-02-20 13:09 ` Nikolay Aleksandrov
2019-02-20 13:11 ` Nikolay Aleksandrov
2019-02-20 14:46 ` Hangbin Liu
2019-02-21 12:50 ` Sebastian Gottschall
2019-02-21 11:41 ` Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 02/24] ipv6/ndisc: Preserve IPv6 control buffer if protocol error handlers are called Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 03/24] net/mlx5e: fix csum adjustments caused by RXFCS Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 04/24] net: sched: gred: pass the right attribute to gred_change_table_def() Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 05/24] net: stmmac: Fix stmmac_mdio_reset() when building stmmac as modules Greg Kroah-Hartman
2018-11-02 18:34 ` Greg Kroah-Hartman [this message]
2018-11-02 18:34 ` [PATCH 4.19 07/24] Revert "net: simplify sock_poll_wait" Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 08/24] rtnetlink: Disallow FDB configuration for non-Ethernet device Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 09/24] vhost: Fix Spectre V1 vulnerability Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 10/24] bonding: fix length of actor system Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 11/24] openvswitch: Fix push/pop ethernet validation Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 12/24] net/ipv6: Allow onlink routes to have a device mismatch if it is the default route Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 13/24] net/smc: fix smc_buf_unuse to use the lgr pointer Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 14/24] mlxsw: spectrum_switchdev: Dont ignore deletions of learned MACs Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 15/24] mlxsw: core: Fix devlink unregister flow Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 16/24] net: drop skb on failure in ip_check_defrag() Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 17/24] net: Properly unlink GRO packets on overflow Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 18/24] r8169: fix broken Wake-on-LAN from S5 (poweroff) Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 19/24] Revert "be2net: remove desc field from be_eq_obj" Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 20/24] sctp: check policy more carefully when getting pr status Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 21/24] sparc64: Export __node_distance Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 22/24] sparc64: Make corrupted user stacks more debuggable Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 23/24] sparc64: Wire up compat getpeername and getsockname Greg Kroah-Hartman
2018-11-02 18:34 ` [PATCH 4.19 24/24] net: bridge: remove ipv6 zero address check in mcast queries Greg Kroah-Hartman
2018-11-03 14:33 ` [PATCH 4.19 00/24] 4.19.1-stable review Guenter Roeck
2018-11-04 4:24 ` Naresh Kamboju
2018-11-04 7:10 ` Greg Kroah-Hartman
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=20181102182840.762357879@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=samanthakumar@google.com \
--cc=stable@vger.kernel.org \
--cc=stranche@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox