linux-kernel - [PATCH 5/7] bcachefs: Move prefaulting out of hot write path

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: <20250129181758.ACB89DF8@davehans-spike.ostc.intel.com>
Date: Wed, 29 Jan 2025 10:17:58 -0800
From: Dave Hansen <dave.hansen@...ux.intel.com>
To: linux-kernel@...r.kernel.org
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,Ted Ts'o <tytso@....edu>,Christian Brauner <brauner@...nel.org>,Darrick J. Wong <djwong@...nel.org>,Matthew Wilcox (Oracle) <willy@...radead.org>,Al Viro <viro@...iv.linux.org.uk>,linux-fsdevel@...r.kernel.org,Dave Hansen <dave.hansen@...ux.intel.com>,kent.overstreet@...ux.dev,linux-bcachefs@...r.kernel.org
Subject: [PATCH 5/7] bcachefs: Move prefaulting out of hot write path

From: Dave Hansen <dave.hansen@...ux.intel.com>

Prefaulting the write source buffer incurs an extra userspace access
in the common fast path. Make bch2_buffered_write() consistent with
generic_perform_write(): only touch userspace an extra time when
copy_page_from_iter_atomic() has failed to make progress.

This also zaps a comment. It referred to a possible deadlock and to
userspace address checks. Neither of those things are a concern when
using copy_folio_from_iter_atomic() for atomic usercopies. It
prevents deadlocks by disabling page faults and it leverages user
copy functions that have their own access_ok() checks.

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: Kent Overstreet <kent.overstreet@...ux.dev>
Cc: linux-bcachefs@...r.kernel.org

---

 b/fs/bcachefs/fs-io-buffered.c |   30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff -puN fs/bcachefs/fs-io-buffered.c~bcachefs-postfault fs/bcachefs/fs-io-buffered.c
--- a/fs/bcachefs/fs-io-buffered.c~bcachefs-postfault	2025-01-29 09:03:35.727656612 -0800
+++ b/fs/bcachefs/fs-io-buffered.c	2025-01-29 09:03:35.731656945 -0800
@@ -970,26 +970,6 @@ static ssize_t bch2_buffered_write(struc
 		unsigned offset = pos & (PAGE_SIZE - 1);
 		unsigned bytes = iov_iter_count(iter);
 again:
-		/*
-		 * Bring in the user page that we will copy from _first_.
-		 * Otherwise there's a nasty deadlock on copying from the
-		 * same page as we're writing to, without it being marked
-		 * up-to-date.
-		 *
-		 * Not only is this an optimisation, but it is also required
-		 * to check that the address is actually valid, when atomic
-		 * usercopies are used, below.
-		 */
-		if (unlikely(fault_in_iov_iter_readable(iter, bytes))) {
-			bytes = min_t(unsigned long, iov_iter_count(iter),
-				      PAGE_SIZE - offset);
-
-			if (unlikely(fault_in_iov_iter_readable(iter, bytes))) {
-				ret = -EFAULT;
-				break;
-			}
-		}
-
 		if (unlikely(fatal_signal_pending(current))) {
 			ret = -EINTR;
 			break;
@@ -1012,6 +992,16 @@ again:
 			 */
 			bytes = min_t(unsigned long, PAGE_SIZE - offset,
 				      iov_iter_single_seg_count(iter));
+
+			/*
+			 * Faulting in 'iter' may be required for forward
+			 * progress. Do it here, out outside the fast path
+			 * and when not holding any folio locks.
+			 */
+			if (fault_in_iov_iter_readable(iter, bytes) == bytes) {
+				ret = -EFAULT;
+				break;
+			}
 			goto again;
 		}
 		pos += ret;
_