Hello,
Mikhail Karpov, le mer. 01 juil. 2026 21:01:17 +0700, a ecrit:
> Similar to mmap, I checked the code where malloc, calloc, and realloc were
> called. In these places, variable values weren't checked for NULL before
> being accessed.
Thanks for this careful work!
> diff --git a/defpager/backing.c b/defpager/backing.c
> index 56fe6551..14511004 100644
> --- a/defpager/backing.c
> +++ b/defpager/backing.c
> @@ -50,6 +50,9 @@ init_backing (char *name)
>
> bmap_len = backing_store->size / vm_page_size / NBBY;
> bmap = malloc (bmap_len);
> + if (!bmap)
> + return ENOMEM;
I'd rather return errno rather than hardcoding ENOMEM whenever possible,
in case malloc may have other issues, better report them precisely.
(but take care that an intermediate function call may overwrite errno so
in some case you need to save it).
> diff --git a/defpager/defpager.c b/defpager/defpager.c
> index 3b3cda1e..5f42fd98 100644
> --- a/defpager/defpager.c
> +++ b/defpager/defpager.c
> @@ -63,7 +68,9 @@ pager_read_page (struct user_pager_info *pager,
> /* We never request write locks. */
> *write_lock = 0;
>
> - expand_map (pager, page);
> + error_t err = expand_map (pager, page);
> + if (err)
> + return err;
>
> if (!pager->map[pfn])
> vm_allocate (mach_task_self (), buf, vm_page_size, 1);
Thinking about it: vm_allocate also would need to be checked for.
> diff --git a/eth-multiplexer/netfs_impl.c b/eth-multiplexer/netfs_impl.c
> index 83a23132..1350616d 100644
> --- a/eth-multiplexer/netfs_impl.c
> +++ b/eth-multiplexer/netfs_impl.c
> @@ -80,26 +80,31 @@ new_node (struct lnode *ln, struct node **np)
> return err;
> }
>
> -struct node *
> -lookup (const char *name)
> +static error_t
> +lookup (struct node **node, const char *name)
> {
> struct lnode *ln = (struct lnode *) lookup_dev_by_name (name);
>
> char *copied_name = malloc (strlen (name) + 1);
> + if (!copied_name)
> + return ENOMEM;
You can just return 0 rather than making the function more complex to
call.
> @@ -304,7 +309,14 @@ error_t netfs_attempt_lookup (struct iouser *user,
> struct node *dir,
> return err;
> }
>
> - *node = lookup (name);
> + err = lookup (node, name);
> + if (err)
> + {
> + *node = NULL;
> + pthread_mutex_unlock (&dir->lock);
> + return err;
And there you can return errno.
> diff --git a/ext2fs/dir.c b/ext2fs/dir.c
> index 55f26579..051540cf 100644
> --- a/ext2fs/dir.c
> +++ b/ext2fs/dir.c
> @@ -493,6 +493,9 @@ dirscanblock (vm_address_t blockaddr, struct node *dp,
> int idx,
> {
> diskfs_node_disknode (dp)->dirents =
> malloc ((dp->dn_stat.st_size / DIRBLKSIZ) * sizeof (int));
> + if (!diskfs_node_disknode (dp)->dirents)
> + return ENOMEM;
Mmmm, this seems to be just an opportunistic write: we happen to have
checked the whole block so write down to avoid doing it again later, but
it's fine if we don't write it down, we can still just return ENOENT.
> @@ -687,9 +690,13 @@ diskfs_direnter_hard (struct node *dp, const char *name,
> struct node *np,
> anything at all. */
> if (diskfs_node_disknode (dp)->dirents)
> {
> - diskfs_node_disknode (dp)->dirents =
> + int *new_dirents =
> realloc (diskfs_node_disknode (dp)->dirents,
> (dp->dn_stat.st_size / DIRBLKSIZ * sizeof (int)));
> + if (!new_dirents)
> + return ENOMEM;
These, however, indeed need to be reported.
> diff --git a/libps/procstat.c b/libps/procstat.c
> index 4de4216d..ae01b2f4 100644
> --- a/libps/procstat.c
> +++ b/libps/procstat.c
> @@ -204,6 +204,11 @@ merge_procinfo (struct proc_stat *ps, ps_flags_t need,
> ps_flags_t have)
> ps->thread_waits = malloc (WAITS_MALLOC_SIZE);
> ps->thread_waits_len = WAITS_MALLOC_SIZE;
> ps->thread_waits_vm_alloced = 0;
> + if (! ps->thread_waits)
> + {
> + free (new_pi);
Not always, see the fetch_procinfo error handling below, it's only if we
didn't already have proc_info.
> diff --git a/libstore/remap.c b/libstore/remap.c
> index bbe78509..82fae2fd 100644
> --- a/libstore/remap.c
> +++ b/libstore/remap.c
> @@ -317,7 +317,14 @@ store_remap_runs (const struct store_run *runs, size_t
> num_runs,
> }
>
> if (xruns_alloced > *num_xruns)
> - *xruns = realloc (*xruns, *num_xruns * sizeof (struct store_run));
> + {
> + void *new_xruns = realloc (*xruns, *num_xruns
> + * sizeof (struct store_run));
> + if (!new_xruns)
> + ERR (ENOMEM);
We are actually down-allocating, so if we fail it's fine, we can just
keep the original pointer.
> +
> + xruns = new_xruns;
> + }
>
> return 0;
> }
> diff --git a/nfs/ops.c b/nfs/ops.c
> index affdd931..c5100354 100644
> --- a/nfs/ops.c
> +++ b/nfs/ops.c
> @@ -2090,6 +2090,9 @@ netfs_attempt_mksymlink (struct iouser *cred,
> free (np->nn->transarg.name);
>
> np->nn->transarg.name = malloc (strlen (arg) + 1);
> + if (!np->nn->transarg.name)
> + return ENOMEM;
But we have freed np->nn->transarg.name above. We don't want to free it
if we fail to allocate the new version.
> diff --git a/term/users.c b/term/users.c
> index 629534ff..de8b4239 100644
> --- a/term/users.c
> +++ b/term/users.c
> @@ -445,6 +445,8 @@ S_term_open_ctty (struct trivfs_protid *cred,
> if (!err)
> {
> struct protid_hook *hook = malloc (sizeof (struct protid_hook));
> + if (!hook)
> + return ENOMEM;
You also need to deref newcred.
> diff --git a/utils/msgport.c b/utils/msgport.c
> index e3ea4302..4b6df302 100644
> --- a/utils/msgport.c
> +++ b/utils/msgport.c
> @@ -555,6 +555,8 @@ add_cmd (cmd_func_t func, size_t minargs, size_t maxargs,
> struct cmds_argp_params *params = state->input;
> size_t num_cmds = *params->num_cmds + 1;
> cmd_t *cmds = realloc (*params->cmds, num_cmds * sizeof(cmd_t));
> + if (!cmds)
> + return ENOMEM;
>
> *params->cmds = cmds;
> *params->num_cmds = num_cmds;
> @@ -565,6 +567,9 @@ add_cmd (cmd_func_t func, size_t minargs, size_t maxargs,
> if (maxargs)
> {
> cmd->args = malloc (maxargs * sizeof (char *));
> + if (!cmd->args)
> + return ENOMEM;
You also need to free cmds.
Samuel
