From 426311f44087c770d94dad3a65b84c718ccf042e Mon Sep 17 00:00:00 2001 From: Jan Polensky Date: Wed, 26 Feb 2025 16:02:10 +0100 Subject: [PATCH] ipl_tools/cmd_chreipl.c: Replace /sys mount point with util_path_sysfs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default sysfs path is always set to '/sys' unless the SYSFS_ROOT environment variable is defined. To address security concerns, secure_getenv() is used within util_path_sysfs() to protect against malicious values in SYSFS_ROOT. Additionally, constructing the sysfs path dynamically in an allocated buffer, rather than using a fixed-size buffer, helps prevent potential buffer overflows. These modifications also significantly improve testability by allowing sysfs read and write operations to be redirected to an alternative file path, which enables testing without affecting the active system state. Signed-off-by: Jan Polensky Reviewed-by: Jan Höppner Signed-off-by: Jan Höppner --- ipl_tools/ccw.c | 39 +++++++++++++++++++++++++-------------- ipl_tools/cmd_chreipl.c | 11 +++++++---- ipl_tools/fcp.c | 28 +++++++++++++++++++--------- ipl_tools/nvme.c | 19 +++++++++++++------ ipl_tools/system.c | 12 ++++++++---- 5 files changed, 72 insertions(+), 37 deletions(-) diff --git a/ipl_tools/ccw.c b/ipl_tools/ccw.c index e386673c..fa19ff14 100644 --- a/ipl_tools/ccw.c +++ b/ipl_tools/ccw.c @@ -49,19 +49,19 @@ static int device_sysfs_path(const char *device, char *path, const size_t path_s */ int ccw_is_device(const char *busid) { - char path[PATH_MAX]; + static const char *const driver_paths[] = { "dasd-eckd", "virtio_ccw", "dasd-fba" }; + char *path; + size_t i; + + for (i = 0; i < ARRAY_SIZE(driver_paths); i++) { + path = util_path_sysfs("bus/ccw/drivers/%s/%s", driver_paths[i], busid); + if (access(path, R_OK) == 0) { + free(path); + return 1; + } + free(path); + } - snprintf(path, sizeof(path), - "/sys/bus/ccw/drivers/dasd-eckd/%s", busid); - if (access(path, R_OK) == 0) - return 1; - snprintf(path, sizeof(path), - "/sys/bus/ccw/drivers/virtio_ccw/%s", busid); - if (access(path, R_OK) == 0) - return 1; - snprintf(path, sizeof(path), "/sys/bus/ccw/drivers/dasd-fba/%s", busid); - if (access(path, R_OK) == 0) - return 1; return 0; } @@ -72,6 +72,7 @@ int ccw_is_virtio_device(const char *device) { char path[PATH_MAX] = { '\0' }; unsigned virtio = 0; + char *path_pattern; if (device_sysfs_path(device, path, sizeof(path)) != 0) return -1; @@ -80,8 +81,12 @@ int ccw_is_virtio_device(const char *device) * The output has the following format: * /sys/devices/css0/0.0.0000/0.0.0000/virtio0/block/vda */ - if (sscanf(path, "/sys/devices/css0/%*[0-9a-f.]/%*[0-9a-f.]/virtio%u", &virtio) != 1) + path_pattern = util_path_sysfs("devices/css0/%%*[0-9a-f.]/%%*[0-9a-f.]/virtio%%u"); + if (sscanf(path, path_pattern, &virtio) != 1) { + free(path_pattern); return -1; + } + free(path_pattern); return 0; } @@ -91,6 +96,7 @@ int ccw_is_virtio_device(const char *device) void ccw_busid_get(const char *device, char *busid) { char path[PATH_MAX] = { '\0' }; + char *path_pattern; if (device_sysfs_path(device, path, sizeof(path)) != 0) ERR_EXIT("Could not lookup device number for \"%s\"", device); @@ -100,7 +106,12 @@ void ccw_busid_get(const char *device, char *busid) * /sys/devices/css0/0.0.0119/0.0.3f19/block/dasda * /sys/devices/css0/0.0.0000/0.0.0000/virtio0/block/vda */ - if (sscanf(path, "/sys/devices/css0/%*[0-9a-f.]/%[0-9a-f.]", busid) != 1) + path_pattern = util_path_sysfs("devices/css0/%%*[0-9a-f.]/%%[0-9a-f.]"); + if (sscanf(path, path_pattern, busid) != 1) { + free(path_pattern); ERR_EXIT("Could not lookup device number for \"%s\"", device); + } + free(path_pattern); + return; } diff --git a/ipl_tools/cmd_chreipl.c b/ipl_tools/cmd_chreipl.c index a51a2dc6..245fdd12 100644 --- a/ipl_tools/cmd_chreipl.c +++ b/ipl_tools/cmd_chreipl.c @@ -15,6 +15,7 @@ #include "lib/util_libc.h" #include "lib/zt_common.h" +#include "lib/util_path.h" #include "ipl_tools.h" #include "proc.h" @@ -719,11 +720,12 @@ static void parse_chreipl_options(int argc, char *argv[]) static void check_exists(const char *path, const char *attr) { - char fpath[PATH_MAX]; + char *fpath; - snprintf(fpath, sizeof(fpath), "/sys/firmware/%s", path); + fpath = util_path_sysfs("firmware/%s", path); if (access(fpath, F_OK) != 0) ERR_EXIT("System does not allow one to set %s", attr); + free(fpath); } static void write_str_optional(char *string, char *file, int exit_on_fail, @@ -920,13 +922,14 @@ static void chreipl_nss(void) static void chreipl_node(void) { - char path[PATH_MAX]; + char *path; if (!l.dev_set) ERR_EXIT("No device node specified"); - snprintf(path, sizeof(path), "/sys/block/%s/device", l.dev); + path = util_path_sysfs("block/%s/device", l.dev); if (chdir(path) != 0) ERR_EXIT("Could not find device \"%s\"", l.dev); + free(path); switch (l.reipl_type) { case REIPL_CCW: diff --git a/ipl_tools/fcp.c b/ipl_tools/fcp.c index 8e23424a..9d9b8659 100644 --- a/ipl_tools/fcp.c +++ b/ipl_tools/fcp.c @@ -10,6 +10,7 @@ */ #include "lib/util_libc.h" +#include "lib/util_path.h" #include "ipl_tools.h" /* @@ -18,11 +19,14 @@ */ int fcp_is_device(const char *devno) { - char path[PATH_MAX]; + char *path; - snprintf(path, sizeof(path), "/sys/bus/ccw/drivers/zfcp/%s", devno); - if (chdir(path) != 0) + path = util_path_sysfs("bus/ccw/drivers/zfcp/%s", devno); + if (chdir(path) != 0) { + free(path); return 0; + } + free(path); return 1; } @@ -31,11 +35,12 @@ int fcp_is_device(const char *devno) */ void fcp_wwpn_get(const char *device, char *wwpn) { - char path[PATH_MAX], buf[20]; + char buf[20]; + char *path; FILE *fh; int rc; - snprintf(path, sizeof(path), "/sys/block/%s/device/wwpn", device); + path = util_path_sysfs("block/%s/device/wwpn", device); fh = fopen(path, "r"); if (fh == NULL) ERR_EXIT_ERRNO("Could not open \"%s\"", path); @@ -44,6 +49,7 @@ void fcp_wwpn_get(const char *device, char *wwpn) ERR_EXIT("Could not lookup WWPN \"%s\"", path); util_strlcpy(wwpn, buf, 20); fclose(fh); + free(path); } /* @@ -51,11 +57,12 @@ void fcp_wwpn_get(const char *device, char *wwpn) */ void fcp_lun_get(const char *device, char *lun) { - char path[PATH_MAX], buf[20]; + char buf[20]; + char *path; FILE *fh; int rc; - snprintf(path, sizeof(path), "/sys/block/%s/device/fcp_lun", device); + path = util_path_sysfs("block/%s/device/fcp_lun", device); fh = fopen(path, "r"); if (fh == NULL) ERR_EXIT_ERRNO("Could not open \"%s\"", path); @@ -64,6 +71,7 @@ void fcp_lun_get(const char *device, char *lun) ERR_EXIT("Could not lookup LUN \"%s\"", path); util_strlcpy(lun, buf, 20); fclose(fh); + free(path); } /* @@ -71,11 +79,12 @@ void fcp_lun_get(const char *device, char *lun) */ void fcp_busid_get(const char *device, char *devno) { - char buf[4096], path[PATH_MAX]; + char buf[4096]; + char *path; FILE *fh; int rc; - snprintf(path, sizeof(path), "/sys/block/%s/device/hba_id", device); + path = util_path_sysfs("block/%s/device/hba_id", device); fh = fopen(path, "r"); if (fh == NULL) ERR_EXIT_ERRNO("Could not open \"%s\"", path); @@ -84,4 +93,5 @@ void fcp_busid_get(const char *device, char *devno) ERR_EXIT("Could not find device \"%s\"", path); strcpy(devno, buf); fclose(fh); + free(path); } diff --git a/ipl_tools/nvme.c b/ipl_tools/nvme.c index 05c2b87c..7d84fc77 100644 --- a/ipl_tools/nvme.c +++ b/ipl_tools/nvme.c @@ -16,6 +16,7 @@ #include "lib/util_libc.h" #include "lib/util_file.h" +#include "lib/util_path.h" #include "ipl_tools.h" static void nvme_dev_from_bdev(char *dev_name) @@ -31,8 +32,9 @@ static void nvme_dev_from_bdev(char *dev_name) */ void nvme_fid_get(const char *device, char *fid) { - char path[PATH_MAX], buf[FID_MAX_LEN]; char nvme_dev[NVME_DEV_MAX_LEN]; + char buf[FID_MAX_LEN]; + char *path; /* * An NVMe may present multiple namespaces and thus block devices, even @@ -42,24 +44,27 @@ void nvme_fid_get(const char *device, char *fid) util_strlcpy(nvme_dev, device, sizeof(nvme_dev)); nvme_dev_from_bdev(nvme_dev); - snprintf(path, PATH_MAX, "/sys/class/nvme/%s/device/function_id", nvme_dev); + path = util_path_sysfs("class/nvme/%s/device/function_id", nvme_dev); if (util_file_read_line(buf, FID_MAX_LEN, path)) ERR_EXIT_ERRNO("Could not read from \"%s\"", path); util_strlcpy(fid, buf, FID_MAX_LEN); + free(path); } /* * Return the nsid of a device */ void nvme_nsid_get(const char *device, char *nsid) { - char path[PATH_MAX], buf[FID_MAX_LEN]; + char buf[FID_MAX_LEN]; + char *path; - snprintf(path, PATH_MAX, "/sys/block/%s/nsid", device); + path = util_path_sysfs("block/%s/nsid", device); if (util_file_read_line(buf, FID_MAX_LEN, path)) ERR_EXIT_ERRNO("Could not read from \"%s\"", path); util_strlcpy(nsid, buf, FID_MAX_LEN); + free(path); } static int next_entry(DIR *dir, char *in_path, char *out_path, @@ -93,13 +98,14 @@ static int next_entry(DIR *dir, char *in_path, char *out_path, static int nvme_getdev_by_fid(char *fidstr, char *devpath) { - char temp_path[PATH_MAX+19], real_path[PATH_MAX]; - char *sys_path = "/sys/class/nvme"; + char temp_path[PATH_MAX + 19], real_path[PATH_MAX]; u_int64_t target_fid, curfid; + char *sys_path; DIR *dir; char *end; int rc = -1; + sys_path = util_path_sysfs("class/nvme"); target_fid = strtoul(fidstr, &end, 16); if (*end) ERR_EXIT("Invalid function_id given %s", fidstr); @@ -126,6 +132,7 @@ static int nvme_getdev_by_fid(char *fidstr, char *devpath) } closedir(dir); + free(sys_path); return rc; } diff --git a/ipl_tools/system.c b/ipl_tools/system.c index cd4fc1b8..e9a41544 100644 --- a/ipl_tools/system.c +++ b/ipl_tools/system.c @@ -106,17 +106,19 @@ void print_fw_str(const char *fmt, const char *dir, const char *file) */ void write_str(char *string, char *file) { - char path[PATH_MAX], value[4096]; + char value[4096]; + char *path; int fh; + path = util_path_sysfs("firmware/%s", file); snprintf(value, sizeof(value), "%s\n", string); - snprintf(path, sizeof(path), "/sys/firmware/%s", file); fh = open(path, O_WRONLY); if (fh < 0) ERR_EXIT_ERRNO("Could not open \"%s\"", file); if (write(fh, value, strlen(value)) < 0) ERR_EXIT_ERRNO("Could not set \"%s\"", file); close(fh); + free(path); } /* @@ -124,16 +126,18 @@ void write_str(char *string, char *file) */ int write_str_errno(char *string, char *file) { - char path[PATH_MAX], value[4096]; + char value[4096]; + char *path; int fh; + path = util_path_sysfs("firmware/%s", file); snprintf(value, sizeof(value), "%s\n", string); - snprintf(path, sizeof(path), "/sys/firmware/%s", file); fh = open(path, O_WRONLY); if (fh < 0) return errno; if (write(fh, value, strlen(value)) < 0) return errno; close(fh); + free(path); return 0; }