[RFC 1/1] fs/jffs2: Avoid a possible deadlock in jffs2_wbuf_recover

From: Wang Zhaolong
Date: Tue Mar 18 2025 - 22:47:29 EST

jffs2_wbuf_recover can deadlock when called from __jffs2_flush_wbuf
error path, because both functions try to acquire c->wbuf_sem.

jffs2_write_end
jffs2_write_inode_range
jffs2_write_dnode
jffs2_flash_writev
down_write(&c->wbuf_sem) # first hold lock
__jffs2_flush_wbuf
mtd_write() # return error
jffs2_wbuf_recover
jffs2_reserve_space_gc
jffs2_do_reserve_space
jffs2_flush_wbuf_pad
down_write(&c->wbuf_sem) # AA deadlock

Fix this by adding a wbuf_sem_held parameter to jffs2_reserve_space_gc
and jffs2_do_reserve_space, which can be used to indicate that the caller
already holds c->wbuf_sem. If this is the case, jffs2_do_reserve_space
will directly call __jffs2_flush_wbuf instead of going through
jffs2_flush_wbuf_pad, which would try to acquire the lock again.

Signed-off-by: Wang Zhaolong <wangzhaolong1@xxxxxxxxxx>
---
fs/jffs2/gc.c | 12 ++++++------
fs/jffs2/nodelist.h | 12 +++++++++++-
fs/jffs2/nodemgmt.c | 16 ++++++++++------
fs/jffs2/os-linux.h | 1 +
fs/jffs2/wbuf.c | 13 ++-----------
fs/jffs2/write.c | 4 ++--
fs/jffs2/xattr.c | 4 ++--
7 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 1b833bbffcf5..d2c0b1a14294 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -608,11 +608,11 @@ static int jffs2_garbage_collect_pristine(struct jffs2_sb_info *c,
don't want to force wastage of the end of a block if splitting would
work. */
if (ic && alloclen > sizeof(struct jffs2_raw_inode) + JFFS2_MIN_DATA_LEN)
alloclen = sizeof(struct jffs2_raw_inode) + JFFS2_MIN_DATA_LEN;

- ret = jffs2_reserve_space_gc(c, alloclen, &alloclen, rawlen);
+ ret = jffs2_reserve_space_gc(c, alloclen, &alloclen, rawlen, 0);
/* 'rawlen' is not the exact summary size; it is only an upper estimation */

if (ret)
return ret;

@@ -718,11 +718,11 @@ static int jffs2_garbage_collect_pristine(struct jffs2_sb_info *c,
jffs2_dbg(1, "Retrying failed write of REF_PRISTINE node.\n");

jffs2_dbg_acct_sanity_check(c,jeb);
jffs2_dbg_acct_paranoia_check(c, jeb);

- ret = jffs2_reserve_space_gc(c, rawlen, &dummy, rawlen);
+ ret = jffs2_reserve_space_gc(c, rawlen, &dummy, rawlen, 0);
/* this is not the exact summary size of it,
it is only an upper estimation */

if (!ret) {
jffs2_dbg(1, "Allocated space at 0x%08x to retry failed write.\n",
@@ -792,11 +792,11 @@ static int jffs2_garbage_collect_metadata(struct jffs2_sb_info *c, struct jffs2_
__func__, mdatalen);

}

ret = jffs2_reserve_space_gc(c, sizeof(ri) + mdatalen, &alloclen,
- JFFS2_SUMMARY_INODE_SIZE);
+ JFFS2_SUMMARY_INODE_SIZE, 0);
if (ret) {
pr_warn("jffs2_reserve_space_gc of %zd bytes for garbage_collect_metadata failed: %d\n",
sizeof(ri) + mdatalen, ret);
goto out;
}
@@ -873,11 +873,11 @@ static int jffs2_garbage_collect_dirent(struct jffs2_sb_info *c, struct jffs2_er
rd.type = fd->type;
rd.node_crc = cpu_to_je32(crc32(0, &rd, sizeof(rd)-8));
rd.name_crc = cpu_to_je32(crc32(0, fd->name, rd.nsize));

ret = jffs2_reserve_space_gc(c, sizeof(rd)+rd.nsize, &alloclen,
- JFFS2_SUMMARY_DIRENT_SIZE(rd.nsize));
+ JFFS2_SUMMARY_DIRENT_SIZE(rd.nsize), 0);
if (ret) {
pr_warn("jffs2_reserve_space_gc of %zd bytes for garbage_collect_dirent failed: %d\n",
sizeof(rd)+rd.nsize, ret);
return ret;
}
@@ -1097,11 +1097,11 @@ static int jffs2_garbage_collect_hole(struct jffs2_sb_info *c, struct jffs2_eras
ri.mtime = cpu_to_je32(JFFS2_F_I_MTIME(f));
ri.data_crc = cpu_to_je32(0);
ri.node_crc = cpu_to_je32(crc32(0, &ri, sizeof(ri)-8));

ret = jffs2_reserve_space_gc(c, sizeof(ri), &alloclen,
- JFFS2_SUMMARY_INODE_SIZE);
+ JFFS2_SUMMARY_INODE_SIZE, 0);
if (ret) {
pr_warn("jffs2_reserve_space_gc of %zd bytes for garbage_collect_hole failed: %d\n",
sizeof(ri), ret);
return ret;
}
@@ -1343,11 +1343,11 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
uint32_t datalen;
uint32_t cdatalen;
uint16_t comprtype = JFFS2_COMPR_NONE;

ret = jffs2_reserve_space_gc(c, sizeof(ri) + JFFS2_MIN_DATA_LEN,
- &alloclen, JFFS2_SUMMARY_INODE_SIZE);
+ &alloclen, JFFS2_SUMMARY_INODE_SIZE, 0);

if (ret) {
pr_warn("jffs2_reserve_space_gc of %zd bytes for garbage_collect_dnode failed: %d\n",
sizeof(ri) + JFFS2_MIN_DATA_LEN, ret);
break;
diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
index 2e98fa277dab..f3c494901cf4 100644
--- a/fs/jffs2/nodelist.h
+++ b/fs/jffs2/nodelist.h
@@ -383,11 +383,11 @@ extern uint32_t __jffs2_ref_totlen(struct jffs2_sb_info *c,
/* nodemgmt.c */
int jffs2_thread_should_wake(struct jffs2_sb_info *c);
int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
uint32_t *len, int prio, uint32_t sumsize);
int jffs2_reserve_space_gc(struct jffs2_sb_info *c, uint32_t minsize,
- uint32_t *len, uint32_t sumsize);
+ uint32_t *len, uint32_t sumsize, int wbuf_sem_held);
struct jffs2_raw_node_ref *jffs2_add_physical_node_ref(struct jffs2_sb_info *c,
uint32_t ofs, uint32_t len,
struct jffs2_inode_cache *ic);
void jffs2_complete_reservation(struct jffs2_sb_info *c);
void jffs2_mark_node_obsolete(struct jffs2_sb_info *c, struct jffs2_raw_node_ref *raw);
@@ -471,10 +471,20 @@ int jffs2_do_mount_fs(struct jffs2_sb_info *c);
int jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count);
void jffs2_free_jeb_node_refs(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);

