linux-kernel - [PATCH 01/10] bcachefs: thread_with_stdio: eliminate double buffering

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: <170873668902.1861398.15653563093468101671.stgit@frogsfrogsfrogs>
Date: Fri, 23 Feb 2024 17:14:00 -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 01/10] bcachefs: thread_with_stdio: eliminate double buffering

From: Kent Overstreet <kent.overstreet@...ux.dev>

The output buffer lock has to be a spinlock so that we can write to it
from interrupt context, so we can't use a direct copy_to_user; this
switches thread_with_file_read() to use fault_in_writeable() and
copy_to_user_nofault(), similar to how thread_with_file_write() works.

Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>
Reviewed-by: Darrick J. Wong <djwong@...nel.org>
Signed-off-by: Darrick J. Wong <djwong@...nel.org>
---
 fs/bcachefs/thread_with_file.c |   56 ++++++++++++----------------------------
 fs/bcachefs/thread_with_file.h |    1 -
 2 files changed, 17 insertions(+), 40 deletions(-)


diff --git a/fs/bcachefs/thread_with_file.c b/fs/bcachefs/thread_with_file.c
index 9220d7de10db6..8c3afb4c3204f 100644
--- a/fs/bcachefs/thread_with_file.c
+++ b/fs/bcachefs/thread_with_file.c
@@ -67,16 +67,15 @@ int bch2_run_thread_with_file(struct thread_with_file *thr,
 
 static inline bool thread_with_stdio_has_output(struct thread_with_stdio *thr)
 {
-	return thr->stdio.output_buf.pos ||
-		thr->output2.nr ||
-		thr->thr.done;
+	return thr->stdio.output_buf.pos || thr->thr.done;
 }
 
-static ssize_t thread_with_stdio_read(struct file *file, char __user *buf,
+static ssize_t thread_with_stdio_read(struct file *file, char __user *ubuf,
 				      size_t len, loff_t *ppos)
 {
 	struct thread_with_stdio *thr =
 		container_of(file->private_data, struct thread_with_stdio, thr);
+	struct printbuf *buf = &thr->stdio.output_buf;
 	size_t copied = 0, b;
 	int ret = 0;
 
@@ -89,44 +88,25 @@ static ssize_t thread_with_stdio_read(struct file *file, char __user *buf,
 	if (ret)
 		return ret;
 
-	if (thr->thr.done)
-		return 0;
-
-	while (len) {
-		ret = darray_make_room(&thr->output2, thr->stdio.output_buf.pos);
-		if (ret)
+	while (len && buf->pos) {
+		if (fault_in_writeable(ubuf, len) == len) {
+			ret = -EFAULT;
 			break;
+		}
 
 		spin_lock_irq(&thr->stdio.output_lock);
-		b = min_t(size_t, darray_room(thr->output2), thr->stdio.output_buf.pos);
+		b = min_t(size_t, len, buf->pos);
 
-		memcpy(&darray_top(thr->output2), thr->stdio.output_buf.buf, b);
-		memmove(thr->stdio.output_buf.buf,
-			thr->stdio.output_buf.buf + b,
-			thr->stdio.output_buf.pos - b);
-
-		thr->output2.nr += b;
-		thr->stdio.output_buf.pos -= b;
+		if (b && !copy_to_user_nofault(ubuf, buf->buf, b)) {
+			memmove(buf->buf,
+				buf->buf + b,
+				buf->pos - b);
+			buf->pos -= b;
+			ubuf	+= b;
+			len	-= b;
+			copied	+= b;
+		}
 		spin_unlock_irq(&thr->stdio.output_lock);
-
-		b = min(len, thr->output2.nr);
-		if (!b)
-			break;
-
-		b -= copy_to_user(buf, thr->output2.data, b);
-		if (!b) {
-			ret = -EFAULT;
-			break;
-		}
-
-		copied	+= b;
-		buf	+= b;
-		len	-= b;
-
-		memmove(thr->output2.data,
-			thr->output2.data + b,
-			thr->output2.nr - b);
-		thr->output2.nr -= b;
 	}
 
 	return copied ?: ret;
@@ -140,7 +120,6 @@ static int thread_with_stdio_release(struct inode *inode, struct file *file)
 	bch2_thread_with_file_exit(&thr->thr);
 	printbuf_exit(&thr->stdio.input_buf);
 	printbuf_exit(&thr->stdio.output_buf);
-	darray_exit(&thr->output2);
 	thr->exit(thr);
 	return 0;
 }
@@ -245,7 +224,6 @@ int bch2_run_thread_with_stdio(struct thread_with_stdio *thr,
 	spin_lock_init(&thr->stdio.output_lock);
 	init_waitqueue_head(&thr->stdio.output_wait);
 
-	darray_init(&thr->output2);
 	thr->exit = exit;
 
 	return bch2_run_thread_with_file(&thr->thr, &thread_with_stdio_fops, fn);
diff --git a/fs/bcachefs/thread_with_file.h b/fs/bcachefs/thread_with_file.h
index 05879c5048c87..b5098b52db709 100644
--- a/fs/bcachefs/thread_with_file.h
+++ b/fs/bcachefs/thread_with_file.h
@@ -20,7 +20,6 @@ int bch2_run_thread_with_file(struct thread_with_file *,
 struct thread_with_stdio {
 	struct thread_with_file	thr;
 	struct stdio_redirect	stdio;
-	DARRAY(char)		output2;
 	void			(*exit)(struct thread_with_stdio *);
 };