linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josef Oskera <joskera@redhat.com>
To: netdev@vger.kernel.org
Cc: Josef Oskera <joskera@redhat.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Kees Cook <keescook@chromium.org>,
	linux-rdma@vger.kernel.org (open list:MELLANOX MLX4 core VPI
	driver), linux-kernel@vger.kernel.org (open list)
Subject: [PATCH net] mlx4: supress fortify for inlined xmit
Date: Fri, 17 Feb 2023 10:45:41 +0100	[thread overview]
Message-ID: <20230217094541.2362873-1-joskera@redhat.com> (raw)

This call "skb_copy_from_linear_data(skb, inl + 1, spc)" triggers FORTIFY memcpy()
warning on ppc64 platform.

In function ‘fortify_memcpy_chk’,
    inlined from ‘skb_copy_from_linear_data’ at ./include/linux/skbuff.h:4029:2,
    inlined from ‘build_inline_wqe’ at drivers/net/ethernet/mellanox/mlx4/en_tx.c:722:4,
    inlined from ‘mlx4_en_xmit’ at drivers/net/ethernet/mellanox/mlx4/en_tx.c:1066:3:
./include/linux/fortify-string.h:513:25: error: call to ‘__write_overflow_field’ declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  513 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Same behaviour on x86 you can get if you use "__always_inline" instead of
"inline" for skb_copy_from_linear_data() in skbuff.h

The call here copies data into inlined tx destricptor, which has 104 bytes
(MAX_INLINE) space for data payload. In this case "spc" is known in compile-time
but the destination is used with hidden knowledge (real structure of destination
is different from that the compiler can see). That cause the fortify warning
because compiler can check bounds, but the real bounds are different.
"spc" can't be bigger than 64 bytes (MLX4_INLINE_ALIGN), so the data can always
fit into inlined tx descriptor.
The fact that "inl" points into inlined tx descriptor is determined earlier
in mlx4_en_xmit().

Fixes: f68f2ff91512c1 fortify: Detect struct member overflows in memcpy() at compile-time
Signed-off-by: Josef Oskera <joskera@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index c5758637b7bed6..f30ca9fe90e5b4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -719,7 +719,16 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc,
 			inl = (void *) (inl + 1) + spc;
 			memcpy(((void *)(inl + 1)), fragptr, skb->len - spc);
 		} else {
-			skb_copy_from_linear_data(skb, inl + 1, spc);
+			unsafe_memcpy(inl + 1, skb->data, spc,
+					/* This copies data into inlined tx descriptor, which has
+					 * 104 bytes (MAX_INLINE) space for data.
+					 * Real structure of destination is in this case hidden for
+					 * the compiler
+					 * "spc" is compile-time known variable and can't be bigger
+					 * than 64 (MLX4_INLINE_ALIGN).
+					 * Bounds and other conditions are checked in current
+					 * function and earlier in mlx4_en_xmit()
+					 */);
 			inl = (void *) (inl + 1) + spc;
 			skb_copy_from_linear_data_offset(skb, spc, inl + 1,
 							 hlen - spc);
-- 
2.39.0


             reply	other threads:[~2023-02-17  9:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-17  9:45 Josef Oskera [this message]
2023-02-18 16:26 ` [PATCH net] mlx4: supress fortify for inlined xmit Kees Cook
2023-02-19  9:16   ` Tariq Toukan
2023-02-20  8:30     ` Tariq Toukan

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=20230217094541.2362873-1-joskera@redhat.com \
    --to=joskera@redhat.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tariqt@nvidia.com \
    /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;
as well as URLs for NNTP newsgroup(s).