Hello,
I've corrected the parts you pointed out. There's just one thing that
bothers me:
On Tue, 23 Jun 2026 02:20:15 Samuel Thibault wrote:
> Since we can't return an error code, better use assert_backtrace to
> crash.
>
> > + }
> > + }
> > else
> > - buf = alloca (qr->amount);
> > + {
> > + buf = alloca (qr->amount);
> > + if (!buf)
> > + {
> > + pthread_spin_unlock (&readlock);
> > + return;
>
> Same here.
It turns out that alloca doesn't return NULL if it fails, so this check
is pointless. But how safe is this function to use?
Thanks,
--
Mikhail Karpov
From 60596d3ebd97ab734cbe83eeeb1d0e1259d08748 Mon Sep 17 00:00:00 2001
From: Mikhail Karpov <[email protected]>
Date: Fri, 26 Jun 2026 15:49:28 +0700
Subject: [PATCH] Added checks for mmap return value in several places
---
boot/boot.c | 26 ++++++++++++++--
boot/userland-boot.c | 3 ++
console/pager.c | 12 ++++++--
exec/exec.c | 15 +++++++++
fatfs/dir.c | 7 ++++-
fatfs/main.c | 1 +
isofs/lookup.c | 7 ++++-
isofs/pager.c | 8 +++--
libmachdev/trivfs_server.c | 2 ++
libnetfs/file-get-translator.c | 56 +++++++++++++++++++++++++++++++---
libnetfs/io-read.c | 9 +++++-
lwip/pfinet-ops.c | 10 ++++--
lwip/socket-ops.c | 10 ++++--
nfs/ops.c | 12 +++++++-
pfinet/socket-ops.c | 11 +++++--
pflocal/pf.c | 9 +++++-
term/ptyio.c | 12 +++++++-
term/users.c | 11 ++++++-
trans/magic.c | 7 +++--
19 files changed, 200 insertions(+), 28 deletions(-)
diff --git a/boot/boot.c b/boot/boot.c
index 70a82f06..54feb333 100644
--- a/boot/boot.c
+++ b/boot/boot.c
@@ -898,7 +898,10 @@ read_reply (void)
pthread_spin_unlock (&queuelock);
if (qr->type == DEV_READ)
- buf = mmap (0, qr->amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ buf = mmap (0, qr->amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ assert_backtrace (buf != MAP_FAILED);
+ }
else
buf = alloca (qr->amount);
amtread = read (0, buf, qr->amount);
@@ -1138,7 +1141,14 @@ ds_device_read (device_t device,
ioctl (0, FIONREAD, &avail);
if (avail)
{
- *data = mmap (0, bytes_wanted, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ void *new_data = mmap (0, bytes_wanted, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ unlock_readlock ();
+ return errno;
+ }
+ *data = new_data;
*datalen = read (0, *data, bytes_wanted);
unlock_readlock ();
return (*datalen == -1 ? D_IO_ERROR : D_SUCCESS);
@@ -1484,7 +1494,17 @@ S_io_read (mach_port_t object,
{
data_t orig_data = *data;
if (amount > *datalen)
- *data = mmap (0, amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *new_data = mmap (0, amount, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ unlock_readlock();
+ return errno;
+ }
+
+ *data = new_data;
+ }
*datalen = read (0, *data, amount);
if (*datalen == -1 && *data != orig_data)
munmap (*data, amount);
diff --git a/boot/userland-boot.c b/boot/userland-boot.c
index 4bbfbbb6..1739eb91 100644
--- a/boot/userland-boot.c
+++ b/boot/userland-boot.c
@@ -212,6 +212,7 @@ load_image (task_t t,
buf = (vm_address_t) mmap (0, bufsz,
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ assert_backtrace (buf != MAP_FAILED);
lseek (fd, ph->p_offset, SEEK_SET);
err = read (fd, (void *)(buf + offs), ph->p_filesz);
@@ -247,6 +248,7 @@ load_image (task_t t,
amount = headercruft + hdr.a.a_text + hdr.a.a_data;
rndamount = round_page (amount);
buf = mmap (0, rndamount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ assert_backtrace (buf != MAP_FAILED);
lseek (fd, sizeof hdr.a - headercruft, SEEK_SET);
err = read (fd, buf, amount);
assert_backtrace (err == amount);
@@ -315,6 +317,7 @@ boot_script_exec_cmd (void *hook,
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);
+ assert_backtrace (args != MAP_FAILED);
str_start = ((vm_address_t) arg_pos
+ (argc + 2) * sizeof (char *) + sizeof (intptr_t));
p = args + ((vm_address_t) arg_pos & (vm_page_size - 1));
diff --git a/console/pager.c b/console/pager.c
index a18d4d13..a2c90697 100644
--- a/console/pager.c
+++ b/console/pager.c
@@ -68,10 +68,16 @@ pager_read_page (struct user_pager_info *upi, vm_offset_t page,
{
*buf = upi->memobj_pages[page / vm_page_size];
upi->memobj_pages[page / vm_page_size] = (vm_address_t) NULL;
+
+ return 0;
}
- else
- *buf = (vm_address_t) mmap (0, vm_page_size, PROT_READ|PROT_WRITE,
- MAP_ANON, 0, 0);
+
+ void *new_buf = mmap (0, vm_page_size, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_buf == MAP_FAILED)
+ return errno;
+
+ *buf = (vm_address_t) new_buf;
return 0;
}
diff --git a/exec/exec.c b/exec/exec.c
index 4d7bada6..c0fa86f6 100644
--- a/exec/exec.c
+++ b/exec/exec.c
@@ -1003,6 +1003,11 @@ do_exec (file_t file,
new_envplen = sizeof (ld_origin_s) - 1 + pathlen + envplen;
new_envp = mmap (0, new_envplen,
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (new_envp == MAP_FAILED)
+ {
+ e.error = errno;
+ goto stdout;
+ }
memcpy (new_envp, ld_origin_s + 1, sizeof (ld_origin_s) - 2);
memcpy (new_envp + sizeof (ld_origin_s) - 2, abspath, pathlen);
@@ -1044,6 +1049,11 @@ do_exec (file_t file,
/* Allocate a new vector that is big enough. */
boot->intarray = mmap (0, INIT_INT_MAX * sizeof (int),
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (boot->intarray == MAP_FAILED)
+ {
+ e.error = errno;
+ goto stdout;
+ }
memcpy (boot->intarray, intarray, nints * sizeof (int));
intarray_dealloc = !intarray_copy;
}
@@ -1069,6 +1079,11 @@ do_exec (file_t file,
boot->nports = nports < INIT_PORT_MAX ? INIT_PORT_MAX : nports;
boot->portarray = mmap (0, boot->nports * sizeof (mach_port_t),
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (boot->portarray == MAP_FAILED)
+ {
+ e.error = errno;
+ goto stdout;
+ }
/* Start by copying the array as passed. */
for (i = 0; i < nports; ++i)
boot->portarray[i] = portarray[i];
diff --git a/fatfs/dir.c b/fatfs/dir.c
index dbe243c6..03515528 100644
--- a/fatfs/dir.c
+++ b/fatfs/dir.c
@@ -854,7 +854,12 @@ diskfs_get_directs (struct node *dp,
allocsize = bufsiz ? round_page (bufsiz) : vm_page_size * 4;
if (allocsize > *datacnt)
{
- *data = mmap (0, allocsize, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ void *new_data = mmap (0, allocsize, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ return errno;
+
+ *data = new_data;
ouralloc = 1;
}
diff --git a/fatfs/main.c b/fatfs/main.c
index da6aa949..8f44ed8b 100644
--- a/fatfs/main.c
+++ b/fatfs/main.c
@@ -235,6 +235,7 @@ main (int argc, char **argv)
zerocluster = (vm_address_t) mmap (0, bytes_per_cluster, PROT_READ|PROT_WRITE,
MAP_ANON, 0, 0);
+ assert_backtrace (zerocluster != MAP_FAILED);
fetch_root ();
diff --git a/isofs/lookup.c b/isofs/lookup.c
index d9322fac..51eabcfb 100644
--- a/isofs/lookup.c
+++ b/isofs/lookup.c
@@ -244,7 +244,12 @@ diskfs_get_directs (struct node *dp,
allocsize = bufsiz ? round_page (bufsiz) : vm_page_size * 4;
if (allocsize > *datacnt)
{
- *data = mmap (0, allocsize, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ void *new_data = mmap (0, allocsize, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ return errno;
+
+ *data = new_data;
ouralloc = 1;
}
diff --git a/isofs/pager.c b/isofs/pager.c
index 7838bdfc..621214c9 100644
--- a/isofs/pager.c
+++ b/isofs/pager.c
@@ -54,8 +54,12 @@ pager_read_page (struct user_pager_info *upi,
if (page >= np->dn_stat.st_size)
{
- *buf = (vm_address_t) mmap (0, vm_page_size, PROT_READ|PROT_WRITE,
- MAP_ANON, 0, 0);
+ void *new_buf = mmap (0, vm_page_size, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_buf == MAP_FAILED)
+ return errno;
+
+ *buf = (vm_address_t) new_buf;
return 0;
}
diff --git a/libmachdev/trivfs_server.c b/libmachdev/trivfs_server.c
index 2c905a63..5f8ccc88 100644
--- a/libmachdev/trivfs_server.c
+++ b/libmachdev/trivfs_server.c
@@ -294,6 +294,8 @@ trivfs_S_fsys_init (struct trivfs_control *fsys,
portarray = mmap (0, INIT_PORT_MAX * sizeof *portarray,
PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ assert_backtrace (portarray != MAP_FAILED);
+
for (i = 0; i < INIT_PORT_MAX; ++i)
portarray[i] = MACH_PORT_NULL;
portarray[INIT_PORT_PROC] = procserver;
diff --git a/libnetfs/file-get-translator.c b/libnetfs/file-get-translator.c
index 63c74c0b..d6984c63 100644
--- a/libnetfs/file-get-translator.c
+++ b/libnetfs/file-get-translator.c
@@ -55,7 +55,16 @@ netfs_S_file_get_translator (struct protid *user,
if (!err)
{
if (len > *translen)
- *trans = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
+
+ *trans = buf;
+ }
memcpy (*trans, string, len);
*translen = len;
free (string);
@@ -66,7 +75,16 @@ netfs_S_file_get_translator (struct protid *user,
unsigned int len = sizeof _HURD_SYMLINK + np->nn_stat.st_size + 1;
if (len > *translen)
- *trans = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
+
+ *trans = buf;
+ }
memcpy (*trans, _HURD_SYMLINK, sizeof _HURD_SYMLINK);
err = netfs_attempt_readlink (user->user, np,
@@ -98,7 +116,17 @@ netfs_S_file_get_translator (struct protid *user,
buflen++; /* terminating nul */
if (buflen > *translen)
- *trans = mmap (0, buflen, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, buflen, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
+
+ *trans = buf;
+ }
memcpy (*trans, buf, buflen);
free (buf);
*translen = buflen;
@@ -111,7 +139,16 @@ netfs_S_file_get_translator (struct protid *user,
len = sizeof _HURD_FIFO;
if (len > *translen)
- *trans = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
+
+ *trans = buf;
+ }
memcpy (*trans, _HURD_FIFO, sizeof _HURD_FIFO);
*translen = len;
err = 0;
@@ -122,7 +159,16 @@ netfs_S_file_get_translator (struct protid *user,
len = sizeof _HURD_IFSOCK;
if (len > *translen)
- *trans = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
+
+ *trans = buf;
+ }
memcpy (*trans, _HURD_IFSOCK, sizeof _HURD_IFSOCK);
*translen = len;
err = 0;
diff --git a/libnetfs/io-read.c b/libnetfs/io-read.c
index 96b003b3..16abee56 100644
--- a/libnetfs/io-read.c
+++ b/libnetfs/io-read.c
@@ -50,8 +50,15 @@ netfs_S_io_read (struct protid *user,
if (amount > data_size)
{
+ void *new_data = mmap (0, amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&node->lock);
+ return errno;
+ }
+
alloced = 1;
- *data = mmap (0, amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ *data = new_data;
}
data_size = amount;
diff --git a/lwip/pfinet-ops.c b/lwip/pfinet-ops.c
index 93373e51..e87cfe0d 100644
--- a/lwip/pfinet-ops.c
+++ b/lwip/pfinet-ops.c
@@ -102,8 +102,14 @@ lwip_S_pfinet_siocgifconf (io_t port,
{
/* Possibly allocate a new buffer */
if (*len < amount)
- ifc.ifc_buf = (char *) mmap (0, amount, PROT_READ | PROT_WRITE,
- MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, amount, PROT_READ | PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ return ENOMEM;
+
+ ifc.ifc_buf = buf;
+ }
else
ifc.ifc_buf = *ifr;
diff --git a/lwip/socket-ops.c b/lwip/socket-ops.c
index 0d8eaee6..55f5a606 100644
--- a/lwip/socket-ops.c
+++ b/lwip/socket-ops.c
@@ -287,8 +287,14 @@ lwip_S_socket_whatis_address (struct sock_addr * addr,
*type = addr->address.sa.sa_family;
if (*datalen < addr->address.sa.sa_len)
- *data = mmap (0, addr->address.sa.sa_len,
- PROT_READ | PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, addr->address.sa.sa_len, PROT_READ | PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ return ENOMEM;
+
+ *data = buf;
+ }
*datalen = addr->address.sa.sa_len;
memcpy (*data, &addr->address.sa, addr->address.sa.sa_len);
diff --git a/nfs/ops.c b/nfs/ops.c
index 95e8b11c..affdd931 100644
--- a/nfs/ops.c
+++ b/nfs/ops.c
@@ -2022,7 +2022,17 @@ netfs_get_dirents (struct iouser *cred, struct node *np,
else
allocsize = round_page (bufsiz);
if (allocsize > *datacnt)
- *data = mmap (0, allocsize, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *new_data = mmap (0, allocsize, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ free (buf);
+ return errno;
+ }
+
+ *data = new_data;
+ }
/* Skip ahead to the correct entry. */
bp = buf;
diff --git a/pfinet/socket-ops.c b/pfinet/socket-ops.c
index ce10682b..ff2e82bb 100644
--- a/pfinet/socket-ops.c
+++ b/pfinet/socket-ops.c
@@ -345,8 +345,15 @@ S_socket_whatis_address (struct sock_addr *addr,
*type = addr->address.sa_family;
if (*datalen < addr->address.sa_len)
- *data = mmap (0, addr->address.sa_len,
- PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *new_data = mmap (0, addr->address.sa_len, PROT_READ|PROT_WRITE,
+ MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ return ENOMEM;
+
+ *data = new_data;
+ }
+
*datalen = addr->address.sa_len;
memcpy (*data, &addr->address, addr->address.sa_len);
diff --git a/pflocal/pf.c b/pflocal/pf.c
index 457ebfbd..d69cedc8 100644
--- a/pflocal/pf.c
+++ b/pflocal/pf.c
@@ -141,7 +141,14 @@ S_socket_whatis_address (struct addr *addr,
*sockaddr_type = AF_LOCAL;
if (*sockaddr_len < addr_len)
- *sockaddr = mmap (0, addr_len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *buf = mmap (0, addr_len, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ return errno;
+
+ *sockaddr = buf;
+ }
+
((struct sockaddr *) *sockaddr)->sa_len = addr_len;
((struct sockaddr *) *sockaddr)->sa_family = *sockaddr_type;
((struct sockaddr *) *sockaddr)->sa_data[0] = 0;
diff --git a/term/ptyio.c b/term/ptyio.c
index 73a99e24..859d18be 100644
--- a/term/ptyio.c
+++ b/term/ptyio.c
@@ -353,7 +353,17 @@ pty_io_read (struct trivfs_protid *cred,
if (size > amount)
size = amount;
if (size > *datalen)
- *data = mmap (0, size, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *new_data = mmap (0, size, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&global_lock);
+ return errno;
+ }
+
+ *data = new_data;
+ }
+
*datalen = size;
if (control_byte)
diff --git a/term/users.c b/term/users.c
index bb5a9e09..629534ff 100644
--- a/term/users.c
+++ b/term/users.c
@@ -721,7 +721,16 @@ trivfs_S_io_read (struct trivfs_protid *cred,
max = (amount < avail) ? amount : avail;
if (max > *datalen)
- *data = mmap (0, max, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ {
+ void *new_data = mmap (0, max, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
+ if (new_data == MAP_FAILED)
+ {
+ pthread_mutex_unlock (&global_lock);
+ return errno;
+ }
+
+ *data = new_data;
+ }
cancel = 0;
cp = *data;
diff --git a/trans/magic.c b/trans/magic.c
index 7c809fbc..73fdaea3 100644
--- a/trans/magic.c
+++ b/trans/magic.c
@@ -424,9 +424,12 @@ parse_opt (int opt, char *arg, struct argp_state *state)
& ~(__alignof (struct dirent) - 1));
return (struct dirent *) ((char *) d + d->d_reclen);
}
- struct dirent *d;
- m->dirbuf = mmap (0, getpagesize (), PROT_READ|PROT_WRITE,
+ void *buf = mmap (0, getpagesize (), PROT_READ|PROT_WRITE,
MAP_ANON, 0, 0);
+ if (buf == MAP_FAILED)
+ return errno;
+ m->dirbuf = buf;
+ struct dirent *d;
d = add (m->dirbuf, ".");
d = add (d, "..");
m->dirbufsize = (char *) d - (char *) m->dirbuf;
--
2.43.0
