Messages in this thread | | | Date | Fri, 16 Feb 2024 13:42:28 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string false-positive |
| |
Hi Arnd,
On Fri, Feb 16, 2024 at 09:26:40PM +0100, Arnd Bergmann wrote: > 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.
I have some observations and notes from my initial investigation into this issue on our GitHub issue tracker but I have not produced a minimized test case either.
https://github.com/ClangBuiltLinux/linux/issues/1985
> 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 >
| |