From 4dc7082215ef2d41b6f4a78997584da6b8546a8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= Date: Sat, 5 Apr 2025 08:55:58 -0300 Subject: [PATCH] Introduce KMOD_STRERROR to ensure usage is safer. While recent strerror implementations are thread aware/safe, old ones are not, and the issue of shared library safety remains. That's why strerrordesc_np() was added that is thread safe, async-signal-safe (does not call malloc) and there is no data race with other functions. Use that if available, else fallback to the closest that is calling strerror_r with a buffer whose scope expires inmediately. This concerns only libkmod, in the future the toolchain may implement per-dso allocations to make the problem disappear in practice. Note that strerrordesc_np does not display error in the current locale but only in english, this is an small price to pay for safety. --- libkmod/libkmod-builtin.c | 4 ++-- libkmod/libkmod-config.c | 2 +- libkmod/libkmod-file-xz.c | 6 +++--- libkmod/libkmod-file-zlib.c | 2 +- libkmod/libkmod-file-zstd.c | 2 +- libkmod/libkmod-internal.h | 7 +++++++ libkmod/libkmod-module.c | 33 ++++++++++++++++----------------- libkmod/libkmod.c | 14 +++++++------- meson.build | 2 +- 9 files changed, 39 insertions(+), 33 deletions(-) diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c index f354d455..697aa9d0 100644 --- a/libkmod/libkmod-builtin.c +++ b/libkmod/libkmod-builtin.c @@ -90,7 +90,7 @@ static ssize_t get_strings(struct kmod_builtin_info *info, const char *modname, if (n == -1) { if (!feof(info->fp)) { count = -errno; - ERR(info->ctx, "get_string: %s\n", strerror(errno)); + ERR(info->ctx, "get_string: %m\n"); } break; } @@ -182,7 +182,7 @@ ssize_t kmod_builtin_get_modinfo(struct kmod_ctx *ctx, const char *modname, *modinfo = strbuf_to_vector(&buf, (size_t)count); if (*modinfo == NULL) { count = -errno; - ERR(ctx, "strbuf_to_vector: %s\n", strerror(errno)); + ERR(ctx, "strbuf_to_vector: %m\n"); } } diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c index ee1cb946..c27dd961 100644 --- a/libkmod/libkmod-config.c +++ b/libkmod/libkmod-config.c @@ -661,7 +661,7 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) close(fd); if (err < 0) { ERR(config->ctx, "could not read from '/proc/cmdline': %s\n", - strerror(-err)); + KMOD_STRERROR(-err)); return err; } diff --git a/libkmod/libkmod-file-xz.c b/libkmod/libkmod-file-xz.c index f6938770..31dd679d 100644 --- a/libkmod/libkmod-file-xz.c +++ b/libkmod/libkmod-file-xz.c @@ -46,7 +46,7 @@ static void xz_uncompress_belch(struct kmod_file *file, lzma_ret ret) { switch (ret) { case LZMA_MEM_ERROR: - ERR(file->ctx, "xz: %s\n", strerror(ENOMEM)); + ERR(file->ctx, "xz: %s\n", KMOD_STRERROR(ENOMEM)); break; case LZMA_FORMAT_ERROR: ERR(file->ctx, "xz: File format not recognized\n"); @@ -128,13 +128,13 @@ int kmod_file_load_xz(struct kmod_file *file) ret = dlopen_lzma(); if (ret < 0) { - ERR(file->ctx, "xz: can't load and resolve symbols (%s)", strerror(-ret)); + ERR(file->ctx, "xz: can't load and resolve symbols (%s)", KMOD_STRERROR(-ret)); return -EINVAL; } lzret = sym_lzma_stream_decoder(&strm, UINT64_MAX, LZMA_CONCATENATED); if (lzret == LZMA_MEM_ERROR) { - ERR(file->ctx, "xz: %s\n", strerror(ENOMEM)); + ERR(file->ctx, "xz: %s\n", KMOD_STRERROR(ENOMEM)); return -ENOMEM; } else if (lzret != LZMA_OK) { ERR(file->ctx, "xz: Internal error (bug)\n"); diff --git a/libkmod/libkmod-file-zlib.c b/libkmod/libkmod-file-zlib.c index b26c7d1e..25699766 100644 --- a/libkmod/libkmod-file-zlib.c +++ b/libkmod/libkmod-file-zlib.c @@ -56,7 +56,7 @@ int kmod_file_load_zlib(struct kmod_file *file) ret = dlopen_zlib(); if (ret < 0) { ERR(file->ctx, "zlib: can't load and resolve symbols (%s)", - strerror(-ret)); + KMOD_STRERROR(-ret)); return -EINVAL; } diff --git a/libkmod/libkmod-file-zstd.c b/libkmod/libkmod-file-zstd.c index f403a01b..87435525 100644 --- a/libkmod/libkmod-file-zstd.c +++ b/libkmod/libkmod-file-zstd.c @@ -55,7 +55,7 @@ int kmod_file_load_zstd(struct kmod_file *file) ret = dlopen_zstd(); if (ret < 0) { ERR(file->ctx, "zstd: can't load and resolve symbols (%s)", - strerror(-ret)); + KMOD_STRERROR(-ret)); return -EINVAL; } diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h index 4706071d..93b105e9 100644 --- a/libkmod/libkmod-internal.h +++ b/libkmod/libkmod-internal.h @@ -25,7 +25,14 @@ #define KMOD_EXPORT __attribute__((visibility("default"))) + #define KCMD_LINE_SIZE 4096 +#ifdef HAVE_STRERRORDESC_NP +#define KMOD_STRERROR(errnum) strerrordesc_np(errnum) +#else +#define KMOD_ERRNO_BUF_LEN 1024 +#define KMOD_STRERROR(errnum) strerror_r(errnum, (char[KMOD_ERRNO_BUF_LEN]){}, KMOD_ERRNO_BUF_LEN) +#endif #ifndef HAVE_SECURE_GETENV #warning secure_getenv is not available diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index ff04ee19..fcdd79cd 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -160,7 +160,7 @@ void kmod_module_parse_depline(struct kmod_module *mod, char *line) err = kmod_module_new_from_path(ctx, path, &depmod); if (err < 0) { - ERR(ctx, "ctx=%p path=%s error=%s\n", ctx, path, strerror(-err)); + ERR(ctx, "ctx=%p path=%s error=%s\n", ctx, path, KMOD_STRERROR(-err)); goto fail; } @@ -333,7 +333,7 @@ KMOD_EXPORT int kmod_module_new_from_path(struct kmod_ctx *ctx, const char *path err = stat(abspath, &st); if (err < 0) { err = -errno; - DBG(ctx, "stat %s: %s\n", path, strerror(errno)); + DBG(ctx, "stat %s: %m\n", path); free(abspath); return err; } @@ -663,8 +663,7 @@ static int do_init_module(struct kmod_module *mod, unsigned int flags, const cha stripped = kmod_elf_strip(elf, flags); if (stripped == NULL) { - ERR(mod->ctx, "Failed to strip version information: %s\n", - strerror(errno)); + ERR(mod->ctx, "Failed to strip version information: %m\n"); return -errno; } mem = stripped; @@ -713,7 +712,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod, unsigned int err = do_init_module(mod, flags, args); if (err < 0) - INFO(mod->ctx, "Failed to insert module '%s': %s\n", path, strerror(-err)); + INFO(mod->ctx, "Failed to insert module '%s': %s\n", path, KMOD_STRERROR(-err)); return err; } @@ -892,7 +891,7 @@ static int __kmod_module_fill_softdep(struct kmod_module *mod, struct kmod_list err = kmod_module_get_softdeps(mod, &pre, &post); if (err < 0) { - ERR(mod->ctx, "could not get softdep: %s\n", strerror(-err)); + ERR(mod->ctx, "could not get softdep: %s\n", KMOD_STRERROR(-err)); goto fail; } @@ -1361,7 +1360,7 @@ KMOD_EXPORT int kmod_module_new_from_loaded(struct kmod_ctx *ctx, struct kmod_li fp = fopen("/proc/modules", "re"); if (fp == NULL) { int err = -errno; - ERR(ctx, "could not open /proc/modules: %s\n", strerror(errno)); + ERR(ctx, "could not open /proc/modules: %m\n"); return err; } @@ -1375,7 +1374,7 @@ KMOD_EXPORT int kmod_module_new_from_loaded(struct kmod_ctx *ctx, struct kmod_li err = kmod_module_new_from_name(ctx, name, &m); if (err < 0) { ERR(ctx, "could not get module from name '%s': %s\n", name, - strerror(-err)); + KMOD_STRERROR(-err)); goto eat_line; } @@ -1434,7 +1433,7 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) if (fd < 0) { err = -errno; - DBG(mod->ctx, "could not open '%s': %s\n", path, strerror(-err)); + DBG(mod->ctx, "could not open '%s': %s\n", path, KMOD_STRERROR(-err)); if (pathlen > (int)sizeof("/initstate") - 1) { struct stat st; @@ -1443,14 +1442,14 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) return KMOD_MODULE_COMING; } - DBG(mod->ctx, "could not open '%s': %s\n", path, strerror(-err)); + DBG(mod->ctx, "could not open '%s': %s\n", path, KMOD_STRERROR(-err)); return err; } err = read_str_safe(fd, buf, sizeof(buf)); close(fd); if (err < 0) { - ERR(mod->ctx, "could not read from '%s': %s\n", path, strerror(-err)); + ERR(mod->ctx, "could not read from '%s': %s\n", path, KMOD_STRERROR(-err)); return err; } @@ -1498,7 +1497,7 @@ KMOD_EXPORT long kmod_module_get_size(const struct kmod_module *mod) fp = fopen("/proc/modules", "re"); if (fp == NULL) { int err = -errno; - ERR(mod->ctx, "could not open /proc/modules: %s\n", strerror(errno)); + ERR(mod->ctx, "could not open /proc/modules: %m\n"); close(dfd); return err; } @@ -1550,7 +1549,7 @@ KMOD_EXPORT int kmod_module_get_refcnt(const struct kmod_module *mod) fd = open(path, O_RDONLY | O_CLOEXEC); if (fd < 0) { err = -errno; - DBG(mod->ctx, "could not open '%s': %s\n", path, strerror(errno)); + DBG(mod->ctx, "could not open '%s': %m\n", path); return err; } @@ -1558,7 +1557,7 @@ KMOD_EXPORT int kmod_module_get_refcnt(const struct kmod_module *mod) close(fd); if (err < 0) { ERR(mod->ctx, "could not read integer from '%s': '%s'\n", path, - strerror(-err)); + KMOD_STRERROR(-err)); return err; } @@ -1579,7 +1578,7 @@ KMOD_EXPORT struct kmod_list *kmod_module_get_holders(const struct kmod_module * d = opendir(dname); if (d == NULL) { - ERR(mod->ctx, "could not open '%s': %s\n", dname, strerror(errno)); + ERR(mod->ctx, "could not open '%s': %m\n", dname); return NULL; } @@ -1597,7 +1596,7 @@ KMOD_EXPORT struct kmod_list *kmod_module_get_holders(const struct kmod_module * err = kmod_module_new_from_name(mod->ctx, dent->d_name, &holder); if (err < 0) { ERR(mod->ctx, "could not create module for '%s': %s\n", - dent->d_name, strerror(-err)); + dent->d_name, KMOD_STRERROR(-err)); goto fail; } @@ -1645,7 +1644,7 @@ KMOD_EXPORT struct kmod_list *kmod_module_get_sections(const struct kmod_module d = opendir(dname); if (d == NULL) { - ERR(mod->ctx, "could not open '%s': %s\n", dname, strerror(errno)); + ERR(mod->ctx, "could not open '%s': %m\n", dname); return NULL; } diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c index f4510d0e..0594ee60 100644 --- a/libkmod/libkmod.c +++ b/libkmod/libkmod.c @@ -198,7 +198,7 @@ static enum kmod_file_compression_type get_kernel_compression(struct kmod_ctx *c err = read_str_safe(fd, buf, sizeof(buf)); close(fd); if (err < 0) { - ERR(ctx, "could not read from '%s': %s\n", path, strerror(-err)); + ERR(ctx, "could not read from '%s': %s\n", path, KMOD_STRERROR(-err)); return KMOD_FILE_COMPRESSION_NONE; } @@ -385,7 +385,7 @@ static int kmod_lookup_alias_from_alias_bin(struct kmod_ctx *ctx, err = kmod_module_new_from_alias(ctx, name, realname->value, &mod); if (err < 0) { ERR(ctx, "Could not create module for alias=%s realname=%s: %s\n", - name, realname->value, strerror(-err)); + name, realname->value, KMOD_STRERROR(-err)); goto fail; } @@ -492,7 +492,7 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, err = kmod_module_new_from_name(ctx, name, &mod); if (err < 0) { ERR(ctx, "Could not create module from name %s: %s\n", name, - strerror(-err)); + KMOD_STRERROR(-err)); return err; } @@ -544,7 +544,7 @@ int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name, n = kmod_module_new_from_name(ctx, name, &mod); if (n < 0) { ERR(ctx, "Could not create module from name %s: %s\n", name, - strerror(-n)); + KMOD_STRERROR(-n)); goto finish; } @@ -586,7 +586,7 @@ int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name, if (err < 0) { ERR(ctx, "Could not create module for alias=%s modname=%s: %s\n", - name, modname, strerror(-err)); + name, modname, KMOD_STRERROR(-err)); goto fail; } @@ -628,7 +628,7 @@ int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name, err = kmod_module_new_from_name(ctx, modname, &mod); if (err < 0) { ERR(ctx, "Could not create module from name %s: %s\n", - modname, strerror(-err)); + modname, KMOD_STRERROR(-err)); return err; } @@ -665,7 +665,7 @@ int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name, err = kmod_module_new_from_name(ctx, modname, &mod); if (err < 0) { ERR(ctx, "Could not create module from name %s: %s\n", - modname, strerror(-err)); + modname, KMOD_STRERROR(-err)); return err; } diff --git a/meson.build b/meson.build index 811c9d08..6bd4e4b0 100644 --- a/meson.build +++ b/meson.build @@ -35,7 +35,7 @@ cdata.set10('_GNU_SOURCE', true) _funcs = [ 'open64', 'stat64', 'fopen64', '__stat64_time64', - 'secure_getenv', + 'secure_getenv', 'strerrordesc_np', ] foreach func : _funcs if cc.has_function(func, args : '-D_GNU_SOURCE')