Re: [PATCH net] xfrm_user: fix info leak in build_mapping()
From: Jakub Kicinski
Date: Mon Apr 06 2026 - 11:55:20 EST
On Mon, 6 Apr 2026 17:33:03 +0200 Greg Kroah-Hartman wrote:
> struct xfrm_usersa_id has a one-byte padding hole after the proto
> field, which ends up never getting set to zero before copying out to
> userspace. Fix that up by zeroing out the whole structure before
> setting individual variables.
>
> Fixes: 3a2dfbe8acb1 ("xfrm: Notify changes in UDP encapsulation via netlink")
> Cc: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
> Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: Simon Horman <horms@xxxxxxxxxx>
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> Note, I think this is correct, as I don't think a new skb has it's
> fields pre-zeroed out, or am I totally wrong here?
You're right, skb owner is responsible for clearing after put.
Tho, Netlink is not as perf critical as real networking, I wish
we at least had a helper which reserves the space and clears it :/
This is not the first or the second time we hit this sort of a bug.
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 8a854fa9567d..1bb8d05561df 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -4165,6 +4165,7 @@ static int build_mapping(struct sk_buff *skb, struct xfrm_state *x,
>
> um = nlmsg_data(nlh);
>
> + memset(&um->id, 0, sizeof(um->id));
> memcpy(&um->id.daddr, &x->id.daddr, sizeof(um->id.daddr));
> um->id.spi = x->id.spi;
> um->id.family = x->props.family;