[PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string false-positive

From: Arnd Bergmann
Date: Fri Feb 16 2024 - 15:27:30 EST


From: Arnd Bergmann <arnd@xxxxxxxx>

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@xxxxxxxx>
---
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