#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
/* wbuf.c */
+/* Meaning of pad argument:
+ 0: Do not pad. Probably pointless - we only ever use this when we can't pad anyway.
+ 1: Pad, do not adjust nextblock free_size
+ 2: Pad, adjust nextblock free_size
+*/
+#define NOPAD 0
+#define PAD_NOACCOUNT 1
+#define PAD_ACCOUNTING 2
+
+int __jffs2_flush_wbuf(struct jffs2_sb_info *c, int pad);
int jffs2_flush_wbuf_gc(struct jffs2_sb_info *c, uint32_t ino);
int jffs2_flush_wbuf_pad(struct jffs2_sb_info *c);
int jffs2_check_nand_cleanmarker(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);
int jffs2_write_nand_cleanmarker(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);
#endif
diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c
index 3fb9f9807b66..0a1bf6947277 100644
--- a/fs/jffs2/nodemgmt.c
+++ b/fs/jffs2/nodemgmt.c
@@ -49,11 +49,11 @@ static int jffs2_rp_can_write(struct jffs2_sb_info *c)
jffs2_dbg(1, "forbid writing\n");
return 0;
}

static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
- uint32_t *len, uint32_t sumsize);
+ uint32_t *len, uint32_t sumsizew, int buf_sem_held);

/**
* jffs2_reserve_space - request physical space to write nodes to flash
* @c: superblock info
* @minsize: Minimum acceptable size of allocation
@@ -196,11 +196,11 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,

mutex_lock(&c->alloc_sem);
spin_lock(&c->erase_completion_lock);
}

- ret = jffs2_do_reserve_space(c, minsize, len, sumsize);
+ ret = jffs2_do_reserve_space(c, minsize, len, sumsize, 0);
if (ret) {
jffs2_dbg(1, "%s(): ret is %d\n", __func__, ret);
}
}

@@ -212,20 +212,20 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
mutex_unlock(&c->alloc_sem);
return ret;
}

int jffs2_reserve_space_gc(struct jffs2_sb_info *c, uint32_t minsize,
- uint32_t *len, uint32_t sumsize)
+ uint32_t *len, uint32_t sumsize, int wbuf_sem_held)
{
int ret;
minsize = PAD(minsize);

jffs2_dbg(1, "%s(): Requested 0x%x bytes\n", __func__, minsize);

while (true) {
spin_lock(&c->erase_completion_lock);
- ret = jffs2_do_reserve_space(c, minsize, len, sumsize);
+ ret = jffs2_do_reserve_space(c, minsize, len, sumsize, wbuf_sem_held);
if (ret) {
jffs2_dbg(1, "%s(): looping, ret is %d\n",
__func__, ret);
}
spin_unlock(&c->erase_completion_lock);
@@ -354,11 +354,11 @@ static int jffs2_find_nextblock(struct jffs2_sb_info *c)
return 0;
}

/* Called with alloc sem _and_ erase_completion_lock */
static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
- uint32_t *len, uint32_t sumsize)
+ uint32_t *len, uint32_t sumsize, int wbuf_sem_held)
{
struct jffs2_eraseblock *jeb = c->nextblock;
uint32_t reserved_size; /* for summary information at the end of the jeb */
int ret;

@@ -416,11 +416,15 @@ static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,

if (jffs2_wbuf_dirty(c)) {
spin_unlock(&c->erase_completion_lock);
jffs2_dbg(1, "%s(): Flushing write buffer\n",
__func__);
- jffs2_flush_wbuf_pad(c);
+ if (wbuf_sem_held) {
+ __jffs2_flush_wbuf(c, PAD_NOACCOUNT);
+ } else {
+ jffs2_flush_wbuf_pad(c);
+ }
spin_lock(&c->erase_completion_lock);
jeb = c->nextblock;
goto restart;
}

diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h
index 86ab014a349c..c9dda7126850 100644
--- a/fs/jffs2/os-linux.h
+++ b/fs/jffs2/os-linux.h
@@ -76,10 +76,11 @@ static inline void jffs2_init_inode_info(struct jffs2_inode_info *f)
#define jffs2_cleanmarker_oob(c) (0)
#define jffs2_write_nand_cleanmarker(c,jeb) (-EIO)

#define jffs2_flash_write(c, ofs, len, retlen, buf) jffs2_flash_direct_write(c, ofs, len, retlen, buf)
#define jffs2_flash_read(c, ofs, len, retlen, buf) (mtd_read((c)->mtd, ofs, len, retlen, buf))
+#define __jffs2_flush_wbuf(c, pad) ({ do{} while(0); (void)(c), 0; })
#define jffs2_flush_wbuf_pad(c) ({ do{} while(0); (void)(c), 0; })
#define jffs2_flush_wbuf_gc(c, i) ({ do{} while(0); (void)(c), (void) i, 0; })
#define jffs2_write_nand_badblock(c,jeb,bad_offset) (1)
#define jffs2_nand_flash_setup(c) (0)
#define jffs2_nand_flash_cleanup(c) do {} while(0)
diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
index 4061e0ba7010..799559760dc1 100644
--- a/fs/jffs2/wbuf.c
+++ b/fs/jffs2/wbuf.c
@@ -384,11 +384,11 @@ static void jffs2_wbuf_recover(struct jffs2_sb_info *c)
}
/* OK... we're to rewrite (end-start) bytes of data from first_raw onwards.
Either 'buf' contains the data, or we find it in the wbuf */

