Received: by 2002:ac0:98c7:0:0:0:0:0 with SMTP id g7-v6csp2452177imd; Fri, 2 Nov 2018 11:38:19 -0700 (PDT) X-Google-Smtp-Source: AJdET5e1HdrCbV40icXBiYxZfsKR2764oB6v/8LvqtcGIDQ29humDVNwIb0XBrDlhs1/tcGMXIle X-Received: by 2002:a65:620f:: with SMTP id d15-v6mr12242562pgv.120.1541183899134; Fri, 02 Nov 2018 11:38:19 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1541183899; cv=none; d=google.com; s=arc-20160816; b=F/0cXvBq1P3wQ+ElLayb1j3P4gssGohaBohvi/9ywCBOfCzw6jj//9njA4GcabiLjs ifbU8sNPncVi+EMhWQnZNfa4SPBU1HIg2smxrVV3gWbqfPU/UY0ZOflImpO4vTLyDfB2 LP59hkO2Wxbw7eeqa+6byqcgkstT4AhrPLg0Capoo4ywSkYN4Z20gkleZ/FfT8CcXQGP lZF71l4Vvezaw9A5e1fJuGpuw16T96YIFa+rV6AZkehFgSK+bE1xsQTCGZGKg1x6WBBQ 7KCZ9gn6rL+X8GSgqULzBwfHjM4N6OPOcie0oS9UMzN/2ID4pXt7BCr8DTiPyotejJrc D0zA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=WNwMVSfsvd6MNFZwTHcuOKo0+juxnsdQ7ZDhv0azPTA=; b=TkprhB9w4tN3huri055BR1fzZFXhyXCbO502U2P2ImYx7/D/xAbtesQmy7/x0T65an hBkaU8bIVNtpIKqbZF+HuuP5G9gNFu+tMvET88L1Gzs1Kcgg6JYKq/X1wULqMXTeiCMV 89pn5+3dlptxDehnUzVme8IxoUn20O3UaXVXP/tYdfS+R5NQSaTHVY6nLDsuGvWGopz3 31Mw0zPBYYI6Ie0njvoP/0J0DPIjEO8PISEVAlPtYRAaZmlP8FMQ5GJcE8C/bwSHv+Gm NTfQQ3wQUTowQQbhhh4ouDnMFg9K0iF9170zkCkPQXf0M9SC292KAOn0GR1j3aGYIytq DLQA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=itxTL+Rv; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w33-v6si30486315pga.590.2018.11.02.11.38.04; Fri, 02 Nov 2018 11:38:19 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=itxTL+Rv; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728552AbeKCDpZ (ORCPT + 99 others); Fri, 2 Nov 2018 23:45:25 -0400 Received: from mail.kernel.org ([198.145.29.99]:39162 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727556AbeKCDpY (ORCPT ); Fri, 2 Nov 2018 23:45:24 -0400 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8BD412081B; Fri, 2 Nov 2018 18:37:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1541183833; bh=47pQTWGO0LeA0MqfAbmPAdHtdxEAtWlwLn/XRcZVAas=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=itxTL+Rv2uHmtlR/fMWhMH0rtLYe9LlF86JljOEV9E/TNiEL8W+Wq4JJpg4ozC96O o9Ex+runLxu5npEyYbHUWKWS8DbAbsOzgJiYee3mu20euiE65sJkv/ig1PSIaXOYct tpLsyR+dfB52QXVZ0F+B4WXfSVQfuqza3vldtDXY= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Sam Kumar , Eric Dumazet , Sean Tranchetti , "David S. Miller" Subject: [PATCH 4.19 06/24] net: udp: fix handling of CHECKSUM_COMPLETE packets Date: Fri, 2 Nov 2018 19:34:39 +0100 Message-Id: <20181102182840.762357879@linuxfoundation.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102182839.725385066@linuxfoundation.org> References: <20181102182839.725385066@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.19-stable review patch. If anyone has any objections, please let me know. ------------------ From: Sean Tranchetti [ 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 Cc: Eric Dumazet Signed-off-by: Sean Tranchetti Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- 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);