Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Keith Packard <keithpac@amazon.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-staging@lists.linux.dev, linux-block@vger.kernel.org,
	linux-kbuild@vger.kernel.org, clang-built-linux@googlegroups.com
Subject: [PATCH 21/64] cxgb4: Use struct_group() for memcpy() region
Date: Tue, 27 Jul 2021 13:58:12 -0700	[thread overview]
Message-ID: <20210727205855.411487-22-keescook@chromium.org> (raw)
In-Reply-To: <20210727205855.411487-1-keescook@chromium.org>

In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.

Use struct_group() in struct fw_eth_tx_pkt_vm_wr around members ethmacdst,
ethmacsrc, ethtype, and vlantci, so they can be referenced together. This
will allow memcpy() and sizeof() to more easily reason about sizes,
improve readability, and avoid future warnings about writing beyond the
end of ethmacdst.

"pahole" shows no size nor member offset changes to struct
fw_eth_tx_pkt_vm_wr. "objdump -d" shows no object code changes.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c      |  8 +++++---
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 10 ++++++----
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c    |  7 ++-----
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 6a099cb34b12..9080b2c5ffe8 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -1842,8 +1842,10 @@ static netdev_tx_t cxgb4_vf_eth_xmit(struct sk_buff *skb,
 	 * (including the VLAN tag) into the header so we reject anything
 	 * smaller than that ...
 	 */
-	fw_hdr_copy_len = sizeof(wr->ethmacdst) + sizeof(wr->ethmacsrc) +
-			  sizeof(wr->ethtype) + sizeof(wr->vlantci);
+	BUILD_BUG_ON(sizeof(wr->firmware) !=
+		     (sizeof(wr->ethmacdst) + sizeof(wr->ethmacsrc) +
+		      sizeof(wr->ethtype) + sizeof(wr->vlantci)));
+	fw_hdr_copy_len = sizeof(wr->firmware);
 	ret = cxgb4_validate_skb(skb, dev, fw_hdr_copy_len);
 	if (ret)
 		goto out_free;
@@ -1924,7 +1926,7 @@ static netdev_tx_t cxgb4_vf_eth_xmit(struct sk_buff *skb,
 	wr->equiq_to_len16 = cpu_to_be32(wr_mid);
 	wr->r3[0] = cpu_to_be32(0);
 	wr->r3[1] = cpu_to_be32(0);
-	skb_copy_from_linear_data(skb, (void *)wr->ethmacdst, fw_hdr_copy_len);
+	skb_copy_from_linear_data(skb, &wr->firmware, fw_hdr_copy_len);
 	end = (u64 *)wr + flits;
 
 	/* If this is a Large Send Offload packet we'll put in an LSO CPL
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
index 0a326c054707..2419459a0b85 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
@@ -794,10 +794,12 @@ struct fw_eth_tx_pkt_vm_wr {
 	__be32 op_immdlen;
 	__be32 equiq_to_len16;
 	__be32 r3[2];
-	u8 ethmacdst[6];
-	u8 ethmacsrc[6];
-	__be16 ethtype;
-	__be16 vlantci;
+	struct_group(firmware,
+		u8 ethmacdst[ETH_ALEN];
+		u8 ethmacsrc[ETH_ALEN];
+		__be16 ethtype;
+		__be16 vlantci;
+	);
 };
 
 #define FW_CMD_MAX_TIMEOUT 10000
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index 7bc80eeb2c21..671ca93e64ab 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -1167,10 +1167,7 @@ netdev_tx_t t4vf_eth_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct cpl_tx_pkt_core *cpl;
 	const struct skb_shared_info *ssi;
 	dma_addr_t addr[MAX_SKB_FRAGS + 1];
-	const size_t fw_hdr_copy_len = (sizeof(wr->ethmacdst) +
-					sizeof(wr->ethmacsrc) +
-					sizeof(wr->ethtype) +
-					sizeof(wr->vlantci));
+	const size_t fw_hdr_copy_len = sizeof(wr->firmware);
 
 	/*
 	 * The chip minimum packet length is 10 octets but the firmware
@@ -1267,7 +1264,7 @@ netdev_tx_t t4vf_eth_xmit(struct sk_buff *skb, struct net_device *dev)
 	wr->equiq_to_len16 = cpu_to_be32(wr_mid);
 	wr->r3[0] = cpu_to_be32(0);
 	wr->r3[1] = cpu_to_be32(0);
-	skb_copy_from_linear_data(skb, (void *)wr->ethmacdst, fw_hdr_copy_len);
+	skb_copy_from_linear_data(skb, &wr->firmware, fw_hdr_copy_len);
 	end = (u64 *)wr + flits;
 
 	/*
-- 
2.30.2


  parent reply	other threads:[~2021-07-27 21:17 UTC|newest]

Thread overview: 158+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 20:57 [PATCH 00/64] Introduce strict memcpy() bounds checking Kees Cook
2021-07-27 20:57 ` [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region Kees Cook
2021-07-28  0:55   ` Gustavo A. R. Silva
2021-07-28  1:50     ` Kees Cook
2021-07-28  8:59   ` David Sterba
2021-07-28  9:14     ` Dan Carpenter
2021-07-28 21:37       ` Bart Van Assche
2021-07-28 21:37         ` David Sterba
2021-07-29  5:56           ` Greg Kroah-Hartman
2021-07-29  8:20             ` Dan Carpenter
2021-07-30  6:00               ` Kees Cook
2021-07-30  8:38                 ` David Sterba
2021-07-30  9:00                   ` Dan Carpenter
2021-07-30 16:44                     ` Kees Cook
2021-07-30 17:08                       ` Nick Desaulniers
2021-07-30 19:18                         ` Kees Cook
2021-07-27 20:57 ` [PATCH 02/64] mac80211: Use flex-array for radiotap header bitmap Kees Cook
2021-07-28  7:35   ` Dan Carpenter
2021-07-28  9:23     ` David Sterba
2021-07-28 21:54       ` Kees Cook
2021-07-29 10:45         ` David Sterba
2021-07-30  6:06           ` Kees Cook
2021-07-28 21:20     ` Kees Cook
2021-07-28 23:14     ` Kees Cook
2021-07-28 23:33     ` Kees Cook
2021-07-29  8:25       ` Dan Carpenter
2021-07-27 20:57 ` [PATCH 03/64] rpmsg: glink: Replace strncpy() with strscpy_pad() Kees Cook
2021-07-28  2:07   ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 04/64] stddef: Introduce struct_group() helper macro Kees Cook
2021-07-28  2:32   ` Gustavo A. R. Silva
2021-07-28 10:54   ` Rasmus Villemoes
2021-07-28 21:59     ` Kees Cook
2021-07-30 22:19       ` Williams, Dan J
2021-07-31  2:59         ` Kees Cook
     [not found]           ` <CAKwiHFheDv2pwsm6Fa+-KnOFyvk7bfZQjb2BQ-CSwH61gxgVYg@mail.gmail.com>
2021-07-31 15:10             ` Kees Cook
2021-07-27 20:57 ` [PATCH 05/64] skbuff: Switch structure bounds to struct_group() Kees Cook
2021-07-28  3:50   ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 06/64] bnxt_en: Use struct_group_attr() for memcpy() region Kees Cook
2021-07-28  1:03   ` Michael Chan
2021-07-28  4:45   ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 07/64] staging: rtl8192e: Use struct_group() " Kees Cook
2021-07-27 22:30   ` Larry Finger
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-27 20:57 ` [PATCH 08/64] staging: rtl8192u: " Kees Cook
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 09/64] staging: rtl8723bs: Avoid field-overflowing memcpy() Kees Cook
2021-07-28  5:46   ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 10/64] lib80211: Use struct_group() for memcpy() region Kees Cook
2021-07-28  5:52   ` Greg Kroah-Hartman
2021-08-13  8:04   ` Johannes Berg
2021-08-13 15:49     ` Kees Cook
2021-08-13 19:44       ` Johannes Berg
2021-07-27 20:58 ` [PATCH 11/64] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
2021-07-27 20:58 ` [PATCH 12/64] mwl8k: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58 ` [PATCH 13/64] libertas: " Kees Cook
2021-07-27 20:58 ` [PATCH 14/64] libertas_tf: " Kees Cook
2021-07-27 20:58 ` [PATCH 15/64] ipw2x00: " Kees Cook
2021-07-28 18:55   ` Stanislav Yakovlev
2021-07-27 20:58 ` [PATCH 16/64] thermal: intel: int340x_thermal: " Kees Cook
2021-07-27 20:58 ` [PATCH 17/64] iommu/amd: " Kees Cook
2021-07-27 20:58 ` [PATCH 18/64] cxgb3: " Kees Cook
2021-07-27 20:58 ` [PATCH 19/64] ip: Use struct_group() for memcpy() regions Kees Cook
2021-07-28  5:55   ` Greg Kroah-Hartman
2021-07-28  6:14     ` Gustavo A. R. Silva
2021-07-28  6:19       ` Greg Kroah-Hartman
2021-07-28  6:31         ` Gustavo A. R. Silva
2021-07-28  6:37           ` Gustavo A. R. Silva
2021-07-28  6:41           ` Greg Kroah-Hartman
2021-07-28 21:01     ` Kees Cook
2021-07-29  1:59       ` Bart Van Assche
2021-07-27 20:58 ` [PATCH 20/64] intersil: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58 ` Kees Cook [this message]
2021-07-27 20:58 ` [PATCH 22/64] bnx2x: " Kees Cook
2021-07-27 20:58 ` [PATCH 23/64] drm/amd/pm: " Kees Cook
2021-07-30  2:07   ` Alex Deucher
2021-07-27 20:58 ` [PATCH 24/64] staging: wlan-ng: " Kees Cook
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 25/64] drm/mga/mga_ioc32: " Kees Cook
2021-07-28  5:56   ` Greg Kroah-Hartman
2021-07-29 12:11     ` Daniel Vetter
2021-07-31  4:20       ` Kees Cook
2021-07-27 20:58 ` [PATCH 26/64] net/mlx5e: " Kees Cook
2021-07-27 20:58 ` [PATCH 27/64] HID: cp2112: " Kees Cook
2021-07-27 20:58 ` [PATCH 28/64] compiler_types.h: Remove __compiletime_object_size() Kees Cook
2021-07-27 20:58 ` [PATCH 29/64] lib/string: Move helper functions out of string.c Kees Cook
2021-07-27 20:58 ` [PATCH 30/64] fortify: Move remaining fortify helpers into fortify-string.h Kees Cook
2021-07-27 20:58 ` [PATCH 31/64] fortify: Explicitly disable Clang support Kees Cook
2021-07-27 21:18   ` Nathan Chancellor
2021-07-27 21:47     ` Kees Cook
2021-07-27 20:58 ` [PATCH 32/64] fortify: Add compile-time FORTIFY_SOURCE tests Kees Cook
2021-07-27 20:58 ` [PATCH 33/64] lib: Introduce CONFIG_TEST_MEMCPY Kees Cook
2021-07-27 23:31   ` Bart Van Assche
2021-07-27 23:33     ` Randy Dunlap
2021-07-28  1:30     ` Kees Cook
2021-07-27 20:58 ` [PATCH 34/64] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-07-27 22:43   ` Nick Desaulniers
2021-07-28  1:47     ` Kees Cook
2021-07-28 11:19   ` Rasmus Villemoes
2021-07-30  2:39     ` Kees Cook
2021-07-27 20:58 ` [PATCH 35/64] fortify: Detect struct member overflows in memmove() " Kees Cook
2021-07-27 20:58 ` [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp Kees Cook
2021-07-28  1:39   ` Martin K. Petersen
2021-07-28 18:57     ` Kees Cook
2021-07-29  3:35       ` Martin K. Petersen
2021-07-30 19:11         ` Tyrel Datwyler
2021-07-30 18:16     ` Tyrel Datwyler
2021-07-27 20:58 ` [PATCH 37/64] string.h: Introduce memset_after() for wiping trailing members/padding Kees Cook
2021-07-27 20:58 ` [PATCH 38/64] xfrm: Use memset_after() to clear padding Kees Cook
2021-07-27 20:58 ` [PATCH 39/64] mac80211: Use memset_after() to clear tx status Kees Cook
2021-07-31 15:55   ` Kees Cook
2021-08-13  7:40     ` Johannes Berg
2021-08-13 16:08       ` Kees Cook
2021-08-13 18:19         ` Johannes Berg
2021-08-13  7:41     ` Johannes Berg
2021-07-27 20:58 ` [PATCH 40/64] net: 802: Use memset_after() to clear struct fields Kees Cook
2021-07-27 20:58 ` [PATCH 41/64] net: dccp: Use memset_after() for TP zeroing Kees Cook
2021-07-27 20:58 ` [PATCH 42/64] net: qede: Use memset_after() for counters Kees Cook
2021-07-31 16:07   ` Kees Cook
2021-07-27 20:58 ` [PATCH 43/64] ath11k: Use memset_after() for clearing queue descriptors Kees Cook
2021-07-27 20:58 ` [PATCH 44/64] iw_cxgb4: Use memset_after() for cpl_t5_pass_accept_rpl Kees Cook
2021-07-27 20:58 ` [PATCH 45/64] intel_th: msu: Use memset_after() for clearing hw header Kees Cook
2021-07-27 20:58 ` [PATCH 46/64] IB/mthca: Use memset_after() for clearing mpt_entry Kees Cook
2021-07-27 20:58 ` [PATCH 47/64] btrfs: Use memset_after() to clear end of struct Kees Cook
2021-07-28  9:42   ` David Sterba
2021-07-28 21:56     ` Kees Cook
2021-07-29 10:33       ` David Sterba
2021-07-31 15:25         ` Kees Cook
2021-08-09 11:20           ` David Sterba
2021-07-27 20:58 ` [PATCH 48/64] drbd: Use struct_group() to zero algs Kees Cook
2021-07-28 21:45   ` Bart Van Assche
2021-07-30  2:31     ` Kees Cook
2021-07-30  2:57       ` Bart Van Assche
2021-07-30  9:25         ` Lars Ellenberg
2021-07-30 15:32       ` Nick Desaulniers
2021-07-27 20:58 ` [PATCH 49/64] cm4000_cs: Use struct_group() to zero struct cm4000_dev region Kees Cook
2021-07-28  5:48   ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 50/64] KVM: x86: Use struct_group() to zero decode cache Kees Cook
2021-07-27 20:58 ` [PATCH 51/64] tracing: Use struct_group() to zero struct trace_iterator Kees Cook
2021-07-27 20:58 ` [PATCH 52/64] dm integrity: Use struct_group() to zero struct journal_sector Kees Cook
2021-07-27 20:58 ` [PATCH 53/64] HID: roccat: Use struct_group() to zero kone_mouse_event Kees Cook
2021-07-27 20:58 ` [PATCH 54/64] ipv6: Use struct_group() to zero rt6_info Kees Cook
2021-07-29 18:58   ` Jakub Kicinski
2021-07-31 15:01     ` Kees Cook
2021-07-27 20:58 ` [PATCH 55/64] RDMA/mlx5: Use struct_group() to zero struct mlx5_ib_mr Kees Cook
2021-07-27 20:58 ` [PATCH 56/64] ethtool: stats: Use struct_group() to clear all stats at once Kees Cook
2021-07-27 20:58 ` [PATCH 57/64] netfilter: conntrack: Use struct_group() to zero struct nf_conn Kees Cook
2021-07-27 20:58 ` [PATCH 58/64] powerpc: Split memset() to avoid multi-field overflow Kees Cook
2021-08-05 11:36   ` Michael Ellerman
2021-07-27 20:58 ` [PATCH 59/64] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
2021-07-27 20:58 ` [PATCH 60/64] fortify: Work around Clang inlining bugs Kees Cook
2021-07-27 20:58 ` [PATCH 61/64] Makefile: Enable -Warray-bounds Kees Cook
2021-07-27 20:58 ` [PATCH 62/64] netlink: Avoid false-positive memcpy() warning Kees Cook
2021-07-28  5:49   ` Greg Kroah-Hartman
2021-07-28 11:24     ` Rasmus Villemoes
2021-07-30  1:39       ` Kees Cook
2021-07-30  1:41     ` Kees Cook
2021-07-27 20:58 ` [PATCH 63/64] iwlwifi: dbg_ini: Split memcpy() to avoid multi-field write Kees Cook
2021-07-27 20:58 ` [PATCH 64/64] fortify: Add run-time WARN for cross-field memcpy() Kees Cook

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=20210727205855.411487-22-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavoars@kernel.org \
    --cc=keithpac@amazon.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH 21/64] cxgb4: Use struct_group() for memcpy() region' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox