[PATCH] jffs2: keep alloc_sem held when a write retry cannot reserve space

[PATCH] jffs2: keep alloc_sem held when a write retry cannot reserve space

[Song jeonghak]

When writing a node fails, jffs2_write_dnode() and jffs2_write_dirent()
retry once in a newly reserved area (jffs2_write_dnode() does not retry
for ALLOC_NORETRY). For allocation modes other than ALLOC_GC they first
drop f->sem and complete the current reservation, which releases
c->alloc_sem, and then call jffs2_reserve_space() again.

jffs2_reserve_space() returns with alloc_sem held only on success. If
the new reservation fails, both functions return an error without
holding alloc_sem. Their callers expect alloc_sem to still be held on
error and call jffs2_complete_reservation(), so alloc_sem is released
twice. syzbot reported:

  WARNING: bad unlock balance detected!
  syz-executor317/5670 is trying to release lock (&c->alloc_sem) at:
  jffs2_do_create+0x1ae/0xe00 fs/jffs2/write.c:474
  but there are no more locks to release!

As mutex_unlock() does not refuse an unlock from a task that does not
own the mutex, the second unlock can also release alloc_sem while
another task holds it. This follows from reading the code and was not
observed in testing.

Take alloc_sem again when the retry reservation fails, before f->sem is
taken again, so that the lock order documented in fs/jffs2/README.Locking
(alloc_sem before f->sem) is kept. jffs2_do_unlink() already takes
alloc_sem without a reservation in the same way. If the reservation was
interrupted by a signal, this waits for alloc_sem uninterruptibly, as
jffs2_reserve_space() itself does when it first takes it.

Tested with the syzbot reproducer on mtdram: the warning is gone while
the failing path is still reached. The jffs2_write_dirent() path was
checked by code review only.

Claude (Anthropic) was used to help with the root cause analysis, the
reproduction setup and drafting this changelog. The fix in
jffs2_write_dnode() was written by the submitter; the assistant adjusted
its comment and coding style and applied the same change to
jffs2_write_dirent().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+250fde257a3eebba4426@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=250fde257a3eebba4426
Cc: stable@vger.kernel.org
Signed-off-by: Song jeonghak <thdwjdgkr123@gmail.com>
---
Testing details: next-20261023 with lockdep, 128 KiB mtdram with 4 KiB
erase blocks. The warning was reported in 6 of 7 runs before the change
(the cause of the one miss was not identified) and in 0 of 3 runs after
it; in all three runs after the change the failed write and the -ENOSPC
from creat() are still seen and no other lockdep report appears. A basic
test on an empty mtdram device (mkdir, create, 20 KiB write, rename,
symlink, unlink, remount, filling the device to ENOSPC, remount) passes
before and after. Not tested on NAND flash.

Two pending patches, "jffs2: Fix a lock leak in jffs2_flash_writev()"
and "jffs2: fix wbuf_sem leak on summary failure in
jffs2_flash_writev()", handle a failure of jffs2_sum_add_kvec() inside
the write-buffer path in wbuf.c. This patch is independent of them: it
fixes the retry path in write.c, reached after any failed write whose
retry reservation then fails.

 fs/jffs2/write.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
index cda9a361368e..0ac3220e7b05 100644
--- a/fs/jffs2/write.c
+++ b/fs/jffs2/write.c
@@ -147,6 +147,13 @@ struct jffs2_full_dnode *jffs2_write_dnode(struct jffs2_sb_info *c, struct jffs2
 
 				ret = jffs2_reserve_space(c, sizeof(*ri) + datalen, &dummy,
 							  alloc_mode, JFFS2_SUMMARY_INODE_SIZE);
+				/*
+				 * jffs2_reserve_space() drops alloc_sem on failure,
+				 * but our callers call jffs2_complete_reservation()
+				 * on error. Take it back, before f->sem.
+				 */
+				if (ret)
+					mutex_lock(&c->alloc_sem);
 				mutex_lock(&f->sem);
 			}
 
@@ -299,6 +306,9 @@ struct jffs2_full_dirent *jffs2_write_dirent(struct jffs2_sb_info *c, struct jff
 
 				ret = jffs2_reserve_space(c, sizeof(*rd) + namelen, &dummy,
 							  alloc_mode, JFFS2_SUMMARY_DIRENT_SIZE(namelen));
+				/* See the comment in jffs2_write_dnode(). */
+				if (ret)
+					mutex_lock(&c->alloc_sem);
 				mutex_lock(&f->sem);
 			}
 
-- 
2.43.0