diff options
author | Yu Watanabe <watanabe.yu+github@gmail.com> | 2023-03-22 08:49:49 +0900 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2023-03-22 16:17:13 +0100 |
commit | 9b032f932c4172fac379234d9d42cf2b266ccaea (patch) | |
tree | 9f6da6c886284f2aef1f72af346f5a963d8fa811 /src/coredump | |
parent | bf9f07a62966043c0f1fd7ac4a249790643d1f9a (diff) | |
download | systemd-9b032f932c4172fac379234d9d42cf2b266ccaea.tar.gz |
coredump: use unaligned_read_ne{32,64}() to parse auxv
Fixes a bug introduced by 3e4d0f6cf99f8677edd6a237382a65bfe758de03.
The auxv metadata is unaligned, as the length of the prefix
"COREDUMP_PROC_AUXV=" is 19. Hence, parse_auxv{32,64}() may triger
an undefined behavior (or at least cause slow down), which can be
detected when running on an undefined behavior sanitizer.
This also introduces a macro to define `parse_auxv{32,64}()`.
Fixes #26912.
Diffstat (limited to 'src/coredump')
-rw-r--r-- | src/coredump/coredump.c | 149 |
1 files changed, 60 insertions, 89 deletions
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 266471a9c8..1f259cf669 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -49,6 +49,7 @@ #include "sync-util.h" #include "tmpfile-util.h" #include "uid-alloc-range.h" +#include "unaligned.h" #include "user-util.h" /* The maximum size up to which we process coredumps. We use 1G on 32bit systems, and 32G on 64bit systems */ @@ -335,95 +336,65 @@ static int make_filename(const Context *context, char **ret) { return 0; } -static int parse_auxv64( - const uint64_t *auxv, - size_t size_bytes, - int *at_secure, - uid_t *uid, - uid_t *euid, - gid_t *gid, - gid_t *egid) { - - assert(auxv || size_bytes == 0); - - if (size_bytes % (2 * sizeof(uint64_t)) != 0) - return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); - - size_t words = size_bytes / sizeof(uint64_t); - - /* Note that we set output variables even on error. */ - - for (size_t i = 0; i + 1 < words; i += 2) - switch (auxv[i]) { - case AT_SECURE: - *at_secure = auxv[i + 1] != 0; - break; - case AT_UID: - *uid = auxv[i + 1]; - break; - case AT_EUID: - *euid = auxv[i + 1]; - break; - case AT_GID: - *gid = auxv[i + 1]; - break; - case AT_EGID: - *egid = auxv[i + 1]; - break; - case AT_NULL: - if (auxv[i + 1] != 0) - goto error; - return 0; - } - error: - return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), - "AT_NULL terminator not found, cannot parse auxv structure."); -} - -static int parse_auxv32( - const uint32_t *auxv, - size_t size_bytes, - int *at_secure, - uid_t *uid, - uid_t *euid, - gid_t *gid, - gid_t *egid) { - - assert(auxv || size_bytes == 0); - - size_t words = size_bytes / sizeof(uint32_t); - - if (size_bytes % (2 * sizeof(uint32_t)) != 0) - return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); +#define _DEFINE_PARSE_AUXV(size, type, unaligned_read) \ + static int parse_auxv##size( \ + const void *auxv, \ + size_t size_bytes, \ + int *at_secure, \ + uid_t *uid, \ + uid_t *euid, \ + gid_t *gid, \ + gid_t *egid) { \ + \ + assert(auxv || size_bytes == 0); \ + \ + if (size_bytes % (2 * sizeof(type)) != 0) \ + return log_warning_errno(SYNTHETIC_ERRNO(EIO), \ + "Incomplete auxv structure (%zu bytes).", \ + size_bytes); \ + \ + size_t words = size_bytes / sizeof(type); \ + \ + /* Note that we set output variables even on error. */ \ + \ + for (size_t i = 0; i + 1 < words; i += 2) { \ + type key, val; \ + \ + key = unaligned_read((uint8_t*) auxv + i * sizeof(type)); \ + val = unaligned_read((uint8_t*) auxv + (i + 1) * sizeof(type)); \ + \ + switch (key) { \ + case AT_SECURE: \ + *at_secure = val != 0; \ + break; \ + case AT_UID: \ + *uid = val; \ + break; \ + case AT_EUID: \ + *euid = val; \ + break; \ + case AT_GID: \ + *gid = val; \ + break; \ + case AT_EGID: \ + *egid = val; \ + break; \ + case AT_NULL: \ + if (val != 0) \ + goto error; \ + return 0; \ + } \ + } \ + error: \ + return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), \ + "AT_NULL terminator not found, cannot parse auxv structure."); \ + } - /* Note that we set output variables even on error. */ +#define DEFINE_PARSE_AUXV(size)\ + _DEFINE_PARSE_AUXV(size, uint##size##_t, unaligned_read_ne##size) - for (size_t i = 0; i + 1 < words; i += 2) - switch (auxv[i]) { - case AT_SECURE: - *at_secure = auxv[i + 1] != 0; - break; - case AT_UID: - *uid = auxv[i + 1]; - break; - case AT_EUID: - *euid = auxv[i + 1]; - break; - case AT_GID: - *gid = auxv[i + 1]; - break; - case AT_EGID: - *egid = auxv[i + 1]; - break; - case AT_NULL: - if (auxv[i + 1] != 0) - goto error; - return 0; - } - error: - return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), - "AT_NULL terminator not found, cannot parse auxv structure."); -} +DEFINE_PARSE_AUXV(32); +DEFINE_PARSE_AUXV(64); static int grant_user_access(int core_fd, const Context *context) { int at_secure = -1; @@ -460,11 +431,11 @@ static int grant_user_access(int core_fd, const Context *context) { "Core file has non-native endianness, not adjusting permissions."); if (elf[EI_CLASS] == ELFCLASS64) - r = parse_auxv64((const uint64_t*) context->meta[META_PROC_AUXV], + r = parse_auxv64(context->meta[META_PROC_AUXV], context->meta_size[META_PROC_AUXV], &at_secure, &uid, &euid, &gid, &egid); else - r = parse_auxv32((const uint32_t*) context->meta[META_PROC_AUXV], + r = parse_auxv32(context->meta[META_PROC_AUXV], context->meta_size[META_PROC_AUXV], &at_secure, &uid, &euid, &gid, &egid); if (r < 0) |