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-next>] [day] [month] [year] [list]
Message-ID: <20250201195044.28007-1-aha310510@gmail.com>
Date: Sun,  2 Feb 2025 04:50:44 +0900
From: Jeongjun Park <aha310510@...il.com>
To: kent.overstreet@...ux.dev
Cc: linux-bcachefs@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jeongjun Park <aha310510@...il.com>
Subject: [PATCH v3] bcachefs: fix deadlock in journal_entry_open()

In the previous commit b3d82c2f2761, code was added to prevent journal sequence
overflow. Among them, the code added to journal_entry_open() uses the
bch2_fs_fatal_err_on() function to handle errors.

However, __journal_res_get() , which calls journal_entry_open() , calls
journal_entry_open() while holding journal->lock , but bch2_fs_fatal_err_on()
internally tries to acquire journal->lock , which results in a deadlock.

So we need to add a locked helper like bch2_journal_halt_locked() to handle
fatal errors even when the journal->lock is held.

Fixes: b3d82c2f2761 ("bcachefs: Guard against journal seq overflow")
Signed-off-by: Jeongjun Park <aha310510@...il.com>
---
 fs/bcachefs/error.c    |  6 +++---
 fs/bcachefs/fs-ioctl.c |  4 ++--
 fs/bcachefs/journal.c  | 22 +++++++++++++++++-----
 fs/bcachefs/journal.h  |  3 ++-
 fs/bcachefs/recovery.c |  2 +-
 fs/bcachefs/super.c    |  4 ++--
 fs/bcachefs/super.h    |  2 +-
 7 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
index 038da6a61f6b..2482eb33b1ec 100644
--- a/fs/bcachefs/error.c
+++ b/fs/bcachefs/error.c
@@ -20,7 +20,7 @@ bool bch2_inconsistent_error(struct bch_fs *c)
 		return false;
 	case BCH_ON_ERROR_fix_safe:
 	case BCH_ON_ERROR_ro:
-		if (bch2_fs_emergency_read_only(c))
+		if (bch2_fs_emergency_read_only(c, false))
 			bch_err(c, "inconsistency detected - emergency read only at journal seq %llu",
 				journal_cur_seq(&c->journal));
 		return true;
@@ -46,7 +46,7 @@ int bch2_topology_error(struct bch_fs *c)
 
 void bch2_fatal_error(struct bch_fs *c)
 {
-	if (bch2_fs_emergency_read_only(c))
+	if (bch2_fs_emergency_read_only(c, false))
 		bch_err(c, "fatal error - emergency read only");
 }
 
@@ -62,7 +62,7 @@ void bch2_io_error_work(struct work_struct *work)
 	if (dev
 	    ? __bch2_dev_set_state(c, ca, BCH_MEMBER_STATE_ro,
 				  BCH_FORCE_IF_DEGRADED)
-	    : bch2_fs_emergency_read_only(c))
+	    : bch2_fs_emergency_read_only(c, false))
 		bch_err(ca,
 			"too many IO errors, setting %s RO",
 			dev ? "device" : "filesystem");
diff --git a/fs/bcachefs/fs-ioctl.c b/fs/bcachefs/fs-ioctl.c
index 15725b4ce393..92becb6ae5aa 100644
--- a/fs/bcachefs/fs-ioctl.c
+++ b/fs/bcachefs/fs-ioctl.c
@@ -354,14 +354,14 @@ static int bch2_ioc_goingdown(struct bch_fs *c, u32 __user *arg)
 		if (ret)
 			break;
 		bch2_journal_flush(&c->journal);
-		bch2_fs_emergency_read_only(c);
+		bch2_fs_emergency_read_only(c, false);
 		bdev_thaw(c->vfs_sb->s_bdev);
 		break;
 	case FSOP_GOING_FLAGS_LOGFLUSH:
 		bch2_journal_flush(&c->journal);
 		fallthrough;
 	case FSOP_GOING_FLAGS_NOLOGFLUSH:
-		bch2_fs_emergency_read_only(c);
+		bch2_fs_emergency_read_only(c, false);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/fs/bcachefs/journal.c b/fs/bcachefs/journal.c
index 2cd20114b74b..4b3bdbfd7782 100644
--- a/fs/bcachefs/journal.c
+++ b/fs/bcachefs/journal.c
@@ -310,14 +310,23 @@ static void __journal_entry_close(struct journal *j, unsigned closed_val, bool t
 	__bch2_journal_buf_put(j, old.idx, le64_to_cpu(buf->data->seq));
 }
 
