From 5e5d49264f7cc41df9d2b195efbffea60f71f94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B6ppner?= Date: Thu, 17 Jun 2021 23:00:56 +0200 Subject: [PATCH] libutil: Simplify util_path_sysfs and helper functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using util_path_sysfs always leaves 5 bytes of memory unfreed as the value for the sysfs mount point is stored in a static variable to avoid multiple queries of /proc/mount. $ valgrind ./util_path_example sysfs ==3629315== Memcheck, a memory error detector ==3629315== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==3629315== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==3629315== Command: ./util_path_example sysfs ==3629315== Path for cpu: "/sys/devices/system/cpu" Path for memory: "/sys/devices/system/memory" ==3629315== ==3629315== HEAP SUMMARY: ==3629315== in use at exit: 5 bytes in 1 blocks ==3629315== total heap usage: 22 allocs, 21 frees, 18,435 bytes allocated ==3629315== ==3629315== LEAK SUMMARY: ==3629315== definitely lost: 0 bytes in 0 blocks ==3629315== indirectly lost: 0 bytes in 0 blocks ==3629315== possibly lost: 0 bytes in 0 blocks ==3629315== still reachable: 5 bytes in 1 blocks ==3629315== suppressed: 0 bytes in 0 blocks ==3629315== Rerun with --leak-check=full to see details of leaked memory ==3629315== ==3629315== For lists of detected and suppressed errors, rerun with: -s ==3629315== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) As per the Kernel rules for accessing sysfs information [1], searching for the sysfs mount point is a waste of time and systems that don't have sysfs mounted at /sys are considered broken. With those things in mind, util_path_sysfs() and especially sys_mount_point() can be simplified. sys_mount_point() will always return '/sys' unless the environment variable SYSFS_ROOT is set. With SYSFS_ROOT still being present, special container setups or test case scenarios are still possible but might need to be modified if they previously relied on util_path_sysfs() automatically finding the correct sysfs mount point. To make things more secure against malicious strings in SYSFS_ROOT, secure_getenv() is being used and the ordering of creating the formatted path string in util_path_sysfs() is changed slightly. Furthermore, the static variable is removed as no complicated query of the /proc fs is required anymore. Memory for the sysfs mount point value is properly freed now at the end of util_path_sysfs(). [1] https://www.kernel.org/doc/html/latest/admin-guide/sysfs-rules.html Reviewed-by: Ingo Franzki Signed-off-by: Jan Höppner --- libutil/util_path.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/libutil/util_path.c b/libutil/util_path.c index 8d8b1e39..0be3b16a 100644 --- a/libutil/util_path.c +++ b/libutil/util_path.c @@ -20,7 +20,6 @@ #include "lib/util_libc.h" #include "lib/util_path.h" #include "lib/util_prg.h" -#include "lib/util_proc.h" /* * Verify that directory exists @@ -39,28 +38,20 @@ static void verify_dir(const char *dir) /* * Return sysfs mount point + * + * The caller must free the memory for mount_point. */ -static char *sys_mount_point(void) +static void sys_mount_point(char **mount_point) { - struct util_proc_mnt_entry mnt_entry; - static char *mount_point; char *dir; - if (mount_point) - return mount_point; /* Check the environment variable */ - dir = getenv("SYSFS_ROOT"); - if (dir) { - mount_point = util_strdup(dir); - } else { - if (util_proc_mnt_get_entry("/proc/mounts", "sysfs", - &mnt_entry)) - errx(EXIT_FAILURE, "No mount point found for sysfs"); - mount_point = util_strdup(mnt_entry.file); - util_proc_mnt_free_entry(&mnt_entry); - } - verify_dir(mount_point); - return mount_point; + dir = secure_getenv("SYSFS_ROOT"); + if (dir) + *mount_point = util_strdup(dir); + else + *mount_point = util_strdup("/sys"); + verify_dir(*mount_point); } /** @@ -76,15 +67,17 @@ static char *sys_mount_point(void) */ char *util_path_sysfs(const char *fmt, ...) { - char *path, *fmt_tot; + char *path, *fmt_path, *sysfs; va_list ap; - util_asprintf(&fmt_tot, "%s/%s", sys_mount_point(), fmt); - /* Format and return full sysfs path */ va_start(ap, fmt); - util_vasprintf(&path, fmt_tot, ap); + util_vasprintf(&fmt_path, fmt, ap); va_end(ap); - free(fmt_tot); + sys_mount_point(&sysfs); + /* Format and return full sysfs path */ + util_asprintf(&path, "%s/%s", sysfs, fmt_path); + free(fmt_path); + free(sysfs); return path; }