ipl_tools: Refactor read helper using util_file_read_text_file()

Replace read_str() helper in ipl_tools/system.c with
util_file_read_text_file() to enhance maintainability and reduce
code duplication.

Additionally, allocate the buffer dynamically instead of using
fixed-size buffer to prevent potential overflows and data loss.

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>

ipl_tools/cmd_lsreipl.c

@@ -55,11 +55,12 @@ void print_nss(int show_ipl)
 void print_fcp(int show_ipl, int dump)
 {
 	char *dir = show_ipl ? "ipl" : "reipl/fcp";
-	char loadparm[9], loadparm_path[PATH_MAX];
+	char loadparm_path[PATH_MAX];
 	char *path_bootparms = util_path_sysfs("firmware/%s/scp_data", dir);
 	char *path_loadparm = util_path_sysfs("firmware/%s/loadparm", dir);
 	char *path_reipl_clear = util_path_sysfs("firmware/reipl/fcp/clear");
 	char *path_secure_boot = util_path_sysfs("firmware/ipl/secure");
+	char *loadparm;
 
 	if (dump)
 		printf("%-12s fcp_dump\n", get_ipl_banner(show_ipl));
@@ -73,10 +74,11 @@ void print_fcp(int show_ipl, int dump)
 	print_fw_str("br_lba:      %s\n", dir, "br_lba");
 	if (access(path_loadparm, R_OK) == 0) {
 		sprintf(loadparm_path, "%s/%s", dir, "loadparm");
-		read_fw_str(loadparm, loadparm_path, sizeof(loadparm));
+		loadparm = read_fw_str(loadparm_path);
 		if (strcmp(loadparm, "        ") == 0)
 			loadparm[0] = 0;
 		printf("Loadparm:    \"%s\"\n", loadparm);
+		free(loadparm);
 	}
 	if (access(path_bootparms, R_OK) == 0)
 		print_fw_str("Bootparms:   \"%s\"\n", dir, "scp_data");
@@ -93,11 +95,12 @@ void print_fcp(int show_ipl, int dump)
 void print_nvme(int show_ipl, int dump)
 {
 	char *dir = show_ipl ? "ipl" : "reipl/nvme";
-	char loadparm[9], loadparm_path[PATH_MAX];
+	char loadparm_path[PATH_MAX];
 	char *path_bootparms = util_path_sysfs("firmware/%s/scp_data", dir);
 	char *path_loadparm = util_path_sysfs("firmware/%s/loadparm", dir);
 	char *path_reipl_clear = util_path_sysfs("firmware/reipl/nvme/clear");
 	char *path_secure_boot = util_path_sysfs("firmware/ipl/secure");
+	char *loadparm;
 
 	if (dump)
 		printf("%-12s nvme_dump\n", get_ipl_banner(show_ipl));
@@ -110,10 +113,11 @@ void print_nvme(int show_ipl, int dump)
 	print_fw_str("br_lba:      %s\n", dir, "br_lba");
 	if (access(path_loadparm, R_OK) == 0) {
 		sprintf(loadparm_path, "%s/%s", dir, "loadparm");
-		read_fw_str(loadparm, loadparm_path, sizeof(loadparm));
+		loadparm = read_fw_str(loadparm_path);
 		if (strcmp(loadparm, "        ") == 0)
 			loadparm[0] = 0;
 		printf("Loadparm:    \"%s\"\n", loadparm);
+		free(loadparm);
 	}
 	if (access(path_bootparms, R_OK) == 0)
 		print_fw_str("Bootparms:   \"%s\"\n", dir, "scp_data");
@@ -129,20 +133,22 @@ void print_nvme(int show_ipl, int dump)
 
 void print_ccw(int show_ipl)
 {
-	char loadparm[9], loadparm_path[PATH_MAX];
+	char loadparm_path[PATH_MAX];
 	char *dir = show_ipl ? "ipl" : "reipl/ccw";
 	char *path_loadparm = util_path_sysfs("firmware/%s/loadparm", dir);
 	char *path_bootparms = util_path_sysfs("firmware/%s/parm", dir);
 	char *path_reipl_clear = util_path_sysfs("firmware/reipl/ccw/clear");
+	char *loadparm;
 
 	printf("%-12s ccw\n", get_ipl_banner(show_ipl));
 	print_fw_str("Device:      %s\n", dir, "device");
 	if (access(path_loadparm, R_OK) == 0) {
 		sprintf(loadparm_path, "%s/%s", dir, "loadparm");
-		read_fw_str(loadparm, loadparm_path, sizeof(loadparm));
+		loadparm = read_fw_str(loadparm_path);
 		if (strcmp(loadparm, "        ") == 0)
 			loadparm[0] = 0;
 		printf("Loadparm:    \"%s\"\n", loadparm);
+		free(loadparm);
 	}
 	if (access(path_bootparms, R_OK) == 0)
 		print_fw_str("Bootparms:   \"%s\"\n", dir, "parm");
@@ -156,9 +162,10 @@ void print_ccw(int show_ipl)
 void print_eckd(int show_ipl, const char *name)
 {
 	char *dir = show_ipl ? "ipl" : "reipl/eckd";
-	char loadparm[9], loadparm_path[PATH_MAX];
+	char loadparm_path[PATH_MAX];
 	char *path_loadparm = util_path_sysfs("firmware/%s/loadparm", dir);
 	char *path_secure_boot = util_path_sysfs("firmware/ipl/secure");
+	char *loadparm;
 
 	printf("%-12s %s\n", get_ipl_banner(show_ipl), name);
 
@@ -168,10 +175,11 @@ void print_eckd(int show_ipl, const char *name)
 	print_fw_str("Bootparm:    \"%s\"\n", dir, "scp_data");
 	if (access(path_loadparm, R_OK) == 0) {
 		sprintf(loadparm_path, "%s/%s", dir, "loadparm");
-		read_fw_str(loadparm, loadparm_path, sizeof(loadparm));
+		loadparm = read_fw_str(loadparm_path);
 		if (strcmp(loadparm, "        ") == 0)
 			loadparm[0] = 0;
 		printf("Loadparm:    \"%s\"\n", loadparm);
+		free(loadparm);
 	}
 	if (!show_ipl)
 		print_fw_str("clear:       %s\n", dir, "clear");
@@ -212,16 +220,14 @@ static void parse_lsreipl_options(int argc, char *argv[])
 
 void cmd_lsreipl(int argc, char *argv[])
 {
-	char reipl_type_str[1024];
+	char *reipl_type_str;
 
 	parse_lsreipl_options(argc, argv);
 
 	if (l.ipl_set)
-		read_fw_str(reipl_type_str, "ipl/ipl_type",
+		reipl_type_str = read_fw_str("ipl/ipl_type");
-			    sizeof(reipl_type_str));
 	else
-		read_fw_str(reipl_type_str, "reipl/reipl_type",
+		reipl_type_str = read_fw_str("reipl/reipl_type");
-			    sizeof(reipl_type_str));
 
 	if (strcmp(reipl_type_str, "fcp") == 0)
 		print_fcp(l.ipl_set, 0);
@@ -241,5 +247,6 @@ void cmd_lsreipl(int argc, char *argv[])
 	else
 		printf("%s: %s (unknown)\n", get_ipl_banner(l.ipl_set),
 		       reipl_type_str);
+	free(reipl_type_str);
 	exit(0);
 }

ipl_tools/cmd_lsshut.c

@@ -10,6 +10,7 @@
  */
 
 #include "lib/util_path.h"
+#include "lib/util_file.h"
 #include "ipl_tools.h"
 
 static const char *const usage_lsshut =
@@ -61,12 +62,13 @@ static void parse_lsshut_options(int argc, char *argv[])
  */
 static void read_vmcmd(char *str, const char *path)
 {
-	char buf[128], tmp[512];
 	char *ptr_old, *ptr;
+	char tmp[512];
+	char *buf;
 
 	*str = 0;
+	buf = read_fw_str(path);
 	ptr_old = ptr = buf;
-	read_fw_str(buf, path, sizeof(buf));
 	while ((ptr = strchr(ptr_old, '\n'))) {
 		*ptr = 0;
 		sprintf(tmp, "\"%s\",", ptr_old);
@@ -75,28 +77,31 @@ static void read_vmcmd(char *str, const char *path)
 	}
 	sprintf(tmp, "\"%s\"", ptr_old);
 	strcat(str, tmp);
+	free(buf);
 }
 
 static void print_kdump(void)
 {
 	struct stat sb;
-	char tmp[1024];
 	char *path;
+	char *tmp;
 
 	path = util_path_sysfs("kernel/kexec_crash_loaded");
 	if (stat(path, &sb) != 0) {
 		free(path);
 		return;
 	}
-	read_str(tmp, path, sizeof(tmp));
+	tmp = util_file_read_text_file(path, 1);
 	if (strncmp(tmp, "1", 1) == 0)
 		printf("kdump,");
 	free(path);
+	free(tmp);
 }
 
 static void shutdown_trigger_print(struct shutdown_trigger *trigger)
 {
-	char tmp[1024], cmd[1024], path[PATH_MAX];
+	char cmd[1024], path[PATH_MAX];
+	char *tmp;
 
 	sprintf(path, "shutdown_actions/%s", trigger->name_sysfs);
 
@@ -105,7 +110,7 @@ static void shutdown_trigger_print(struct shutdown_trigger *trigger)
 	if ((trigger == &shutdown_trigger_panic ||
 	     trigger == &shutdown_trigger_restart))
 		print_kdump();
-	read_fw_str(tmp, path, sizeof(tmp));
+	tmp = read_fw_str(path);
 	if (strncmp(tmp, "vmcmd", strlen("vmcmd")) == 0) {
 		sprintf(path, "vmcmd/%s", trigger->name_sysfs);
 		read_vmcmd(cmd, path);
@@ -113,6 +118,7 @@ static void shutdown_trigger_print(struct shutdown_trigger *trigger)
 	} else {
 		printf("%s\n", tmp);
 	}
+	free(tmp);
 }
 
 void cmd_lsshut(int argc, char *argv[])

ipl_tools/ipl_tools.h

@@ -54,8 +54,7 @@ extern int is_root(void);
 
 extern void write_str(char *string, char *file);
 extern int write_str_errno(char *string, char *file);
-extern void read_str(char *string, const char *file, size_t len);
+char *read_fw_str(const char *file);
-extern void read_fw_str(char *string, const char *file, size_t len);
 extern void print_fw_str(const char *fmt, const char *dir, const char *file);
 
 extern void __noreturn print_version_exit(void);

ipl_tools/system.c

@@ -10,6 +10,7 @@
  */
 
 #include "lib/util_path.h"
+#include "lib/util_file.h"
 #include "ipl_tools.h"
 
 /*
@@ -46,36 +47,18 @@ int is_root(void)
 		return 0;
 }
 
-/*
- * Read a string from a particular file
- */
-void read_str(char *string, const char *path, size_t len)
-{
-	size_t rc;
-	FILE *fh;
-
-	fh = fopen(path, "rb");
-	if (fh == NULL)
-		ERR_EXIT_ERRNO("Could not open \"%s\"", path);
-	rc = fread(string, 1, len - 1, fh);
-	if (rc == 0 && ferror(fh))
-		ERR_EXIT_ERRNO("Could not read \"%s\"", path);
-	fclose(fh);
-	string[rc] = 0;
-	if (string[strlen(string) - 1] == '\n')
-		string[strlen(string) - 1] = 0;
-}
-
 /*
  * Read a string from a particular /sys/firmware file
  */
-void read_fw_str(char *string, const char *file, size_t len)
+char *read_fw_str(const char *file)
 {
+	char *string;
 	char *path;
 
 	path = util_path_sysfs("firmware/%s", file);
-	read_str(string, path, len);
+	string = util_file_read_text_file(path, 1);
 	free(path);
+	return string;
 }
 
 /*
@@ -83,11 +66,13 @@ void read_fw_str(char *string, const char *file, size_t len)
  */
 void print_fw_str(const char *fmt, const char *dir, const char *file)
 {
-	char path[PATH_MAX], str[4096];
+	char path[PATH_MAX];
+	char *str;
 
 	snprintf(path, sizeof(path), "%s/%s", dir, file);
-	read_fw_str(str, path, sizeof(str));
+	str = read_fw_str(path);
 	printf(fmt, str);
+	free(str);
 }
 
 /*