-void bch2_journal_halt(struct journal *j)
+void bch2_journal_halt_locked(struct journal *j)
 {
-	spin_lock(&j->lock);
+	lockdep_assert_held(&j->lock);
+
 	__journal_entry_close(j, JOURNAL_ENTRY_ERROR_VAL, true);
 	if (!j->err_seq)
 		j->err_seq = journal_cur_seq(j);
 	journal_wake(j);
-	spin_unlock(&j->lock);
+}
+
+void bch2_journal_halt(struct journal *j, bool locked)
+{
+	if (!locked)
+		spin_lock(&j->lock);
+	bch2_journal_halt_locked(j);
+	if (!locked)
+		spin_unlock(&j->lock);
 }
 
 static bool journal_entry_want_write(struct journal *j)
@@ -382,9 +391,12 @@ static int journal_entry_open(struct journal *j)
 	if (nr_unwritten_journal_entries(j) == ARRAY_SIZE(j->buf))
 		return JOURNAL_ERR_max_in_flight;
 
-	if (bch2_fs_fatal_err_on(journal_cur_seq(j) >= JOURNAL_SEQ_MAX,
-				 c, "cannot start: journal seq overflow"))
+	if (journal_cur_seq(j) >= JOURNAL_SEQ_MAX) {
+		bch_err(c, "cannot start: journal seq overflow");
+		if (bch2_fs_emergency_read_only(c, true))
+			bch_err(c, "fatal error - emergency read only");
 		return JOURNAL_ERR_insufficient_devices; /* -EROFS */
+	}
 
 	BUG_ON(!j->cur_entry_sectors);
 
diff --git a/fs/bcachefs/journal.h b/fs/bcachefs/journal.h
index cb0df0663946..23f5fd970b7d 100644
--- a/fs/bcachefs/journal.h
+++ b/fs/bcachefs/journal.h
@@ -407,7 +407,8 @@ int bch2_journal_flush(struct journal *);
 bool bch2_journal_noflush_seq(struct journal *, u64, u64);
 int bch2_journal_meta(struct journal *);
 
-void bch2_journal_halt(struct journal *);
+void bch2_journal_halt(struct journal *, bool);
+void bch2_journal_halt_locked(struct journal *);
 
 static inline int bch2_journal_error(struct journal *j)
 {
diff --git a/fs/bcachefs/recovery.c b/fs/bcachefs/recovery.c
index 98825437381c..31549181888c 100644
--- a/fs/bcachefs/recovery.c
+++ b/fs/bcachefs/recovery.c
@@ -1076,7 +1076,7 @@ int bch2_fs_recovery(struct bch_fs *c)
 	return ret;
 err:
 fsck_err:
-	bch2_fs_emergency_read_only(c);
+	bch2_fs_emergency_read_only(c, false);
 	goto out;
 }
 
diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index d97ea7bd1171..d3d11fb39632 100644
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -400,11 +400,11 @@ static void bch2_fs_read_only_async(struct bch_fs *c)
 	queue_work(system_long_wq, &c->read_only_work);
 }
 
-bool bch2_fs_emergency_read_only(struct bch_fs *c)
+bool bch2_fs_emergency_read_only(struct bch_fs *c, bool locked)
 {
 	bool ret = !test_and_set_bit(BCH_FS_emergency_ro, &c->flags);
 
-	bch2_journal_halt(&c->journal);
+	bch2_journal_halt(&c->journal, locked);
 	bch2_fs_read_only_async(c);
 
 	wake_up(&bch2_read_only_wait);
diff --git a/fs/bcachefs/super.h b/fs/bcachefs/super.h
index fa6d52216510..e9ff83e06ca2 100644
--- a/fs/bcachefs/super.h
+++ b/fs/bcachefs/super.h
@@ -28,7 +28,7 @@ int bch2_dev_offline(struct bch_fs *, struct bch_dev *, int);
 int bch2_dev_resize(struct bch_fs *, struct bch_dev *, u64);
 struct bch_dev *bch2_dev_lookup(struct bch_fs *, const char *);
 
-bool bch2_fs_emergency_read_only(struct bch_fs *);
+bool bch2_fs_emergency_read_only(struct bch_fs *, bool);
 void bch2_fs_read_only(struct bch_fs *);
 
 int bch2_fs_read_write(struct bch_fs *);
--

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