From patchwork Fri Jun 11 19:05:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 458513 Delivered-To: patch@linaro.org Received: by 2002:a02:735a:0:0:0:0:0 with SMTP id a26csp1210331jae; Fri, 11 Jun 2021 12:06:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwlMNcGiT9MglC7+1AGTHKaPDQ/q1MadTgvFsQBgyg3n5XAmn5sqdf8J4ogUQvTkETKjSUx X-Received: by 2002:a17:906:bc2:: with SMTP id y2mr4866101ejg.489.1623438409382; Fri, 11 Jun 2021 12:06:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623438409; cv=none; d=google.com; s=arc-20160816; b=F16CzZYbi9T0MCioWWFNgROtPqjF4PvG4bGBUGhKo9b9to7RYU1EhhxihJNNKiQKC7 hvUg5CAmULEO7QoIZpIahmYP55OK2IBTYY/HgI3XouM+cTP/cdWtp+72bGO/CxnP7+HF blHL10vItnx9szWqEp9AFAn+gP8odKAGxQVVJuM9G9ODw1TqWBDTY3lz74S65RYOYHs9 4SV3YOY8kDr9VvoySILnU4ATFg19tcpx0t4uL9niOzwFiF075jxswV+1vZ5LlgGbFJ60 Yk/5haItrlQF7q7pnxSRhXs/JORUP6QqBhLbCZmyBVm9augc77Z/3AjJBpGwK6MelbX4 UTXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=qBoOhTYQId0ccnEylChHcGfk116nSyfqpAW9wezghK8=; b=NbU+c91suJ/uW5Mg55BnuIVaA5PGyYTPd0XT2ufMgESUk1p2SBnmLa7cMuITbM3wt9 tB6FWXFsLzm0Eomw81IJmbzE9R6h75BHbU2ahXuSLBpOCOIPxZOC/WVAR8eQcWeg99g/ Z80/iTIY60RNTBAlHKFvsHEyVXrrwm3Z84gsi4rF92Us59fH5SLVqeehNY7aUtfggGXo H9SdlR6L+cguNhbBH0r+0fZRylG53j7ZUSFdvFTq+a+9SXlX+/JooxttRfuQvfLmfnq9 NqXygETJ46wUWTwyDXNRGGopDvBWOKXQBkp6Brg4Xad89WoXiuO4ycE3gbm9hYMbhsdb cKJg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=fluZkObb; spf=pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=netdev-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id y12si5229320ejc.650.2021.06.11.12.06.49; Fri, 11 Jun 2021 12:06:49 -0700 (PDT) Received-SPF: pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=fluZkObb; spf=pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=netdev-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231538AbhFKTIp (ORCPT + 8 others); Fri, 11 Jun 2021 15:08:45 -0400 Received: from mail-io1-f54.google.com ([209.85.166.54]:39766 "EHLO mail-io1-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231437AbhFKTIh (ORCPT ); Fri, 11 Jun 2021 15:08:37 -0400 Received: by mail-io1-f54.google.com with SMTP id f10so17760795iok.6 for ; Fri, 11 Jun 2021 12:06:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=qBoOhTYQId0ccnEylChHcGfk116nSyfqpAW9wezghK8=; b=fluZkObbAwxjzfggtAFSEfH6l9SfdNUXLwdjm3E25/LJNtHCbwMS9cm9iOeGvVuQGd q2HP3mjREoBDi7sxLcUpKK5xGbphnh1xqn+bdWJ2gYVeiqTT9Yg4xQu9uCDYept+wqqm xd19RwOKk3T2sKeoGXX0+ZHp1o9CZRXAZPULUWBgi08qTTmfPi2iBmLGODkZtmjHzMHi ymt1LLiOhKf3l8x5of51uLvUZjMwmcYXdHnjpT3an+a8JvJWSHsBYrHGyo/eMNLqUYU6 2fa2U4i41TpjEw+lJbNGRHfXUR8o6m51PN7oE9UDJrGcZXVWmoEZt0w4mDeF7mtnk3oM rAtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=qBoOhTYQId0ccnEylChHcGfk116nSyfqpAW9wezghK8=; b=gEziM1vl4IIM6HxRIEBuJzXBKbB7JXfVKHPbfECIWlSimWGIrqTNA+AZgZvZ3Jlc+/ cAInBYLuok9ixmefjlpMkuCp6v6eQTzbVsbAThud4chn0+DJFaCdiOFCzF247f0EuVto JyYchdlT266/K2BdLQb71pAaDDlDeGyDZmaXcwo7Z2YUdla6h3zoYYT2BSrcF/7fwORv MLu4cNQYaWwPUcad9I9quRXYbWcOtIthFvnbBFay4lqOGszKiGuWSjgJPq+cX/w2BnSQ bWCEfdo5fjdugUd4vKglISnDlXodFMAPFAKXKgrsrtMlWJLjNuyPa0P62/u31jVwyCDG 3KSw== X-Gm-Message-State: AOAM530W9Y6OUTFX/IieLPR2w+jPmFbvpy2xTa4GIJKS4/XJHZvusVhu Jq4bhQ/n64KIfUz4xCUb7NMpqQ== X-Received: by 2002:a05:6602:2143:: with SMTP id y3mr4364865ioy.89.1623438339234; Fri, 11 Jun 2021 12:05:39 -0700 (PDT) Received: from presto.localdomain (c-73-185-129-58.hsd1.mn.comcast.net. [73.185.129.58]) by smtp.gmail.com with ESMTPSA id p9sm3936566ilc.63.2021.06.11.12.05.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Jun 2021 12:05:38 -0700 (PDT) From: Alex Elder 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 5/8] net: qualcomm: rmnet: IPv4 header has zero checksum Date: Fri, 11 Jun 2021 14:05:26 -0500 Message-Id: <20210611190529.3085813-6-elder@linaro.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20210611190529.3085813-1-elder@linaro.org> References: <20210611190529.3085813-1-elder@linaro.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org In rmnet_map_ipv4_dl_csum_trailer(), an illegal checksum subtraction is done, subtracting hdr_csum (in host byte order) from csum_value (in network byte order). Despite being illegal, it generally works, because it turns out the value subtracted is (or should be) always 0, which has the same representation in either byte order. Doing illegal operations is not good form though, so fix this by verifying the IP header checksum early in that function. If its checksum is non-zero, the packet will be bad, so just return an error. This will cause the packet to passed to the IP layer where it can be dropped. Thereafter, there is no need subtract the IP header checksum from the checksum value in the trailer because we know it is zero. Add a comment explaining this. This type of packet error is different from other types, so add a new statistics counter to track this condition. Signed-off-by: Alex Elder --- .../ethernet/qualcomm/rmnet/rmnet_config.h | 1 + .../ethernet/qualcomm/rmnet/rmnet_map_data.c | 41 ++++++++++++------- .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 + 3 files changed, 29 insertions(+), 14 deletions(-) -- 2.27.0 diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h index 8e64ca98068d9..3d3cba56c5169 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h @@ -49,6 +49,7 @@ struct rmnet_pcpu_stats { struct rmnet_priv_stats { u64 csum_ok; + u64 csum_ip4_header_bad; u64 csum_valid_unset; u64 csum_validation_failed; u64 csum_err_bad_buffer; diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c index 79f1d516b5cca..40d7e0c615f9c 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c @@ -33,13 +33,21 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb, struct rmnet_map_dl_csum_trailer *csum_trailer, struct rmnet_priv *priv) { - __sum16 *csum_field, csum_temp, pseudo_csum, hdr_csum, ip_payload_csum; - u16 csum_value, csum_value_final; - struct iphdr *ip4h; - void *txporthdr; + struct iphdr *ip4h = (struct iphdr *)skb->data; + void *txporthdr = skb->data + ip4h->ihl * 4; + __sum16 *csum_field, csum_temp, pseudo_csum; + __sum16 ip_payload_csum; + u16 csum_value_final; __be16 addend; - ip4h = (struct iphdr *)(skb->data); + /* Computing the checksum over just the IPv4 header--including its + * checksum field--should yield 0. If it doesn't, the IP header + * is bad, so return an error and let the IP layer drop it. + */ + if (ip_fast_csum(ip4h, ip4h->ihl)) { + priv->stats.csum_ip4_header_bad++; + return -EINVAL; + } /* We don't support checksum offload on IPv4 fragments */ if (ip_is_fragment(ip4h)) { @@ -47,25 +55,30 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb, return -EOPNOTSUPP; } - txporthdr = skb->data + ip4h->ihl * 4; - + /* Checksum offload is only supported for UDP and TCP protocols */ csum_field = rmnet_map_get_csum_field(ip4h->protocol, txporthdr); - if (!csum_field) { priv->stats.csum_err_invalid_transport++; return -EPROTONOSUPPORT; } - /* RFC 768 - Skip IPv4 UDP packets where sender checksum field is 0 */ - if (*csum_field == 0 && ip4h->protocol == IPPROTO_UDP) { + /* RFC 768: UDP checksum is optional for IPv4, and is 0 if unused */ + if (!*csum_field && ip4h->protocol == IPPROTO_UDP) { priv->stats.csum_skipped++; return 0; } - csum_value = ~ntohs(csum_trailer->csum_value); - hdr_csum = ~ip_fast_csum(ip4h, (int)ip4h->ihl); - ip_payload_csum = csum16_sub((__force __sum16)csum_value, - (__force __be16)hdr_csum); + /* The checksum value in the trailer is computed over the entire + * IP packet, including the IP header and payload. To derive the + * 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. + * + * 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. + */ + ip_payload_csum = (__force __sum16)~ntohs(csum_trailer->csum_value); pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr, ntohs(ip4h->tot_len) - ip4h->ihl * 4, diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c index fe13017e9a41e..6556b5381ce85 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c @@ -166,6 +166,7 @@ static const struct net_device_ops rmnet_vnd_ops = { static const char rmnet_gstrings_stats[][ETH_GSTRING_LEN] = { "Checksum ok", + "Bad IPv4 header checksum", "Checksum valid bit not set", "Checksum validation failed", "Checksum error bad buffer",