From 052c28073ff26f771d44ef33952a41d18dadd255 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 29 Jun 2014 17:00:45 +0300 Subject: UBIFS: fix a race condition Hu (hujianyang@huawei.com) discovered a race condition which may lead to a situation when UBIFS is unable to mount the file-system after an unclean reboot. The problem is theoretical, though. In UBIFS, we have the log, which basically a set of LEBs in a certain area. The log has the tail and the head. Every time user writes data to the file-system, the UBIFS journal grows, and the log grows as well, because we append new reference nodes to the head of the log. So the head moves forward all the time, while the log tail stays at the same position. At any time, the UBIFS master node points to the tail of the log. When we mount the file-system, we scan the log, and we always start from its tail, because this is where the master node points to. The only occasion when the tail of the log changes is the commit operation. The commit operation has 2 phases - "commit start" and "commit end". The former is relatively short, and does not involve much I/O. During this phase we mostly just build various in-memory lists of the things which have to be written to the flash media during "commit end" phase. During the commit start phase, what we do is we "clean" the log. Indeed, the commit operation will index all the data in the journal, so the entire journal "disappears", and therefore the data in the log become unneeded. So we just move the head of the log to the next LEB, and write the CS node there. This LEB will be the tail of the new log when the commit operation finishes. When the "commit start" phase finishes, users may write more data to the file-system, in parallel with the ongoing "commit end" operation. At this point the log tail was not changed yet, it is the same as it had been before we started the commit. The log head keeps moving forward, though. The commit operation now needs to write the new master node, and the new master node should point to the new log tail. After this the LEBs between the old log tail and the new log tail can be unmapped and re-used again. And here is the possible problem. We do 2 operations: (a) We first update the log tail position in memory (see 'ubifs_log_end_commit()'). (b) And then we write the master node (see the big lock of code in 'do_commit()'). But nothing prevents the log head from moving forward between (a) and (b), and the log head may "wrap" now to the old log tail. And when the "wrap" happens, the contends of the log tail gets erased. Now a power cut happens and we are in trouble. We end up with the old master node pointing to the old tail, which was erased. And replay fails because it expects the master node to point to the correct log tail at all times. This patch merges the abovementioned (a) and (b) operations by moving the master node change code to the 'ubifs_log_end_commit()' function, so that it runs with the log mutex locked, which will prevent the log from being changed benween operations (a) and (b). Cc: stable@vger.kernel.org # 07e19df UBIFS: remove mst_mutex Cc: stable@vger.kernel.org Reported-by: hujianyang Tested-by: hujianyang Signed-off-by: Artem Bityutskiy --- fs/ubifs/commit.c | 8 +++----- fs/ubifs/log.c | 11 ++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c index aa13ad053b14..26b69b2d4a45 100644 --- a/fs/ubifs/commit.c +++ b/fs/ubifs/commit.c @@ -164,10 +164,6 @@ static int do_commit(struct ubifs_info *c) if (err) goto out; err = ubifs_orphan_end_commit(c); - if (err) - goto out; - old_ltail_lnum = c->ltail_lnum; - err = ubifs_log_end_commit(c, new_ltail_lnum); if (err) goto out; err = dbg_check_old_index(c, &zroot); @@ -202,7 +198,9 @@ static int do_commit(struct ubifs_info *c) c->mst_node->flags |= cpu_to_le32(UBIFS_MST_NO_ORPHS); else c->mst_node->flags &= ~cpu_to_le32(UBIFS_MST_NO_ORPHS); - err = ubifs_write_master(c); + + old_ltail_lnum = c->ltail_lnum; + err = ubifs_log_end_commit(c, new_ltail_lnum); if (err) goto out; diff --git a/fs/ubifs/log.c b/fs/ubifs/log.c index a47ddfc9be6b..9bd4aafd5c6f 100644 --- a/fs/ubifs/log.c +++ b/fs/ubifs/log.c @@ -447,9 +447,9 @@ out: * @ltail_lnum: new log tail LEB number * * This function is called on when the commit operation was finished. It - * moves log tail to new position and unmaps LEBs which contain obsolete data. - * Returns zero in case of success and a negative error code in case of - * failure. + * moves log tail to new position and updates the master node so that it stores + * the new log tail LEB number. Returns zero in case of success and a negative + * error code in case of failure. */ int ubifs_log_end_commit(struct ubifs_info *c, int ltail_lnum) { @@ -477,7 +477,12 @@ int ubifs_log_end_commit(struct ubifs_info *c, int ltail_lnum) spin_unlock(&c->buds_lock); err = dbg_check_bud_bytes(c); + if (err) + goto out; + err = ubifs_write_master(c); + +out: mutex_unlock(&c->log_mutex); return err; } -- cgit v1.2.3-70-g09d2 From ba29e721eb2df6df8f33c1f248388bb037a47914 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Wed, 16 Jul 2014 15:22:29 +0300 Subject: UBIFS: fix free log space calculation Hu (hujianyang ) discovered an issue in the 'empty_log_bytes()' function, which calculates how many bytes are left in the log: " If 'c->lhead_lnum + 1 == c->ltail_lnum' and 'c->lhead_offs == c->leb_size', 'h' would equalent to 't' and 'empty_log_bytes()' would return 'c->log_bytes' instead of 0. " At this point it is not clear what would be the consequences of this, and whether this may lead to any problems, but this patch addresses the issue just in case. Cc: stable@vger.kernel.org Tested-by: hujianyang Reported-by: hujianyang Signed-off-by: Artem Bityutskiy --- fs/ubifs/log.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ubifs/log.c b/fs/ubifs/log.c index 9bd4aafd5c6f..c14628fbeee2 100644 --- a/fs/ubifs/log.c +++ b/fs/ubifs/log.c @@ -106,10 +106,14 @@ static inline long long empty_log_bytes(const struct ubifs_info *c) h = (long long)c->lhead_lnum * c->leb_size + c->lhead_offs; t = (long long)c->ltail_lnum * c->leb_size; - if (h >= t) + if (h > t) return c->log_bytes - h + t; - else + else if (h != t) return t - h; + else if (c->lhead_lnum != c->ltail_lnum) + return 0; + else + return c->log_bytes; } /** -- cgit v1.2.3-70-g09d2 From d577bc104f2c01928d586358663de6d0a950130f Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Fri, 19 Sep 2014 11:48:46 +0200 Subject: UBIFS: Remove bogus assert This assertion was only correct before UBIFS had xattr support. Now with xattr support also a directory node can carry data and can act as host node. Suggested-by: Artem Bityutskiy Signed-off-by: Richard Weinberger Signed-off-by: Artem Bityutskiy --- fs/ubifs/journal.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 0e045e75abd8..fb166e204441 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -546,15 +546,14 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, int aligned_dlen, aligned_ilen, sync = IS_DIRSYNC(dir); int last_reference = !!(deletion && inode->i_nlink == 0); struct ubifs_inode *ui = ubifs_inode(inode); - struct ubifs_inode *dir_ui = ubifs_inode(dir); + struct ubifs_inode *host_ui = ubifs_inode(dir); struct ubifs_dent_node *dent; struct ubifs_ino_node *ino; union ubifs_key dent_key, ino_key; dbg_jnl("ino %lu, dent '%.*s', data len %d in dir ino %lu", inode->i_ino, nm->len, nm->name, ui->data_len, dir->i_ino); - ubifs_assert(dir_ui->data_len == 0); - ubifs_assert(mutex_is_locked(&dir_ui->ui_mutex)); + ubifs_assert(mutex_is_locked(&host_ui->ui_mutex)); dlen = UBIFS_DENT_NODE_SZ + nm->len + 1; ilen = UBIFS_INO_NODE_SZ; @@ -658,7 +657,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, ui->synced_i_size = ui->ui_size; spin_unlock(&ui->ui_lock); mark_inode_clean(c, ui); - mark_inode_clean(c, dir_ui); + mark_inode_clean(c, host_ui); return 0; out_finish: -- cgit v1.2.3-70-g09d2 From 4b1a43eab1ab0b1d05bc0c2aa823262da2445a7f Mon Sep 17 00:00:00 2001 From: hujianyang Date: Sat, 20 Sep 2014 14:55:11 +0800 Subject: UBIFS: Align the dump messages of SB_NODE I found the dump messages of UBIFS_SB_NODE is not aligned. This patch remove the extra space from the line which is retracted. Signed-off-by: hujianyang Signed-off-by: Artem Bityutskiy --- fs/ubifs/debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index 177b0152fef4..d0bcf9290ea4 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -334,9 +334,9 @@ void ubifs_dump_node(const struct ubifs_info *c, const void *node) pr_err("\tkey_fmt %d (%s)\n", (int)sup->key_fmt, get_key_fmt(sup->key_fmt)); pr_err("\tflags %#x\n", sup_flags); - pr_err("\t big_lpt %u\n", + pr_err("\tbig_lpt %u\n", !!(sup_flags & UBIFS_FLG_BIGLPT)); - pr_err("\t space_fixup %u\n", + pr_err("\tspace_fixup %u\n", !!(sup_flags & UBIFS_FLG_SPACE_FIXUP)); pr_err("\tmin_io_size %u\n", le32_to_cpu(sup->min_io_size)); pr_err("\tleb_size %u\n", le32_to_cpu(sup->leb_size)); -- cgit v1.2.3-70-g09d2 From 443b39cdd5c37661bf681858b327418c3a5b9d76 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Tue, 16 Sep 2014 15:30:36 +0200 Subject: UBIFS: Fix trivial typo in power_cut_emulated() s/withing/within/ Signed-off-by: Richard Weinberger Signed-off-by: Artem Bityutskiy --- fs/ubifs/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index d0bcf9290ea4..7ed13e1e216a 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -2462,7 +2462,7 @@ static int power_cut_emulated(struct ubifs_info *c, int lnum, int write) if (chance(1, 2)) { d->pc_delay = 1; - /* Fail withing 1 minute */ + /* Fail within 1 minute */ delay = prandom_u32() % 60000; d->pc_timeout = jiffies; d->pc_timeout += msecs_to_jiffies(delay); -- cgit v1.2.3-70-g09d2