linux-kernel - [PATCH 2/5] thread_with_file: fix various printf problems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <170873669433.1861696.1893338653788145361.stgit@frogsfrogsfrogs>
Date: Fri, 23 Feb 2024 17:16:52 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: akpm@...ux-foundation.org, daniel@...o.nz, kent.overstreet@...ux.dev,
 djwong@...nel.org
Cc: linux-xfs@...r.kernel.org, linux-bcachefs@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: [PATCH 2/5] thread_with_file: fix various printf problems

From: Darrick J. Wong <djwong@...nel.org>

Experimentally fix some problems with stdio_redirect_vprintf by creating
a MOO variant with which we can experiment.  We can't do a GFP_KERNEL
allocation while holding the spinlock, and I don't like how the printf
function can silently truncate the output if memory allocation fails.

Signed-off-by: Darrick J. Wong <djwong@...nel.org>
Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>
---
 include/linux/thread_with_file.h |    4 +--
 lib/thread_with_file.c           |   55 ++++++++++++++++++++++++++------------
 2 files changed, 39 insertions(+), 20 deletions(-)


diff --git a/include/linux/thread_with_file.h b/include/linux/thread_with_file.h
index 5f7e85bc8322b..7b133a15d3540 100644
--- a/include/linux/thread_with_file.h
+++ b/include/linux/thread_with_file.h
@@ -68,7 +68,7 @@ int run_thread_with_stdout(struct thread_with_stdio *,
 int stdio_redirect_read(struct stdio_redirect *, char *, size_t);
 int stdio_redirect_readline(struct stdio_redirect *, char *, size_t);
 
-__printf(3, 0) void stdio_redirect_vprintf(struct stdio_redirect *, bool, const char *, va_list);
-__printf(3, 4) void stdio_redirect_printf(struct stdio_redirect *, bool, const char *, ...);
+__printf(3, 0) ssize_t stdio_redirect_vprintf(struct stdio_redirect *, bool, const char *, va_list);
+__printf(3, 4) ssize_t stdio_redirect_printf(struct stdio_redirect *, bool, const char *, ...);
 
 #endif /* _LINUX_THREAD_WITH_FILE_H */
diff --git a/lib/thread_with_file.c b/lib/thread_with_file.c
index 71028611b8d59..70a805ef017f9 100644
--- a/lib/thread_with_file.c
+++ b/lib/thread_with_file.c
@@ -108,49 +108,68 @@ int stdio_redirect_readline(struct stdio_redirect *stdio, char *ubuf, size_t len
 EXPORT_SYMBOL_GPL(stdio_redirect_readline);
 
 __printf(3, 0)
-static void darray_vprintf(darray_char *out, gfp_t gfp, const char *fmt, va_list args)
+static ssize_t darray_vprintf(darray_char *out, gfp_t gfp, const char *fmt, va_list args)
 {
-	size_t len;
+	ssize_t ret;
 
 	do {
 		va_list args2;
+		size_t len;
+
 		va_copy(args2, args);
-
 		len = vsnprintf(out->data + out->nr, darray_room(*out), fmt, args2);
-	} while (len + 1 > darray_room(*out) && !darray_make_room_gfp(out, len + 1, gfp));
+		if (len + 1 <= darray_room(*out)) {
+			out->nr += len;
+			return len;
+		}
 
-	out->nr += min(len, darray_room(*out));
+		ret = darray_make_room_gfp(out, len + 1, gfp);
+	} while (ret == 0);
+
+	return ret;
 }
 
-void stdio_redirect_vprintf(struct stdio_redirect *stdio, bool nonblocking,
-			    const char *fmt, va_list args)
+ssize_t stdio_redirect_vprintf(struct stdio_redirect *stdio, bool nonblocking,
+			       const char *fmt, va_list args)
 {
 	struct stdio_buf *buf = &stdio->output;
 	unsigned long flags;
+	ssize_t ret;
 
-	if (!nonblocking)
-		wait_event(buf->wait, stdio_redirect_has_output_space(stdio));
-	else if (!stdio_redirect_has_output_space(stdio))
-		return;
-	if (stdio->done)
-		return;
-
+again:
 	spin_lock_irqsave(&buf->lock, flags);
-	darray_vprintf(&buf->buf, nonblocking ? GFP_NOWAIT : GFP_KERNEL, fmt, args);
+	ret = darray_vprintf(&buf->buf, GFP_NOWAIT, fmt, args);
 	spin_unlock_irqrestore(&buf->lock, flags);
 
+	if (ret < 0) {
+		if (nonblocking)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(buf->wait,
+				stdio_redirect_has_output_space(stdio));
+		if (ret)
+			return ret;
+		goto again;
+	}
+
 	wake_up(&buf->wait);
+	return ret;
+
 }
 EXPORT_SYMBOL_GPL(stdio_redirect_vprintf);
 
-void stdio_redirect_printf(struct stdio_redirect *stdio, bool nonblocking,
-			   const char *fmt, ...)
+ssize_t stdio_redirect_printf(struct stdio_redirect *stdio, bool nonblocking,
+			      const char *fmt, ...)
 {
 
 	va_list args;
+	ssize_t ret;
+
 	va_start(args, fmt);
-	stdio_redirect_vprintf(stdio, nonblocking, fmt, args);
+	ret = stdio_redirect_vprintf(stdio, nonblocking, fmt, args);
 	va_end(args);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(stdio_redirect_printf);