netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Steffen Klassert <steffen.klassert@secunet.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Leon Romanovsky <leon@kernel.org>, Lin Ma <linma@zju.edu.cn>,
	Simon Horman <horms@kernel.org>, Breno Leitao <leitao@debian.org>,
	Tobias Brunner <tobias@strongswan.org>,
	Raed Salem <raeds@nvidia.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: [PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string false-positive
Date: Fri, 16 Feb 2024 21:26:40 +0100	[thread overview]
Message-ID: <20240216202657.2493685-1-arnd@kernel.org> (raw)

From: Arnd Bergmann <arnd@arndb.de>

clang-19 recently got branched from clang-18 and is not yet released.
The current version introduces exactly one new warning that I came
across in randconfig testing, in the copy_to_user_tmpl() function:

include/linux/fortify-string.h:420:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
  420 |                         __write_overflow_field(p_size_field, size);

I have not yet produced a minimized test case for it, but I have a
local workaround, which avoids the memset() here by replacing it with
an initializer.

The memset is required to avoid leaking stack data to user space
and was added in commit 1f86840f8977 ("xfrm_user: fix info leak in
copy_to_user_tmpl()"). Simply changing the initializer to set all fields
still risks leaking data in the padding between them, which the compiler
is free to do here. To work around that problem, explicit padding fields
have to get added as well.

My first idea was that just adding the padding would avoid the warning
as well, as the padding tends to confused the fortified string helpers,
but it turns out that both changes are required here.

Since this is a false positive, a better fix would likely be to
fix the compiler.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/uapi/linux/xfrm.h | 3 +++
 net/xfrm/xfrm_user.c      | 3 +--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index 6a77328be114..99adac4fa648 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -27,6 +27,7 @@ struct xfrm_id {
 	xfrm_address_t	daddr;
 	__be32		spi;
 	__u8		proto;
+	__u8		__pad[3];
 };
 
 struct xfrm_sec_ctx {
@@ -242,11 +243,13 @@ struct xfrm_user_sec_ctx {
 struct xfrm_user_tmpl {
 	struct xfrm_id		id;
 	__u16			family;
+	__u16			__pad1;
 	xfrm_address_t		saddr;
 	__u32			reqid;
 	__u8			mode;
 	__u8			share;
 	__u8			optional;
+	__u8			__pad2;
 	__u32			aalgos;
 	__u32			ealgos;
 	__u32			calgos;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a5232dcfea46..e81f977e183c 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2011,7 +2011,7 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 static int copy_to_user_tmpl(struct xfrm_policy *xp, struct sk_buff *skb)
 {
-	struct xfrm_user_tmpl vec[XFRM_MAX_DEPTH];
+	struct xfrm_user_tmpl vec[XFRM_MAX_DEPTH] = {};
 	int i;
 
 	if (xp->xfrm_nr == 0)
@@ -2021,7 +2021,6 @@ static int copy_to_user_tmpl(struct xfrm_policy *xp, struct sk_buff *skb)
 		struct xfrm_user_tmpl *up = &vec[i];
 		struct xfrm_tmpl *kp = &xp->xfrm_vec[i];
 
-		memset(up, 0, sizeof(*up));
 		memcpy(&up->id, &kp->id, sizeof(up->id));
 		up->family = kp->encap_family;
 		memcpy(&up->saddr, &kp->saddr, sizeof(up->saddr));
-- 
2.39.2


             reply	other threads:[~2024-02-16 20:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-16 20:26 Arnd Bergmann [this message]
2024-02-16 20:42 ` [PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string false-positive Nathan Chancellor
2024-02-16 21:19 ` Kees Cook
2024-04-08  7:06   ` Arnd Bergmann
2024-04-09 16:15     ` Nathan Chancellor
2024-04-09 19:41       ` Arnd Bergmann
2024-04-10 17:45         ` Nathan Chancellor
2024-04-11 11:35           ` Arnd Bergmann
2024-04-12 21:21             ` Nathan Chancellor

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=20240216202657.2493685-1-arnd@kernel.org \
    --to=arnd@kernel.org \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gustavoars@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=horms@kernel.org \
    --cc=justinstitt@google.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=leon@kernel.org \
    --cc=linma@zju.edu.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=raeds@nvidia.com \
    --cc=steffen.klassert@secunet.com \
    --cc=tobias@strongswan.org \
    /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).