memory-util: introduce munmap_safe() and make use of it everywhere

This new wrapper for munmap() asserts on a bunch of unexpected
conditions:

1. if we try to unmap a valid pointer with a zero size
2. if we try to unmap a valid pointer with a size of SIZE_MAX (this
   is supposed to catch PAGE_SIZE() overflows, which is often used in
   combination)
3. munmap() failing

It also turns invocations with a NULL or MAP_FAILED pointer into NOPs.
That's useful to simplify error clean-up paths in case mmap() failed.

Finaly it returns a NULL pointer to the input type, so that we can
write:

p = mummap_safe(p, sz);

to invalidate both memory and the pointer p in one go.

src/basic/argv-util.c

@@ -9,6 +9,7 @@
 #include "capability-util.h"
 #include "errno-util.h"
 #include "log.h"
+#include "memory-util.h"
 #include "parse-util.h"
 #include "path-util.h"
 #include "process-util.h"
@@ -123,7 +124,7 @@ static int update_argv(const char name[], size_t l) {
                         r = prctl_safe(PR_SET_MM, PR_SET_MM_ARG_END, (unsigned long) nn + l + 1, 0, 0);
                         if (r < 0) {
                                 log_debug_errno(r, "PR_SET_MM_ARG_END hack failed, proceeding without: %m");
-                                (void) munmap(nn, nn_size);
+                                munmap_safe(nn, nn_size);
                                 return r;
                         }
 
@@ -139,8 +140,7 @@ static int update_argv(const char name[], size_t l) {
                                 log_debug_errno(r, "PR_SET_MM_ARG_END failed, proceeding without: %m");
                 }
 
-                if (mm)
-                        (void) munmap(mm, mm_size);
+                munmap_safe(mm, mm_size);
 
                 mm = nn;
                 mm_size = nn_size;

src/basic/locale-util.c

@@ -13,6 +13,7 @@
 #include "fileio.h"
 #include "locale-util.h"
 #include "log.h"
+#include "memory-util.h"
 #include "path-util.h"
 #include "process-util.h"
 #include "set.h"
@@ -165,9 +166,7 @@ static int add_locales_from_archive(Set *locales) {
         r = 0;
 
 finish:
-        if (p != MAP_FAILED)
-                munmap(p, st.st_size);
-
+        munmap_safe(p, st.st_size);
         return r;
 }
 

src/basic/memory-util.h

@@ -2,6 +2,7 @@
 #pragma once
 
 #include <string.h>
+#include <sys/mman.h> /* IWYU pragma: export */
 
 #include "forward.h"
 
@@ -98,3 +99,16 @@ static inline void erase_and_freep(void *p) {
 static inline void erase_char(char *p) {
         explicit_bzero_safe(p, sizeof(char));
 }
+
+#define munmap_safe(p, sz) _munmap_safe(UNIQ_T(P, UNIQ), (p), UNIQ_T(SZ, UNIQ), (sz))
+#define _munmap_safe(_p, p, _sz, sz)                                    \
+        ({                                                              \
+                typeof(p) _p = (p);                                     \
+                if (_p && _p != MAP_FAILED) {                           \
+                        size_t _sz = (sz);                              \
+                        assert(_sz > 0);                                \
+                        assert(_sz != SIZE_MAX); /* catch PAGE_ALIGN() overflows */ \
+                        assert_se(munmap(_p, _sz) >= 0);                \
+                }                                                       \
+                (typeof(p)) NULL;                                       \
+        })

src/core/bpf-restrict-fsaccess.c

@@ -139,7 +139,7 @@ static int proc_mem_can_force_override(void) {
                 r = n > 0 ? 1 : -EIO; /* Nothing written and no error is not a refusal, fail closed. */
 
 finish:
-        (void) munmap(p, page_size());
+        munmap_safe(p, page_size());
         return r;
 }
 

src/journal/journald-kmsg.c

@@ -23,6 +23,7 @@
 #include "journald-syslog.h"
 #include "log.h"
 #include "log-ratelimit.h"
+#include "memory-util.h"
 #include "parse-util.h"
 #include "process-util.h"
 #include "stdio-util.h"
@@ -437,20 +438,6 @@ int manager_open_kernel_seqnum(Manager *m) {
         return 0;
 }
 
-void manager_close_kernel_seqnum(Manager *m) {
-        assert(m);
-
-        manager_unmap_seqnum_file(m->kernel_seqnum, sizeof(*m->kernel_seqnum));
-        m->kernel_seqnum = NULL;
-}
-
-static int manager_unlink_kernel_seqnum(Manager *m) {
-        assert(m);
-        assert(!m->kernel_seqnum); /* The file must not be mmap()ed. */
-
-        return manager_unlink_seqnum_file(m, "kernel-seqnum");
-}
-
 int manager_reopen_dev_kmsg(Manager *m, bool old_read_kmsg) {
         int r;
 
@@ -470,11 +457,11 @@ int manager_reopen_dev_kmsg(Manager *m, bool old_read_kmsg) {
                 m->config.read_kmsg = false;
 
                 /* seqnum file is not necessary anymore. Let's close it. */
-                manager_close_kernel_seqnum(m);
+                m->kernel_seqnum = munmap_safe(m->kernel_seqnum, sizeof(*m->kernel_seqnum));
 
                 /* Also, unlink the file name as we will not warn some kmsg are lost when reading kmsg is
                  * re-enabled later. */
-                manager_unlink_kernel_seqnum(m);
+                (void) manager_unlink_seqnum_file(m, "kernel-seqnum");
         }
 
         /* Close previously configured event source and opened file descriptor. */

src/journal/journald-kmsg.h

@@ -10,6 +10,5 @@ int manager_reopen_dev_kmsg(Manager *m, bool old_read_kmsg);
 void manager_forward_kmsg(Manager *m, int priority, const char *identifier, const char *message, const struct ucred *ucred);
 
 int manager_open_kernel_seqnum(Manager *m);
-void manager_close_kernel_seqnum(Manager *m);
 
 void dev_kmsg_record(Manager *m, char *p, size_t l);

src/journal/journald-manager.c

@@ -2095,11 +2095,11 @@ int manager_map_seqnum_file(
         if (fd < 0)
                 return -errno;
 
-        r = posix_fallocate_loop(fd, 0, size);
+        r = posix_fallocate_loop(fd, /* offset= */ 0, size);
         if (r < 0)
                 return r;
 
-        p = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+        p = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, /* offset= */ 0);
         if (p == MAP_FAILED)
                 return -errno;
 
@@ -2107,15 +2107,6 @@ int manager_map_seqnum_file(
         return 0;
 }
 
-void manager_unmap_seqnum_file(void *p, size_t size) {
-        assert(size > 0);
-
-        if (!p)
-                return;
-
-        assert_se(munmap(p, size) >= 0);
-}
-
 int manager_unlink_seqnum_file(Manager *m, const char *fname) {
         assert(m);
         assert(fname);
@@ -2603,8 +2594,8 @@ Manager* manager_free(Manager *m) {
 
         ordered_hashmap_free(m->ratelimit_groups_by_id);
 
-        manager_unmap_seqnum_file(m->seqnum, sizeof(*m->seqnum));
-        manager_close_kernel_seqnum(m);
+        m->seqnum = munmap_safe(m->seqnum, sizeof(*m->seqnum));
+        m->kernel_seqnum = munmap_safe(m->kernel_seqnum, sizeof(*m->kernel_seqnum));
 
         free(m->buffer);
         free(m->cgroup_root);

src/journal/journald-manager.h

@@ -191,5 +191,4 @@ int manager_start_or_stop_idle_timer(Manager *m);
 void manager_reopen_journals(Manager *m, const JournalConfig *old);
 
 int manager_map_seqnum_file(Manager *m, const char *fname, size_t size, void **ret);
-void manager_unmap_seqnum_file(void *p, size_t size);
 int manager_unlink_seqnum_file(Manager *m, const char *fname);

src/journal/journald-native.c

@@ -469,7 +469,7 @@ int manager_process_native_file(
                                                          "Failed to map memfd: %m");
 
                 manager_process_native_message(m, p, st.st_size, ucred, tv, label);
-                assert_se(munmap(p, ps) >= 0);
+                munmap_safe(p, ps);
 
                 return 0;
         }

src/libsystemd/sd-bus/bus-kernel.c

@@ -1,19 +1,12 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
-#include <sys/mman.h>
-
 #include "bus-kernel.h"
 #include "bus-internal.h"
 #include "fd-util.h"
 #include "memory-util.h"
 
 void close_and_munmap(int fd, void *address, size_t size) {
-        if (size > 0) {
-                size = PAGE_ALIGN(size);
-                assert(size < SIZE_MAX);
-                assert_se(munmap(address, size) >= 0);
-        }
-
+        munmap_safe(address, PAGE_ALIGN(size));
         safe_close(fd);
 }
 

src/libsystemd/sd-bus/bus-message.c

@@ -49,7 +49,7 @@ static void message_free_part(sd_bus_message *m, BusMessageBodyPart *part) {
         } else if (part->munmap_this)
                 /* We don't erase sensitive data here, since the data is memory mapped from someone else, and
                  * we just don't know if it's OK to write to it */
-                munmap(part->mmap_begin, part->mapped);
+                munmap_safe(part->mmap_begin, part->mapped);
         else {
                 /* Erase this if that is requested. Since this is regular memory we know we can write it. */
                 if (m->sensitive)
@@ -2528,11 +2528,10 @@ void bus_body_part_unmap(BusMessageBodyPart *part) {
         if (!part->munmap_this)
                 return;
 
-        assert_se(munmap(part->mmap_begin, part->mapped) == 0);
-
-        part->mmap_begin = NULL;
-        part->data = NULL;
+        part->mmap_begin = munmap_safe(part->mmap_begin, part->mapped);
         part->mapped = 0;
+
+        part->data = NULL;
         part->munmap_this = false;
 
         return;

src/libsystemd/sd-future/fiber.c

@@ -139,7 +139,7 @@ static int fiber_allocate_stack(size_t size, void **ret) {
         if (r == -EINVAL)
                 r = RET_NERRNO(mprotect(guard, page_size(), PROT_NONE));
         if (r < 0) {
-                (void) munmap(stack, size);
+                munmap_safe(stack, size);
                 return r;
         }
 
@@ -520,8 +520,7 @@ static void fiber_free(sd_future *f) {
                 VALGRIND_STACK_DEREGISTER(fiber->stack_id);
 #endif
 
-        if (fiber->stack.iov_base)
-                (void) munmap(fiber->stack.iov_base, fiber->stack.iov_len);
+        munmap_safe(fiber->stack.iov_base, fiber->stack.iov_len);
 
         sd_event_source_disable_unref(fiber->defer_event_source);
         sd_event_source_disable_unref(fiber->exit_event_source);

src/libsystemd/sd-hwdb/sd-hwdb.c

@@ -18,6 +18,7 @@
 #include "hashmap.h"
 #include "hwdb-internal.h"
 #include "log.h"
+#include "memory-util.h"
 #include "nulstr-util.h"
 #include "string-util.h"
 
@@ -457,8 +458,7 @@ _public_ int sd_hwdb_new(sd_hwdb **ret) {
 static sd_hwdb *hwdb_free(sd_hwdb *hwdb) {
         assert(hwdb);
 
-        if (hwdb->map)
-                munmap((void *)hwdb->map, hwdb->st.st_size);
+        munmap_safe((void*) hwdb->map, hwdb->st.st_size);
         safe_fclose(hwdb->f);
         ordered_hashmap_free(hwdb->properties);
         return mfree(hwdb);

src/libsystemd/sd-journal/catalog.c

@@ -528,7 +528,7 @@ static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, voi
             !MUL_SAFE(&total, le64toh(h->catalog_item_size), le64toh(h->n_items)) ||
             !INC_SAFE(&total, le64toh(h->header_size)) ||
             (uint64_t) st.st_size < total) {
-                munmap(p, st.st_size);
+                munmap_safe(p, st.st_size);
                 return -EBADMSG;
         }
 
@@ -625,7 +625,7 @@ int catalog_get(const char *database, sd_id128_t id, char **ret_text) {
         else
                 r = strdup_to(ret_text, s);
 
-        (void) munmap(p, st.st_size);
+        munmap_safe(p, st.st_size);
         return r;
 }
 
@@ -705,7 +705,7 @@ int catalog_list(FILE *f, const char *database, bool oneline) {
                 last_id = items[n].id;
         }
 
-        (void) munmap(p, st.st_size);
+        munmap_safe(p, st.st_size);
         return 0;
 }
 

src/libsystemd/sd-journal/mmap-cache.c

@@ -98,8 +98,7 @@ static Window* window_unlink(Window *w) {
 
         MMapCache *m = mmap_cache_fd_cache(w->fd);
 
-        if (w->ptr)
-                munmap(w->ptr, w->size);
+        w->ptr = munmap_safe(w->ptr, w->size);
 
         if (FLAGS_SET(w->flags, WINDOW_IN_UNUSED)) {
                 if (m->last_unused == w)
@@ -327,7 +326,7 @@ static int add_mmap(
 
         w = window_add(f, offset, size, d);
         if (!w) {
-                (void) munmap(d, size);
+                munmap_safe(d, size);
                 return -ENOMEM;
         }
 

src/shared/journal-authenticate.c

@@ -41,7 +41,7 @@ static JournalAuthContext* journal_auth_free(JournalAuthContext *c) {
                 return NULL;
 
         if (c->fss_file)
-                munmap(c->fss_file, c->fss_mmap_size);
+                munmap_safe(c->fss_file, c->fss_mmap_size);
         else
                 iovec_done_erase(&c->fsprg_state);
 
@@ -60,12 +60,7 @@ static JournalAuthContext* journal_auth_free(JournalAuthContext *c) {
 DEFINE_TRIVIAL_CLEANUP_FUNC(JournalAuthContext*, journal_auth_free);
 
 static void* fssheader_free(FSSHeader *p) {
-        /* mmap() returns MAP_FAILED on error and sets the errno */
-        if (!p || p == MAP_FAILED)
-                return NULL;
-
-        assert_se(munmap(p, PAGE_ALIGN(sizeof(FSSHeader))) >= 0);
-        return NULL;
+        return munmap_safe(p, PAGE_ALIGN(sizeof(FSSHeader)));
 }
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(FSSHeader*, fssheader_free);

src/shared/reboot-util.c

@@ -28,6 +28,7 @@
 #include "io-util.h"
 #include "log.h"
 #include "memfd-util.h"
+#include "memory-util.h"
 #include "pe-binary.h"
 #include "proc-cmdline.h"
 #include "reboot-util.h"
@@ -217,7 +218,7 @@ static int xen_kexec_command(uint64_t cmd) {
         if (r < 0)
                 log_debug_errno(r, "kexec%s failed: %m", cmd == KEXEC_CMD_kexec_status ? "_status" : "");
 
-        munmap(buffer, size);
+        munmap_safe(buffer, size);
 
         return r;
 }

src/test/test-bpf-restrict-fsaccess.c

@@ -44,6 +44,7 @@
 #include "errno-util.h"
 #include "fd-util.h"
 #include "log.h"
+#include "memory-util.h"
 #include "pidref.h"
 #include "process-util.h"
 #include "seccomp-util.h"
@@ -90,7 +91,7 @@ static int mmap_probe(const char *path, int prot, const char *what) {
                 return probe_denied(r, -EPERM);
         }
 
-        (void) munmap(addr, 4096);
+        munmap_safe(addr, 4096);
         log_info("%s of %s succeeded", what, path);
         return PROBE_ALLOWED;
 }
@@ -118,7 +119,7 @@ static int mprotect_probe(const char *path, int prot, bool poke, int new_prot, c
                 *(volatile uint8_t*) addr = 0x90; /* trigger copy-on-write */
 
         r = RET_NERRNO(mprotect(addr, 4096, new_prot));
-        (void) munmap(addr, 4096);
+        munmap_safe(addr, 4096);
 
         if (r < 0) {
                 log_info_errno(r, "%s on %s denied: %m", what, path);
@@ -143,7 +144,7 @@ static int do_anon_mmap_exec(void) {
                 return probe_denied(r, -EPERM);
         }
 
-        (void) munmap(addr, 4096);
+        munmap_safe(addr, 4096);
         log_info("Anonymous PROT_EXEC mmap succeeded");
         return PROBE_ALLOWED;
 }
@@ -200,14 +201,14 @@ static int do_procmem_cow(const char *path) {
         mem_fd = open("/proc/self/mem", O_RDWR | O_CLOEXEC);
         if (mem_fd < 0) {
                 r = -errno;
-                (void) munmap(addr, 4096);
+                munmap_safe(addr, 4096);
                 log_error_errno(r, "Failed to open /proc/self/mem: %m");
                 return PROBE_ERROR;
         }
 
         n = pwrite(mem_fd, &byte, sizeof(byte), (off_t) (uintptr_t) addr);
         r = n < 0 ? -errno : 0;
-        (void) munmap(addr, 4096);
+        munmap_safe(addr, 4096);
 
         if (r < 0) {
                 log_info_errno(r, "/proc/self/mem write to exec page of %s denied: %m", path);

src/test/test-seccomp.c

@@ -551,11 +551,11 @@ TEST(memory_deny_write_execute_mmap) {
 
                 p = mmap(NULL, page_size(), PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
                 assert_se(p != MAP_FAILED);
-                assert_se(munmap(p, page_size()) >= 0);
+                munmap_safe(p, page_size());
 
                 p = mmap(NULL, page_size(), PROT_WRITE|PROT_READ, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
                 assert_se(p != MAP_FAILED);
-                assert_se(munmap(p, page_size()) >= 0);
+                munmap_safe(p, page_size());
 
                 assert_se(seccomp_memory_deny_write_execute() >= 0);
 
@@ -570,12 +570,11 @@ TEST(memory_deny_write_execute_mmap) {
 #endif
                 /* Depending on kernel, libseccomp, and glibc versions, other architectures
                  * might fail or not. Let's not assert success. */
-                if (p != MAP_FAILED)
-                        assert_se(munmap(p, page_size()) == 0);
+                munmap_safe(p, page_size());
 
                 p = mmap(NULL, page_size(), PROT_WRITE|PROT_READ, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
                 assert_se(p != MAP_FAILED);
-                assert_se(munmap(p, page_size()) >= 0);
+                munmap_safe(p, page_size());
 
                 _exit(EXIT_SUCCESS);
         }