Similar to the previous checks, I looked for places where the value returned
by the vm_* functions was not checked.
I am not entirely certain that I am handling the error correctly in
libdiskfs/io-map-cntl.c simply unlocking the mutex and exiting the function
in the second case might not be sufficient.
Also, in the previous patch, pager_read_page simply returned err, even
though
only EIO, EDQUOT, and ENOSPC are permitted return values. I think it would
be worth adding a comment noting this to prevent similar errors in the
future.
Thanks,
--
Mikhail Karpov
From 3ad1e3674c27f3bc0d422af8d023fe02c809689d Mon Sep 17 00:00:00 2001
From: Mikhail Karpov <[email protected]>
Date: Thu, 3 Sep 2026 11:26:56 +0700
Subject: [PATCH 2/2] Added checks for vm_* return value in several place
---
boot/userland-boot.c | 47 ++++++++++++++++++++++++++++-------------
defpager/defpager.c | 8 +++++--
fatfs/dir.c | 11 ++++++----
fatfs/inode.c | 3 +++
isofs/lookup.c | 16 ++++++++++----
libdiskfs/io-map-cntl.c | 24 ++++++++++++++++-----
tmpfs/node.c | 6 +++---
trans/streamio.c | 6 +++++-
8 files changed, 87 insertions(+), 34 deletions(-)
diff --git a/boot/userland-boot.c b/boot/userland-boot.c
index 1739eb9..48cafd4 100644
--- a/boot/userland-boot.c
+++ b/boot/userland-boot.c
@@ -223,13 +223,19 @@ load_image (task_t t,
ph->p_vaddr &= ~(ph->p_align - 1);
ph->p_memsz -= ph->p_vaddr;
- vm_allocate (t, (vm_address_t*)&ph->p_vaddr, ph->p_memsz, 0);
- vm_write (t, ph->p_vaddr, buf, bufsz);
+ err = vm_allocate (t, (vm_address_t*)&ph->p_vaddr,
+ ph->p_memsz, 0);
+ assert_backtrace (err == KERN_SUCCESS);
+
+ err = vm_write (t, ph->p_vaddr, buf, bufsz);
+ assert_backtrace (err == KERN_SUCCESS);
+
munmap ((caddr_t) buf, bufsz);
- vm_protect (t, ph->p_vaddr, ph->p_memsz, 0,
- ((ph->p_flags & PF_R) ? VM_PROT_READ : 0) |
- ((ph->p_flags & PF_W) ? VM_PROT_WRITE : 0) |
- ((ph->p_flags & PF_X) ? VM_PROT_EXECUTE : 0));
+ err = vm_protect (t, ph->p_vaddr, ph->p_memsz, 0,
+ ((ph->p_flags & PF_R) ? VM_PROT_READ : 0) |
+ ((ph->p_flags & PF_W) ? VM_PROT_WRITE : 0) |
+ ((ph->p_flags & PF_X) ? VM_PROT_EXECUTE : 0));
+ assert_backtrace (err == KERN_SUCCESS);
}
return hdr.e.e_entry;
}
@@ -252,17 +258,25 @@ load_image (task_t t,
lseek (fd, sizeof hdr.a - headercruft, SEEK_SET);
err = read (fd, buf, amount);
assert_backtrace (err == amount);
- vm_allocate (t, &base, rndamount, 0);
- vm_write (t, base, (vm_address_t) buf, rndamount);
+ err = vm_allocate (t, &base, rndamount, 0);
+ assert_backtrace (err == KERN_SUCCESS);
+
+ err = vm_write (t, base, (vm_address_t) buf, rndamount);
+ assert_backtrace (err == KERN_SUCCESS);
+
if (magic != OMAGIC)
- vm_protect (t, base, trunc_page (headercruft + hdr.a.a_text),
- 0, VM_PROT_READ | VM_PROT_EXECUTE);
+ {
+ err = vm_protect (t, base, trunc_page (headercruft + hdr.a.a_text),
+ 0, VM_PROT_READ | VM_PROT_EXECUTE);
+ assert_backtrace (err == KERN_SUCCESS);
+ }
munmap ((caddr_t) buf, rndamount);
bssstart = base + hdr.a.a_text + hdr.a.a_data + headercruft;
bsspagestart = round_page (bssstart);
- vm_allocate (t, &bsspagestart,
- hdr.a.a_bss - (bsspagestart - bssstart), 0);
+ err = vm_allocate (t, &bsspagestart,
+ hdr.a.a_bss - (bsspagestart - bssstart), 0);
+ assert_backtrace (err == KERN_SUCCESS);
return hdr.a.a_entry;
}
@@ -313,7 +327,8 @@ boot_script_exec_cmd (void *hook,
arg_len += 5 * sizeof (intptr_t);
stack_end = VM_MAX_ADDRESS;
stack_start = VM_MAX_ADDRESS - 16 * 1024 * 1024;
- vm_allocate (task, &stack_start, stack_end - stack_start, FALSE);
+ err = vm_allocate (task, &stack_start, stack_end - stack_start, FALSE);
+ assert_backtrace (err == KERN_SUCCESS);
arg_pos = (void *) ((stack_end - arg_len) & ~(sizeof (intptr_t) - 1));
args = mmap (0, stack_end - trunc_page ((vm_offset_t) arg_pos),
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
@@ -334,8 +349,10 @@ boot_script_exec_cmd (void *hook,
p = (void *) p + sizeof (char *);
memcpy (p, strings, stringlen);
memset (args, 0, (vm_offset_t)arg_pos & (vm_page_size - 1));
- vm_write (task, trunc_page ((vm_offset_t) arg_pos), (vm_address_t) args,
- stack_end - trunc_page ((vm_offset_t) arg_pos));
+ err = vm_write (task, trunc_page ((vm_offset_t) arg_pos), (vm_address_t) args,
+ stack_end - trunc_page ((vm_offset_t) arg_pos));
+ assert_backtrace (err == KERN_SUCCESS);
+
munmap ((caddr_t) args,
stack_end - trunc_page ((vm_offset_t) arg_pos));
diff --git a/defpager/defpager.c b/defpager/defpager.c
index 02589a2..e34aec9 100644
--- a/defpager/defpager.c
+++ b/defpager/defpager.c
@@ -56,6 +56,10 @@ expand_map (struct user_pager_info *p, vm_offset_t addr)
return 0;
}
+/* The user must define this function. For pager PAGER, read one
+ page from offset PAGE. Set *BUF to be the address of the page,
+ and set *WRITE_LOCK if the page must be provided read-only.
+ The only permissible error returns are EIO, EDQUOT, and ENOSPC. */
error_t
pager_read_page (struct user_pager_info *pager,
vm_offset_t page,
@@ -70,13 +74,13 @@ pager_read_page (struct user_pager_info *pager,
error_t err = expand_map (pager, page);
if (err)
- return err;
+ return EIO;
if (!pager->map[pfn])
{
err = vm_allocate (mach_task_self (), buf, vm_page_size, 1);
if (err)
- return err;
+ return EIO;
}
else
{
diff --git a/fatfs/dir.c b/fatfs/dir.c
index 0351552..1b7bfe0 100644
--- a/fatfs/dir.c
+++ b/fatfs/dir.c
@@ -955,10 +955,13 @@ diskfs_get_directs (struct node *dp,
{
vm_address_t newdata;
- vm_allocate (mach_task_self (), &newdata,
- (ouralloc
- ? (allocsize *= 2)
- : (allocsize = vm_page_size * 2)), 1);
+ err = vm_allocate (mach_task_self (), &newdata,
+ (ouralloc
+ ? (allocsize *= 2)
+ : (allocsize = vm_page_size * 2)), 1);
+ if (err)
+ return err;
+
memcpy ((void *) newdata, (void *) *data, datap - *data);
if (ouralloc)
diff --git a/fatfs/inode.c b/fatfs/inode.c
index cefcba4..7134714 100644
--- a/fatfs/inode.c
+++ b/fatfs/inode.c
@@ -190,6 +190,9 @@ diskfs_user_read_node (struct node *np, struct lookup_context *ctx)
err = vm_map (mach_task_self (),
&buf, buflen, 0, 1, memobj, 0, 0, prot, prot, 0);
mach_port_deallocate (mach_task_self (), memobj);
+ if (err)
+ return err;
+
our_buf = 1;
}
diff --git a/isofs/lookup.c b/isofs/lookup.c
index 51eabcf..f12c62d 100644
--- a/isofs/lookup.c
+++ b/isofs/lookup.c
@@ -345,10 +345,18 @@ diskfs_get_directs (struct node *dp,
{
vm_address_t newdata;
- vm_allocate (mach_task_self (), &newdata,
- (ouralloc
- ? (allocsize *= 2)
- : (allocsize = vm_page_size * 2)), 1);
+ err = vm_allocate (mach_task_self (), &newdata,
+ (ouralloc
+ ? (allocsize *= 2)
+ : (allocsize = vm_page_size * 2)), 1);
+ if (err)
+ {
+ if (ouralloc)
+ munmap (*data, allocsize);
+
+ return err;
+ }
+
memcpy ((void *) newdata, (void *) *data, datap - *data);
if (ouralloc)
diff --git a/libdiskfs/io-map-cntl.c b/libdiskfs/io-map-cntl.c
index 6432fa0..981681f 100644
--- a/libdiskfs/io-map-cntl.c
+++ b/libdiskfs/io-map-cntl.c
@@ -32,11 +32,25 @@ diskfs_S_io_map_cntl (struct protid *cred,
pthread_mutex_lock (&cred->po->np->lock);
if (!cred->mapped)
{
- default_pager_object_create (diskfs_default_pager, &cred->shared_object,
- __vm_page_size);
- vm_map (mach_task_self (), (vm_address_t *)&cred->mapped, vm_page_size,
- 0, 1, cred->shared_object, 0, 0,
- VM_PROT_READ|VM_PROT_WRITE, VM_PROT_READ|VM_PROT_WRITE, 0);
+ error_t err = default_pager_object_create (diskfs_default_pager,
+ &cred->shared_object,
+ __vm_page_size);
+ if (err)
+ {
+ pthread_mutex_unlock (&cred->po->np->lock);
+ return err;
+ }
+
+ err = vm_map (mach_task_self (), (vm_address_t *)&cred->mapped,
+ vm_page_size, 0, 1, cred->shared_object, 0, 0,
+ VM_PROT_READ|VM_PROT_WRITE,
+ VM_PROT_READ|VM_PROT_WRITE, 0);
+ if (err)
+ {
+ pthread_mutex_unlock (&cred->po->np->lock);
+ return err;
+ }
+
cred->mapped->shared_page_magic = SHARED_PAGE_MAGIC;
cred->mapped->conch_status = USER_HAS_NOT_CONCH;
pthread_spin_init (&cred->mapped->lock, PTHREAD_PROCESS_PRIVATE);
diff --git a/tmpfs/node.c b/tmpfs/node.c
index 7689561..ff6c28e 100644
--- a/tmpfs/node.c
+++ b/tmpfs/node.c
@@ -557,9 +557,9 @@ diskfs_get_filemap (struct node *np, vm_prot_t prot)
/* XXX we need to keep a reference to the object, or GNU Mach
will terminate it when we release the map. */
np->dn->u.reg.memref = 0;
- vm_map (mach_task_self (), &np->dn->u.reg.memref, 4096, 0, 1,
- np->dn->u.reg.memobj, 0, 0, VM_PROT_NONE, VM_PROT_NONE,
- VM_INHERIT_NONE);
+ err = vm_map (mach_task_self (), &np->dn->u.reg.memref, 4096, 0, 1,
+ np->dn->u.reg.memobj, 0, 0, VM_PROT_NONE, VM_PROT_NONE,
+ VM_INHERIT_NONE);
assert_perror_backtrace (err);
}
diff --git a/trans/streamio.c b/trans/streamio.c
index f7cd442..694da06 100644
--- a/trans/streamio.c
+++ b/trans/streamio.c
@@ -1046,7 +1046,11 @@ dev_read (size_t amount, void **buf, size_t *len, int nowait)
avail = buffer_size (input_buffer);
max = (amount < avail) ? amount : avail;
if (max > *len)
- vm_allocate (mach_task_self (), (vm_address_t *)buf, max, 1);
+ {
+ err = vm_allocate (mach_task_self (), (vm_address_t *)buf, max, 1);
+ if (err)
+ return err;
+ }
*len = buffer_read (input_buffer, *buf, max);
assert_backtrace (*len == max);
--
2.43.0
