From b8cdfff250689c5a7020b3da5d176453668a3fba Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Fri, 26 Aug 2022 10:10:47 -0700 Subject: [PATCH] libdrgn: add read(2) and pread(2) wrappers that don't return short reads We have a couple of loops that deal with short reads/EINTR from read(2) and pread(2), and upcoming changes would need to add more. Add some wrappers to abstract this away. drgn_read_memory_file() still needs the loop so it can fault on the exact offset that returns EIO. Signed-off-by: Omar Sandoval --- libdrgn/Doxyfile | 2 +- libdrgn/Makefile.am | 2 ++ libdrgn/io.c | 50 ++++++++++++++++++++++++++++++++++++++++++ libdrgn/io.h | 27 +++++++++++++++++++++++ libdrgn/linux_kernel.c | 29 +++++++++--------------- libdrgn/program.c | 23 +++++-------------- 6 files changed, 96 insertions(+), 37 deletions(-) create mode 100644 libdrgn/io.c create mode 100644 libdrgn/io.h diff --git a/libdrgn/Doxyfile b/libdrgn/Doxyfile index 114d110d..69e94e23 100644 --- a/libdrgn/Doxyfile +++ b/libdrgn/Doxyfile @@ -269,7 +269,7 @@ TAB_SIZE = 4 # commands \{ and \} for these it is advised to use the version @{ and @} or use # a double escape (\\{ and \\}) -ALIASES = +ALIASES = manpage{2}="\1(\2)" # Set the OPTIMIZE_OUTPUT_FOR_C tag to YES if your project consists of C sources # only. Doxygen will then generate output that is more tailored for C. For diff --git a/libdrgn/Makefile.am b/libdrgn/Makefile.am index 35f549f1..f0883e11 100644 --- a/libdrgn/Makefile.am +++ b/libdrgn/Makefile.am @@ -55,6 +55,8 @@ libdrgnimpl_la_SOURCES = $(ARCH_DEFS_PYS:_defs.py=.c) \ hash_table.c \ hash_table.h \ helpers.h \ + io.c \ + io.h \ language.c \ language.h \ language_c.c \ diff --git a/libdrgn/io.c b/libdrgn/io.c new file mode 100644 index 00000000..b3a0a6ed --- /dev/null +++ b/libdrgn/io.c @@ -0,0 +1,50 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include + +#include "io.h" + +ssize_t read_all(int fd, void *buf, size_t count) +{ + if (count > SSIZE_MAX) { + errno = EINVAL; + return -1; + } + size_t n = 0; + while (n < count) { + ssize_t r = read(fd, (char *)buf + n, count - n); + if (r < 0) { + if (errno == EINTR) + continue; + return r; + } else if (r == 0) { + break; + } + n += r; + } + return n; +} + +ssize_t pread_all(int fd, void *buf, size_t count, off_t offset) +{ + if (count > SSIZE_MAX) { + errno = EINVAL; + return -1; + } + size_t n = 0; + while (n < count) { + ssize_t r = pread(fd, (char *)buf + n, count - n, offset + n); + if (r < 0) { + if (errno == EINTR) + continue; + return r; + } else if (r == 0) { + break; + } + n += r; + } + return n; +} diff --git a/libdrgn/io.h b/libdrgn/io.h new file mode 100644 index 00000000..b7245773 --- /dev/null +++ b/libdrgn/io.h @@ -0,0 +1,27 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. +// SPDX-License-Identifier: GPL-3.0-or-later + +/** + * @file + * + * Input/output helpers. + */ + +#ifndef DRGN_IO_H +#define DRGN_IO_H + +#include + +/** + * Wrapper around \manpage{read,2} that never returns less bytes than requested unless it + * hits end-of-file. + */ +ssize_t read_all(int fd, void *buf, size_t count); + +/** + * Wrapper around \manpage{pread,2} that never returns less bytes than requested unless + * it hits end-of-file. + */ +ssize_t pread_all(int fd, void *buf, size_t count, off_t offset); + +#endif /* DRGN_IO_H */ diff --git a/libdrgn/linux_kernel.c b/libdrgn/linux_kernel.c index 90c2aa8f..d0798f17 100644 --- a/libdrgn/linux_kernel.c +++ b/libdrgn/linux_kernel.c @@ -23,6 +23,7 @@ #include "error.h" #include "hash_table.h" #include "helpers.h" +#include "io.h" #include "linux_kernel.h" #include "platform.h" #include "program.h" @@ -630,28 +631,18 @@ kernel_module_iterator_gnu_build_id_live(struct kernel_module_iterator *it, goto out; } - char *buf = it->build_id_buf; - size_t size = 0; - while (size < st.st_size) { - ssize_t r = read(fd, buf + size, st.st_size - size); - if (r < 0) { - if (errno == EINTR) - continue; - err = drgn_error_format_os("read", errno, - "%s/%s", path, - ent->d_name); - close(fd); - goto out; - } else if (r == 0) { - break; - } - size += r; + ssize_t r = read_all(fd, it->build_id_buf, st.st_size); + if (r < 0) { + err = drgn_error_format_os("read", errno, "%s/%s", path, + ent->d_name); + close(fd); + goto out; } close(fd); - *build_id_len_ret = parse_gnu_build_id_from_note(buf, size, - false, - build_id_ret); + *build_id_len_ret = + parse_gnu_build_id_from_note(it->build_id_buf, r, false, + build_id_ret); if (*build_id_len_ret) { err = NULL; goto out; diff --git a/libdrgn/program.c b/libdrgn/program.c index bafb5652..c7f3e93e 100644 --- a/libdrgn/program.c +++ b/libdrgn/program.c @@ -21,6 +21,7 @@ #include "debug_info.h" #include "error.h" #include "helpers.h" +#include "io.h" #include "language.h" #include "linux_kernel.h" #include "memory_reader.h" @@ -205,23 +206,11 @@ static struct drgn_error *has_kdump_signature(const char *path, int fd, bool *ret) { char signature[KDUMP_SIG_LEN]; - size_t n = 0; - - while (n < sizeof(signature)) { - ssize_t sret; - - sret = pread(fd, signature + n, sizeof(signature) - n, n); - if (sret == -1) { - if (errno == EINTR) - continue; - return drgn_error_create_os("pread", errno, path); - } else if (sret == 0) { - *ret = false; - return NULL; - } - n += sret; - } - *ret = memcmp(signature, KDUMP_SIGNATURE, sizeof(signature)) == 0; + ssize_t r = pread_all(fd, signature, sizeof(signature), 0); + if (r < 0) + return drgn_error_create_os("pread", errno, path); + *ret = (r == sizeof(signature) + && memcmp(signature, KDUMP_SIGNATURE, sizeof(signature)) == 0); return NULL; }