@@ -1478,11 +1478,10 @@ static int maybe_leb_gced(struct ubifs_info *c, int lnum, int gc_seq1)
int ubifs_tnc_locate(struct ubifs_info *c, const union ubifs_key *key,
void *node, int *lnum, int *offs)
{
- int found, n, err, safely = 0, gc_seq1;
+ int found, n, err;
struct ubifs_znode *znode;
- struct ubifs_zbranch zbr, *zt;
+ struct ubifs_zbranch *zt;
-again:
mutex_lock(&c->tnc_mutex);
found = ubifs_lookup_level0(c, key, &znode, &n);
if (!found) {
@@ -1505,31 +1504,7 @@ int ubifs_tnc_locate(struct ubifs_info *c, const union ubifs_key *key,
err = tnc_read_hashed_node(c, zt, node);
goto out;
}
- if (safely) {
- err = ubifs_tnc_read_node(c, zt, node);
- goto out;
- }
- /* Drop the TNC mutex prematurely and race with garbage collection */
- zbr = znode->zbranch[n];
- gc_seq1 = c->gc_seq;
- mutex_unlock(&c->tnc_mutex);
-
- if (ubifs_get_wbuf(c, zbr.lnum)) {
- /* We do not GC journal heads */
- err = ubifs_tnc_read_node(c, &zbr, node);
- return err;
- }
-
- err = fallible_read_node(c, key, &zbr, node);
- if (err <= 0 || maybe_leb_gced(c, zbr.lnum, gc_seq1)) {
- /*
- * The node may have been GC'ed out from under us so try again
- * while keeping the TNC mutex locked.
- */
- safely = 1;
- goto again;
- }
- return 0;
+ err = ubifs_tnc_read_node(c, zt, node);
out:
mutex_unlock(&c->tnc_mutex);
This problem is described in [1], and Tao has proposed a solution in [2]. There exists a race between node reading and gc, which is shown as: P1 P2 ubifs_tnc_locate zbr.lnum = lnum_a GC // node is moved from lnum_a to lnum_b journal head switching // lnum_a becomes a bud ubifs_get_wbuf(c, zbr.lnum) // true ubifs_tnc_read_node ubifs_read_node_wbuf // read data from lnum_a check node failed ! There are two ways of reading node(See ubifs_tnc_read_node()): 1. Reading from wbuf. The node is written in wbuf(in mem), and wbuf is not written back to flash. 2. Otherwise, reading from flash. In most cases, ubifs_tnc_read_node() is invoked with TNC mutex locked, but except the lockless path in ubifs_tnc_locate() which is imported by commit 601c0bc46753("UBIFS: allow for racing between GC and TNC"). Function ubifs_tnc_locate() is mainly used for path lookup and file reading, VFS has inode/dentry/page cache for multiple times reading, the lockless optimization only works for first reading. Based on the discussion in [2], this patch simply drops the TNC mutex lockless reading path in ubifs_tnc_locate(). Fetch a reproducer in [3]. [1] https://lore.kernel.org/all/fda84926-09d1-1fc7-4b78-99e0d04508bc@huawei.com/T/ [2] https://lore.kernel.org/linux-mtd/20200305092205.127758-1-houtao1@huawei.com/ [3] https://bugzilla.kernel.org/show_bug.cgi?id=218163 Fixes: 601c0bc46753 ("UBIFS: allow for racing between GC and TNC") Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Reported-by: 李傲傲 (Carson Li1/9542) <Carson.Li1@unisoc.com> Analyzed-by: Hou Tao <houtao1@huawei.com> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> --- fs/ubifs/tnc.c | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-)