From c38b668fc3ead664a35130c1acec63518630d1dc Mon Sep 17 00:00:00 2001
From: Zdenek Kabelac <zkabelac@redhat.com>
Date: Mon, 10 Jul 2023 21:44:24 +0200
Subject: [PATCH] debug: fix parsing of /proc/self/stat

The code in init_log_file relies on the process name (COMM) to not
contain whitespaces. This change fixes it by looking up the right-most
parenthesis to safely jump past COMM.

For more context see:
https://www.openwall.com/lists/oss-security/2022/12/21/6

Code is only used with testing, so it should have no impact on regular
users.

Reported-by: Hugues Evrard <hevrard@google.com>

mm
---
 lib/log/log.c | 76 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/lib/log/log.c b/lib/log/log.c
index 118a3ba42..0b79ed16d 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -217,6 +217,45 @@ void init_log_fn(lvm2_log_fn_t log_fn)
 	_lvm2_log_fn = log_fn;
 }
 
+/* Read /proc/self/stat to extract pid and starttime */
+static int _get_pid_starttime(int *pid, unsigned long long *starttime)
+{
+	static const char statfile[] = "/proc/self/stat";
+	char buf[1024];
+	char *p;
+	int fd;
+	int e;
+
+	if ((fd = open(statfile, O_RDONLY)) == -1) {
+		log_sys_debug("open", statfile);
+		return 0;
+	}
+
+	if ((e = read(fd, buf, sizeof(buf) - 1)) <= 0)
+		log_sys_debug("read", statfile);
+
+	if (!close(fd))
+		log_sys_debug("close", statfile);
+
+	if (e <= 0)
+		return 0;
+
+	buf[e] = '\0';
+	if ((sscanf(buf, "%d ", pid) == 1) &&
+	    /* Jump past COMM, don't use scanf with '%s' since COMM may contain a space. */
+	    (p = strrchr(buf, ')')) &&
+	    (scanf(++p, " %*c %*d %*d %*d %*d " /* tty_nr */
+		   "%*d %*u %*u %*u %*u " /* mjflt */
+		   "%*u %*u %*u %*d %*d " /* cstim */
+		   "%*d %*d %*d %*d " /* itrealvalue */
+		   "%llu", &starttime) == 1))
+		return 1;
+
+	log_debug("Cannot parse content of %s.", statfile);
+
+	return 0;
+}
+
 /*
  * Support envvar LVM_LOG_FILE_EPOCH and allow to attach
  * extra keyword (consist of upto 32 alpha chars) to
@@ -228,11 +267,9 @@ void init_log_fn(lvm2_log_fn_t log_fn)
  */
 void init_log_file(const char *log_file, int append)
 {
-	static const char statfile[] = "/proc/self/stat";
 	const char *env;
-	int pid;
-	unsigned long long starttime;
-	FILE *st;
+	int pid = 0;
+	unsigned long long starttime = 0;
 	int i = 0;
 
 	_log_file_path[0] = '\0';
@@ -245,27 +282,17 @@ void init_log_file(const char *log_file, int append)
 			goto no_epoch;
 		}
 
-		if (!(st = fopen(statfile, "r")))
-			log_sys_error("fopen", statfile);
-		else if (fscanf(st, "%d %*s %*c %*d %*d %*d %*d " /* tty_nr */
-			   "%*d %*u %*u %*u %*u " /* mjflt */
-			   "%*u %*u %*u %*d %*d " /* cstim */
-			   "%*d %*d %*d %*d " /* itrealvalue */
-			   "%llu", &pid, &starttime) != 2) {
-			log_warn("WARNING: Cannot parse content of %s.", statfile);
-		} else {
-			if (dm_snprintf(_log_file_path, sizeof(_log_file_path),
-					"%s_%s_%d_%llu", log_file, env, pid, starttime) < 0) {
-				log_warn("WARNING: Debug log file path is too long for epoch.");
-				_log_file_path[0] = '\0';
-			} else {
-				log_file = _log_file_path;
-				append = 1; /* force */
-			}
-		}
+		if (!_get_pid_starttime(&pid, &starttime))
+			log_debug("Failed to obtain pid and starttime.");
 
-		if (st && fclose(st))
-			log_sys_debug("fclose", statfile);
+		if (dm_snprintf(_log_file_path, sizeof(_log_file_path),
+				"%s_%s_%d_%llu", log_file, env, pid, starttime) < 0) {
+			log_warn("WARNING: Debug log file path is too long for epoch.");
+			_log_file_path[0] = '\0';
+		} else {
+			log_file = _log_file_path;
+			append = 1; /* force */
+		}
 
 		if ((env = getenv("LVM_LOG_FILE_MAX_LINES"))) {
 			if (sscanf(env, FMTu64, &_log_file_max_lines) != 1) {
@@ -896,4 +923,3 @@ uint32_t log_journal_str_to_val(const char *str)
 	log_warn("Ignoring unrecognized journal value.");
 	return 0;
 }
-