LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* RE: [PATCH v2 17/63] bnx2x: Use struct_group() for memcpy() region
@ 2021-08-24 13:51 Prabhakar Kushwaha
  0 siblings, 0 replies; 2+ messages in thread
From: Prabhakar Kushwaha @ 2021-08-24 13:51 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: Ariel Elior, Sudarsana Reddy Kalluru, GR-everest-linux-l2,
	David S. Miller, Jakub Kicinski, netdev, Gustavo A. R. Silva,
	Shai Malin, Greg Kroah-Hartman, Andrew Morton, linux-wireless,
	dri-devel, linux-staging, linux-block, linux-kbuild,
	clang-built-linux, Rasmus Villemoes, linux-hardening


> -----Original Message-----
> From: Kees Cook <keescook@chromium.org>
> Sent: Wednesday, August 18, 2021 11:35 AM
> To: linux-kernel@vger.kernel.org
> Cc: Kees Cook <keescook@chromium.org>; Ariel Elior <aelior@marvell.com>;
> Sudarsana Reddy Kalluru <skalluru@marvell.com>; GR-everest-linux-l2 <GR-
> everest-linux-l2@marvell.com>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; netdev@vger.kernel.org; Gustavo A. R. Silva
> <gustavoars@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Andrew Morton <akpm@linux-foundation.org>; linux-wireless@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; Rasmus Villemoes <linux@rasmusvillemoes.dk>;
> linux-hardening@vger.kernel.org
> Subject: [PATCH v2 17/63] bnx2x: Use struct_group() for memcpy() region
> 
> 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 nig_stats around members egress_mac_pkt0_lo,
> egress_mac_pkt0_hi, egress_mac_pkt1_lo, and egress_mac_pkt1_hi (and the
> respective members in struct bnx2x_eth_stats), 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 struct bnx2x_eth_stats's rx_stat_ifhcinbadoctets_hi.
> 
> "pahole" shows no size nor member offset changes to either struct.
> "objdump -d" shows no meaningful object code changes (i.e. only source
> line number induced differences and optimizations).
> 
> Additionally adds BUILD_BUG_ON() to compare the separate struct group
> sizes.
> 
> Cc: Ariel Elior <aelior@marvell.com>
> Cc: Sudarsana Kalluru <skalluru@marvell.com>
> Cc: GR-everest-linux-l2@marvell.com
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---

Reviewed-by: Prabhakar Kushwaha <pkushwaha@marvell.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH v2 17/63] bnx2x: Use struct_group() for memcpy() region
  2021-08-18  6:04 [PATCH v2 00/63] Introduce strict memcpy() bounds checking Kees Cook
@ 2021-08-18  6:04 ` Kees Cook
  0 siblings, 0 replies; 2+ messages in thread
From: Kees Cook @ 2021-08-18  6:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Ariel Elior, Sudarsana Kalluru, GR-everest-linux-l2,
	David S. Miller, Jakub Kicinski, netdev, Gustavo A. R. Silva,
	Greg Kroah-Hartman, Andrew Morton, linux-wireless, dri-devel,
	linux-staging, linux-block, linux-kbuild, clang-built-linux,
	Rasmus Villemoes, linux-hardening

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 nig_stats around members egress_mac_pkt0_lo,
egress_mac_pkt0_hi, egress_mac_pkt1_lo, and egress_mac_pkt1_hi (and the
respective members in struct bnx2x_eth_stats), 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 struct bnx2x_eth_stats's rx_stat_ifhcinbadoctets_hi.

"pahole" shows no size nor member offset changes to either struct.
"objdump -d" shows no meaningful object code changes (i.e. only source
line number induced differences and optimizations).

Additionally adds BUILD_BUG_ON() to compare the separate struct group
sizes.

Cc: Ariel Elior <aelior@marvell.com>
Cc: Sudarsana Kalluru <skalluru@marvell.com>
Cc: GR-everest-linux-l2@marvell.com
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c |  7 ++++---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h | 14 ++++++++++----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index 0b193edb73b8..2bb133ae61c3 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -849,7 +849,8 @@ static int bnx2x_hw_stats_update(struct bnx2x *bp)
 
 	memcpy(old, new, sizeof(struct nig_stats));
 
-	memcpy(&(estats->rx_stat_ifhcinbadoctets_hi), &(pstats->mac_stx[1]),
+	BUILD_BUG_ON(sizeof(estats->shared) != sizeof(pstats->mac_stx[1]));
+	memcpy(&(estats->shared), &(pstats->mac_stx[1]),
 	       sizeof(struct mac_stx));
 	estats->brb_drop_hi = pstats->brb_drop_hi;
 	estats->brb_drop_lo = pstats->brb_drop_lo;
@@ -1634,9 +1635,9 @@ void bnx2x_stats_init(struct bnx2x *bp)
 			REG_RD(bp, NIG_REG_STAT0_BRB_TRUNCATE + port*0x38);
 	if (!CHIP_IS_E3(bp)) {
 		REG_RD_DMAE(bp, NIG_REG_STAT0_EGRESS_MAC_PKT0 + port*0x50,
-			    &(bp->port.old_nig_stats.egress_mac_pkt0_lo), 2);
+			    &(bp->port.old_nig_stats.egress_mac_pkt0), 2);
 		REG_RD_DMAE(bp, NIG_REG_STAT0_EGRESS_MAC_PKT1 + port*0x50,
-			    &(bp->port.old_nig_stats.egress_mac_pkt1_lo), 2);
+			    &(bp->port.old_nig_stats.egress_mac_pkt1), 2);
 	}
 
 	/* Prepare statistics ramrod data */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
index d55e63692cf3..ae93c078707b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
@@ -36,10 +36,14 @@ struct nig_stats {
 	u32 pbf_octets;
 	u32 pbf_packet;
 	u32 safc_inp;
-	u32 egress_mac_pkt0_lo;
-	u32 egress_mac_pkt0_hi;
-	u32 egress_mac_pkt1_lo;
-	u32 egress_mac_pkt1_hi;
+	struct_group(egress_mac_pkt0,
+		u32 egress_mac_pkt0_lo;
+		u32 egress_mac_pkt0_hi;
+	);
+	struct_group(egress_mac_pkt1,
+		u32 egress_mac_pkt1_lo;
+		u32 egress_mac_pkt1_hi;
+	);
 };
 
 enum bnx2x_stats_event {
@@ -83,6 +87,7 @@ struct bnx2x_eth_stats {
 	u32 no_buff_discard_hi;
 	u32 no_buff_discard_lo;
 
+	struct_group(shared,
 	u32 rx_stat_ifhcinbadoctets_hi;
 	u32 rx_stat_ifhcinbadoctets_lo;
 	u32 tx_stat_ifhcoutbadoctets_hi;
@@ -159,6 +164,7 @@ struct bnx2x_eth_stats {
 	u32 tx_stat_dot3statsinternalmactransmiterrors_lo;
 	u32 tx_stat_bmac_ufl_hi;
 	u32 tx_stat_bmac_ufl_lo;
+	);
 
 	u32 pause_frames_received_hi;
 	u32 pause_frames_received_lo;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-08-24 13:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 13:51 [PATCH v2 17/63] bnx2x: Use struct_group() for memcpy() region Prabhakar Kushwaha
  -- strict thread matches above, loose matches on Subject: below --
2021-08-18  6:04 [PATCH v2 00/63] Introduce strict memcpy() bounds checking Kees Cook
2021-08-18  6:04 ` [PATCH v2 17/63] bnx2x: Use struct_group() for memcpy() region Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).