/* ... and get an allocation of space from a shiny new block instead */
- ret = jffs2_reserve_space_gc(c, end-start, &len, JFFS2_SUMMARY_NOSUM_SIZE);
+ ret = jffs2_reserve_space_gc(c, end-start, &len, JFFS2_SUMMARY_NOSUM_SIZE, 1);
if (ret) {
pr_warn("Failed to allocate space for wbuf recovery. Data loss ensues.\n");
kfree(buf);
return;
}
@@ -566,20 +566,11 @@ static void jffs2_wbuf_recover(struct jffs2_sb_info *c)
jffs2_dbg(1, "wbuf recovery completed OK. wbuf_ofs 0x%08x, len 0x%x\n",
c->wbuf_ofs, c->wbuf_len);

}

-/* Meaning of pad argument:
- 0: Do not pad. Probably pointless - we only ever use this when we can't pad anyway.
- 1: Pad, do not adjust nextblock free_size
- 2: Pad, adjust nextblock free_size
-*/
-#define NOPAD 0
-#define PAD_NOACCOUNT 1
-#define PAD_ACCOUNTING 2
-
-static int __jffs2_flush_wbuf(struct jffs2_sb_info *c, int pad)
+int __jffs2_flush_wbuf(struct jffs2_sb_info *c, int pad)
{
struct jffs2_eraseblock *wbuf_jeb;
int ret;
size_t retlen;

diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
index cda9a361368e..1764cf0466fc 100644
--- a/fs/jffs2/write.c
+++ b/fs/jffs2/write.c
@@ -137,11 +137,11 @@ struct jffs2_full_dnode *jffs2_write_dnode(struct jffs2_sb_info *c, struct jffs2
jffs2_dbg_acct_sanity_check(c,jeb);
jffs2_dbg_acct_paranoia_check(c, jeb);

if (alloc_mode == ALLOC_GC) {
ret = jffs2_reserve_space_gc(c, sizeof(*ri) + datalen, &dummy,
- JFFS2_SUMMARY_INODE_SIZE);
+ JFFS2_SUMMARY_INODE_SIZE, 0);
} else {
/* Locking pain */
mutex_unlock(&f->sem);
jffs2_complete_reservation(c);

@@ -289,11 +289,11 @@ struct jffs2_full_dirent *jffs2_write_dirent(struct jffs2_sb_info *c, struct jff
jffs2_dbg_acct_sanity_check(c,jeb);
jffs2_dbg_acct_paranoia_check(c, jeb);

if (alloc_mode == ALLOC_GC) {
ret = jffs2_reserve_space_gc(c, sizeof(*rd) + namelen, &dummy,
- JFFS2_SUMMARY_DIRENT_SIZE(namelen));
+ JFFS2_SUMMARY_DIRENT_SIZE(namelen), 0);
} else {
/* Locking pain */
mutex_unlock(&f->sem);
jffs2_complete_reservation(c);

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index defb4162c3d5..b7b1eb8f012c 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -1241,11 +1241,11 @@ int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xatt
goto out;
}
old_ofs = ref_offset(xd->node);
totlen = PAD(sizeof(struct jffs2_raw_xattr)
+ xd->name_len + 1 + xd->value_len);
- rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XATTR_SIZE);
+ rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XATTR_SIZE, 0);
if (rc) {
JFFS2_WARNING("jffs2_reserve_space_gc()=%d, request=%u\n", rc, totlen);
goto out;
}
rc = save_xattr_datum(c, xd);
@@ -1274,11 +1274,11 @@ int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_
goto out;

old_ofs = ref_offset(ref->node);
totlen = ref_totlen(c, c->gcblock, ref->node);

- rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XREF_SIZE);
+ rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XREF_SIZE, 0);
if (rc) {
JFFS2_WARNING("%s: jffs2_reserve_space_gc() = %d, request = %u\n",
__func__, rc, totlen);
goto out;
}
--
2.34.3