From c64cda8a99135c584a1c61074e225aacdf31060f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 15 Mar 2019 13:54:32 -0700 Subject: [PATCH 01/13] ubifs: remove unnecessary calls to set up directory key In ubifs_unlink() and ubifs_rmdir(), remove the call to fscrypt_get_encryption_info() that precedes fscrypt_setup_filename(). This call was unnecessary, because fscrypt_setup_filename() already tries to set up the directory's encryption key. Signed-off-by: Eric Biggers Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 5767b373a8ff..c3311ea68fe5 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -792,12 +792,6 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry) dentry, inode->i_ino, inode->i_nlink, dir->i_ino); - if (ubifs_crypt_is_encrypted(dir)) { - err = fscrypt_get_encryption_info(dir); - if (err && err != -ENOKEY) - return err; - } - err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm); if (err) return err; @@ -902,12 +896,6 @@ static int ubifs_rmdir(struct inode *dir, struct dentry *dentry) if (err) return err; - if (ubifs_crypt_is_encrypted(dir)) { - err = fscrypt_get_encryption_info(dir); - if (err && err != -ENOKEY) - return err; - } - err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm); if (err) return err; From cf3949670f709d86d975b0cfa4e65be2b40d1e78 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 15 Mar 2019 13:55:03 -0700 Subject: [PATCH 02/13] ubifs: remove unnecessary #ifdef around fscrypt_ioctl_get_policy() When !CONFIG_FS_ENCRYPTION, fscrypt_ioctl_get_policy() is already stubbed out to return -EOPNOTSUPP, so the extra #ifdef is not needed. Signed-off-by: Eric Biggers Signed-off-by: Richard Weinberger --- fs/ubifs/ioctl.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c index 82e4e6a30b04..cd1d4212fc45 100644 --- a/fs/ubifs/ioctl.c +++ b/fs/ubifs/ioctl.c @@ -205,13 +205,8 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return -EOPNOTSUPP; #endif } - case FS_IOC_GET_ENCRYPTION_POLICY: { -#ifdef CONFIG_FS_ENCRYPTION + case FS_IOC_GET_ENCRYPTION_POLICY: return fscrypt_ioctl_get_policy(file, (void __user *)arg); -#else - return -EOPNOTSUPP; -#endif - } default: return -ENOTTY; From fb9a5a3edb4c34ac0d7e6996278604e53a43245f Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Wed, 20 Mar 2019 22:09:55 +0800 Subject: [PATCH 03/13] ubifs: remove unused function __ubifs_shash_final There is no callers in tree, and can be removed. Signed-off-by: YueHaibing Acked-by: Mukesh Ojha Signed-off-by: Richard Weinberger --- fs/ubifs/auth.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index 5bf5fd08879e..2a40cccee5bd 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -146,24 +146,6 @@ struct shash_desc *__ubifs_hash_get_desc(const struct ubifs_info *c) return ubifs_get_desc(c, c->hash_tfm); } -/** - * __ubifs_shash_final - finalize shash - * @c: UBIFS file-system description object - * @desc: the descriptor - * @out: the output hash - * - * Simple wrapper around crypto_shash_final(), safe to be called with - * disabled authentication. - */ -int __ubifs_shash_final(const struct ubifs_info *c, struct shash_desc *desc, - u8 *out) -{ - if (ubifs_authenticated(c)) - return crypto_shash_final(desc, out); - - return 0; -} - /** * ubifs_bad_hash - Report hash mismatches * @c: UBIFS file-system description object From f4844b35d68ab49c41d542c501fffb46ad81009a Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sun, 24 Mar 2019 20:44:51 +0100 Subject: [PATCH 04/13] ubifs: work around high stack usage with clang Building this file with clang can result in large stack usage as seen from this warning: fs/ubifs/auth.c:78:5: error: stack frame size of 1152 bytes in function 'ubifs_prepare_auth_node' The problem is that inlining ubifs_hash_calc_hmac() leads to two SHASH_DESC_ON_STACK() blocks in the same function, and clang for some reason does not reuse the stack space as it should. Putting the first declaration into a separate basic block avoids this problem and reduces the stack allocation to 640 bytes. Signed-off-by: Arnd Bergmann Reviewed-by: Nick Desaulniers Signed-off-by: Richard Weinberger --- fs/ubifs/auth.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index 2a40cccee5bd..3d049194afa4 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -78,7 +78,6 @@ static int ubifs_hash_calc_hmac(const struct ubifs_info *c, const u8 *hash, int ubifs_prepare_auth_node(struct ubifs_info *c, void *node, struct shash_desc *inhash) { - SHASH_DESC_ON_STACK(hash_desc, c->hash_tfm); struct ubifs_auth_node *auth = node; u8 *hash; int err; @@ -87,13 +86,17 @@ int ubifs_prepare_auth_node(struct ubifs_info *c, void *node, if (!hash) return -ENOMEM; - hash_desc->tfm = c->hash_tfm; - hash_desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP; - ubifs_shash_copy_state(c, inhash, hash_desc); + { + SHASH_DESC_ON_STACK(hash_desc, c->hash_tfm); - err = crypto_shash_final(hash_desc, hash); - if (err) - goto out; + hash_desc->tfm = c->hash_tfm; + hash_desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP; + ubifs_shash_copy_state(c, inhash, hash_desc); + + err = crypto_shash_final(hash_desc, hash); + if (err) + goto out; + } err = ubifs_hash_calc_hmac(c, hash, auth->hmac); if (err) From e9cd7dfd7ef939394436ef6d03ff156e587ba39e Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Mon, 1 Apr 2019 15:00:44 +0200 Subject: [PATCH 05/13] ubifs: Do not skip hash checking in data nodes UBIFS bails out early from try_read_node() when it doesn't have to check the CRC. Still the node hash has to be checked, otherwise wrong data could be sneaked into the FS. Fix this by not bailing out early and always checking the node hash. Fixes: 16a26b20d2af ("ubifs: authentication: Add hashes to index nodes") Signed-off-by: Sascha Hauer Signed-off-by: Richard Weinberger --- fs/ubifs/tnc.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 25572ffea163..ebf8c26f5b22 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -479,14 +479,13 @@ static int try_read_node(const struct ubifs_info *c, void *buf, int type, if (node_len != len) return 0; - if (type == UBIFS_DATA_NODE && c->no_chk_data_crc && !c->mounting && - !c->remounting_rw) - return 1; - - crc = crc32(UBIFS_CRC32_INIT, buf + 8, node_len - 8); - node_crc = le32_to_cpu(ch->crc); - if (crc != node_crc) - return 0; + if (type != UBIFS_DATA_NODE || !c->no_chk_data_crc || c->mounting || + c->remounting_rw) { + crc = crc32(UBIFS_CRC32_INIT, buf + 8, node_len - 8); + node_crc = le32_to_cpu(ch->crc); + if (crc != node_crc) + return 0; + } err = ubifs_node_check_hash(c, buf, zbr->hash); if (err) { From 257bb92420d63c4d38da4cf0ae9a0b61ac985eca Mon Sep 17 00:00:00 2001 From: Andrey Abramov Date: Tue, 2 Apr 2019 23:52:10 +0300 Subject: [PATCH 06/13] ubifs: find.c: replace swap function with built-in one Replace swap_dirty_idx function with built-in one, because swap_dirty_idx does only a simple byte to byte swap. Since Spectre mitigations have made indirect function calls more expensive, and the default simple byte copies swap is implemented without them, an "optimized" custom swap function is now a waste of time as well as code. Signed-off-by: Andrey Abramov Reviewed by: George Spelvin Signed-off-by: Richard Weinberger --- fs/ubifs/find.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fs/ubifs/find.c b/fs/ubifs/find.c index f9646835b026..5deaae7fcead 100644 --- a/fs/ubifs/find.c +++ b/fs/ubifs/find.c @@ -747,12 +747,6 @@ static int cmp_dirty_idx(const struct ubifs_lprops **a, return lpa->dirty + lpa->free - lpb->dirty - lpb->free; } -static void swap_dirty_idx(struct ubifs_lprops **a, struct ubifs_lprops **b, - int size) -{ - swap(*a, *b); -} - /** * ubifs_save_dirty_idx_lnums - save an array of the most dirty index LEB nos. * @c: the UBIFS file-system description object @@ -772,8 +766,7 @@ int ubifs_save_dirty_idx_lnums(struct ubifs_info *c) sizeof(void *) * c->dirty_idx.cnt); /* Sort it so that the dirtiest is now at the end */ sort(c->dirty_idx.arr, c->dirty_idx.cnt, sizeof(void *), - (int (*)(const void *, const void *))cmp_dirty_idx, - (void (*)(void *, void *, int))swap_dirty_idx); + (int (*)(const void *, const void *))cmp_dirty_idx, NULL); dbg_find("found %d dirty index LEBs", c->dirty_idx.cnt); if (c->dirty_idx.cnt) dbg_find("dirtiest index LEB is %d with dirty %d and free %d", From 7959cf3a7506d4a2100d5d6f37f605c2f54af488 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Fri, 5 Apr 2019 00:34:36 +0200 Subject: [PATCH 07/13] ubifs: journal: Handle xattrs like files If an inode hosts xattrs, create deletion entries for each inode. That way we can make sure that upon journal replay UBIFS can find find all xattr inodes. Otherwise it can happen that GC consumed already a LEB which contained parts of the TNC that pointed to the xattrs and we no longer find all xattr inodes, which will confuse the LPT and cause space allocation issues. Reported-by: Stefan Agner Fixes: 1e51764a3c2ac ("UBIFS: add new flash file system") Signed-off-by: Richard Weinberger --- fs/ubifs/journal.c | 60 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 729dc76c83df..4f74d443ca44 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -852,10 +852,11 @@ out_free: int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) { int err, lnum, offs; - struct ubifs_ino_node *ino; + struct ubifs_ino_node *ino, *ino_start; struct ubifs_inode *ui = ubifs_inode(inode); - int sync = 0, write_len, ilen = UBIFS_INO_NODE_SZ; + int sync = 0, write_len = 0, ilen = UBIFS_INO_NODE_SZ; int last_reference = !inode->i_nlink; + int kill_xattrs = ui->xattr_cnt && last_reference; u8 hash[UBIFS_HASH_ARR_SZ]; dbg_jnl("ino %lu, nlink %u", inode->i_ino, inode->i_nlink); @@ -867,14 +868,16 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) if (!last_reference) { ilen += ui->data_len; sync = IS_SYNC(inode); + } else if (kill_xattrs) { + write_len += UBIFS_INO_NODE_SZ * ui->xattr_cnt; } if (ubifs_authenticated(c)) - write_len = ALIGN(ilen, 8) + ubifs_auth_node_sz(c); + write_len += ALIGN(ilen, 8) + ubifs_auth_node_sz(c); else - write_len = ilen; + write_len += ilen; - ino = kmalloc(write_len, GFP_NOFS); + ino_start = ino = kmalloc(write_len, GFP_NOFS); if (!ino) return -ENOMEM; @@ -883,12 +886,47 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) if (err) goto out_free; + if (kill_xattrs) { + union ubifs_key key; + struct fscrypt_name nm = {0}; + struct inode *xino; + struct ubifs_dent_node *xent, *pxent = NULL; + + lowest_xent_key(c, &key, inode->i_ino); + while (1) { + xent = ubifs_tnc_next_ent(c, &key, &nm); + if (IS_ERR(xent)) { + err = PTR_ERR(xent); + if (err == -ENOENT) + break; + + goto out_release; + } + + fname_name(&nm) = xent->name; + fname_len(&nm) = le16_to_cpu(xent->nlen); + + xino = ubifs_iget(c->vfs_sb, xent->inum); + ubifs_assert(c, ubifs_inode(xino)->xattr); + + clear_nlink(xino); + pack_inode(c, ino, xino, 0); + ino = (void *)ino + UBIFS_INO_NODE_SZ; + iput(xino); + + kfree(pxent); + pxent = xent; + key_read(c, &xent->key, &key); + } + kfree(pxent); + } + pack_inode(c, ino, inode, 1); err = ubifs_node_calc_hash(c, ino, hash); if (err) goto out_release; - err = write_head(c, BASEHD, ino, write_len, &lnum, &offs, sync); + err = write_head(c, BASEHD, ino_start, write_len, &lnum, &offs, sync); if (err) goto out_release; if (!sync) @@ -903,7 +941,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) if (err) goto out_ro; ubifs_delete_orphan(c, inode->i_ino); - err = ubifs_add_dirt(c, lnum, ilen); + err = ubifs_add_dirt(c, lnum, write_len); } else { union ubifs_key key; @@ -917,7 +955,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) spin_lock(&ui->ui_lock); ui->synced_i_size = ui->ui_size; spin_unlock(&ui->ui_lock); - kfree(ino); + kfree(ino_start); return 0; out_release: @@ -926,7 +964,7 @@ out_ro: ubifs_ro_mode(c, err); finish_reservation(c); out_free: - kfree(ino); + kfree(ino_start); return err; } @@ -966,8 +1004,8 @@ int ubifs_jnl_delete_inode(struct ubifs_info *c, const struct inode *inode) ubifs_assert(c, inode->i_nlink == 0); - if (ui->del_cmtno != c->cmt_no) - /* A commit happened for sure */ + if (ui->xattr_cnt || ui->del_cmtno != c->cmt_no) + /* A commit happened for sure or inode hosts xattrs */ return ubifs_jnl_write_inode(c, inode); down_read(&c->commit_sem); From 988bec41318f3fa897e2f8af271bd456936d6caf Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Fri, 5 Apr 2019 00:34:37 +0200 Subject: [PATCH 08/13] ubifs: orphan: Handle xattrs like files Like for the journal case, make sure that we track all xattr inodes. Otherwise UBIFS might not be able to locate stale xattr inodes upon recovery. Reported-by: Stefan Agner Fixes: 1e51764a3c2ac ("UBIFS: add new flash file system") Signed-off-by: Richard Weinberger --- fs/ubifs/orphan.c | 218 +++++++++++++++++++++++++++++++--------------- fs/ubifs/ubifs.h | 3 + 2 files changed, 149 insertions(+), 72 deletions(-) diff --git a/fs/ubifs/orphan.c b/fs/ubifs/orphan.c index 8f70494efb0c..2f1618f300fb 100644 --- a/fs/ubifs/orphan.c +++ b/fs/ubifs/orphan.c @@ -54,30 +54,24 @@ static int dbg_check_orphans(struct ubifs_info *c); -/** - * ubifs_add_orphan - add an orphan. - * @c: UBIFS file-system description object - * @inum: orphan inode number - * - * Add an orphan. This function is called when an inodes link count drops to - * zero. - */ -int ubifs_add_orphan(struct ubifs_info *c, ino_t inum) +static struct ubifs_orphan *orphan_add(struct ubifs_info *c, ino_t inum, + struct ubifs_orphan *parent_orphan) { struct ubifs_orphan *orphan, *o; struct rb_node **p, *parent = NULL; orphan = kzalloc(sizeof(struct ubifs_orphan), GFP_NOFS); if (!orphan) - return -ENOMEM; + return ERR_PTR(-ENOMEM); orphan->inum = inum; orphan->new = 1; + INIT_LIST_HEAD(&orphan->child_list); spin_lock(&c->orphan_lock); if (c->tot_orphans >= c->max_orphans) { spin_unlock(&c->orphan_lock); kfree(orphan); - return -ENFILE; + return ERR_PTR(-ENFILE); } p = &c->orph_tree.rb_node; while (*p) { @@ -91,7 +85,7 @@ int ubifs_add_orphan(struct ubifs_info *c, ino_t inum) ubifs_err(c, "orphaned twice"); spin_unlock(&c->orphan_lock); kfree(orphan); - return 0; + return ERR_PTR(-EINVAL); } } c->tot_orphans += 1; @@ -100,8 +94,135 @@ int ubifs_add_orphan(struct ubifs_info *c, ino_t inum) rb_insert_color(&orphan->rb, &c->orph_tree); list_add_tail(&orphan->list, &c->orph_list); list_add_tail(&orphan->new_list, &c->orph_new); + + if (parent_orphan) { + list_add_tail(&orphan->child_list, + &parent_orphan->child_list); + } + spin_unlock(&c->orphan_lock); dbg_gen("ino %lu", (unsigned long)inum); + return orphan; +} + +static struct ubifs_orphan *lookup_orphan(struct ubifs_info *c, ino_t inum) +{ + struct ubifs_orphan *o; + struct rb_node *p; + + p = c->orph_tree.rb_node; + while (p) { + o = rb_entry(p, struct ubifs_orphan, rb); + if (inum < o->inum) + p = p->rb_left; + else if (inum > o->inum) + p = p->rb_right; + else { + return o; + } + } + return NULL; +} + +static void __orphan_drop(struct ubifs_info *c, struct ubifs_orphan *o) +{ + rb_erase(&o->rb, &c->orph_tree); + list_del(&o->list); + c->tot_orphans -= 1; + + if (o->new) { + list_del(&o->new_list); + c->new_orphans -= 1; + } + + kfree(o); +} + +static void orphan_delete(struct ubifs_info *c, ino_t inum) +{ + struct ubifs_orphan *orph, *child_orph, *tmp_o; + + spin_lock(&c->orphan_lock); + + orph = lookup_orphan(c, inum); + if (!orph) { + spin_unlock(&c->orphan_lock); + ubifs_err(c, "missing orphan ino %lu", (unsigned long)inum); + dump_stack(); + + return; + } + + if (orph->del) { + spin_unlock(&c->orphan_lock); + dbg_gen("deleted twice ino %lu", + (unsigned long)inum); + return; + } + + if (orph->cmt) { + orph->del = 1; + orph->dnext = c->orph_dnext; + c->orph_dnext = orph; + spin_unlock(&c->orphan_lock); + dbg_gen("delete later ino %lu", + (unsigned long)inum); + return; + } + + list_for_each_entry_safe(child_orph, tmp_o, &orph->child_list, child_list) { + list_del(&child_orph->child_list); + __orphan_drop(c, child_orph); + } + + __orphan_drop(c, orph); + + spin_unlock(&c->orphan_lock); +} + +/** + * ubifs_add_orphan - add an orphan. + * @c: UBIFS file-system description object + * @inum: orphan inode number + * + * Add an orphan. This function is called when an inodes link count drops to + * zero. + */ +int ubifs_add_orphan(struct ubifs_info *c, ino_t inum) +{ + int err = 0; + ino_t xattr_inum; + union ubifs_key key; + struct ubifs_dent_node *xent; + struct fscrypt_name nm = {0}; + struct ubifs_orphan *xattr_orphan; + struct ubifs_orphan *orphan; + + orphan = orphan_add(c, inum, NULL); + if (IS_ERR(orphan)) + return PTR_ERR(orphan); + + lowest_xent_key(c, &key, inum); + while (1) { + xent = ubifs_tnc_next_ent(c, &key, &nm); + if (IS_ERR(xent)) { + err = PTR_ERR(xent); + if (err == -ENOENT) + break; + return err; + } + + fname_name(&nm) = xent->name; + fname_len(&nm) = le16_to_cpu(xent->nlen); + xattr_inum = le64_to_cpu(xent->inum); + + xattr_orphan = orphan_add(c, xattr_inum, orphan); + if (IS_ERR(xattr_orphan)) + return PTR_ERR(xattr_orphan); + + key_read(c, &xent->key, &key); + } + return 0; } @@ -114,49 +235,7 @@ int ubifs_add_orphan(struct ubifs_info *c, ino_t inum) */ void ubifs_delete_orphan(struct ubifs_info *c, ino_t inum) { - struct ubifs_orphan *o; - struct rb_node *p; - - spin_lock(&c->orphan_lock); - p = c->orph_tree.rb_node; - while (p) { - o = rb_entry(p, struct ubifs_orphan, rb); - if (inum < o->inum) - p = p->rb_left; - else if (inum > o->inum) - p = p->rb_right; - else { - if (o->del) { - spin_unlock(&c->orphan_lock); - dbg_gen("deleted twice ino %lu", - (unsigned long)inum); - return; - } - if (o->cmt) { - o->del = 1; - o->dnext = c->orph_dnext; - c->orph_dnext = o; - spin_unlock(&c->orphan_lock); - dbg_gen("delete later ino %lu", - (unsigned long)inum); - return; - } - rb_erase(p, &c->orph_tree); - list_del(&o->list); - c->tot_orphans -= 1; - if (o->new) { - list_del(&o->new_list); - c->new_orphans -= 1; - } - spin_unlock(&c->orphan_lock); - kfree(o); - dbg_gen("inum %lu", (unsigned long)inum); - return; - } - } - spin_unlock(&c->orphan_lock); - ubifs_err(c, "missing orphan ino %lu", (unsigned long)inum); - dump_stack(); + orphan_delete(c, inum); } /** @@ -611,10 +690,16 @@ static int do_kill_orphans(struct ubifs_info *c, struct ubifs_scan_leb *sleb, n = (le32_to_cpu(orph->ch.len) - UBIFS_ORPH_NODE_SZ) >> 3; for (i = 0; i < n; i++) { + union ubifs_key key1, key2; + inum = le64_to_cpu(orph->inos[i]); dbg_rcvry("deleting orphaned inode %lu", (unsigned long)inum); - err = ubifs_tnc_remove_ino(c, inum); + + lowest_ino_key(c, &key1, inum); + highest_ino_key(c, &key2, inum); + + err = ubifs_tnc_remove_range(c, &key1, &key2); if (err) return err; err = insert_dead_orphan(c, inum); @@ -744,26 +829,15 @@ struct check_info { struct rb_root root; }; -static int dbg_find_orphan(struct ubifs_info *c, ino_t inum) +static bool dbg_find_orphan(struct ubifs_info *c, ino_t inum) { - struct ubifs_orphan *o; - struct rb_node *p; + bool found = false; spin_lock(&c->orphan_lock); - p = c->orph_tree.rb_node; - while (p) { - o = rb_entry(p, struct ubifs_orphan, rb); - if (inum < o->inum) - p = p->rb_left; - else if (inum > o->inum) - p = p->rb_right; - else { - spin_unlock(&c->orphan_lock); - return 1; - } - } + found = !!lookup_orphan(c, inum); spin_unlock(&c->orphan_lock); - return 0; + + return found; } static int dbg_ins_check_orphan(struct rb_root *root, ino_t inum) diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 1ae12900e01d..d28102829f6c 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -924,6 +924,8 @@ struct ubifs_budget_req { * @rb: rb-tree node of rb-tree of orphans sorted by inode number * @list: list head of list of orphans in order added * @new_list: list head of list of orphans added since the last commit + * @child_list: list of xattr childs if this orphan hosts xattrs, list head + * if this orphan is a xattr, not used otherwise. * @cnext: next orphan to commit * @dnext: next orphan to delete * @inum: inode number @@ -935,6 +937,7 @@ struct ubifs_orphan { struct rb_node rb; struct list_head list; struct list_head new_list; + struct list_head child_list; struct ubifs_orphan *cnext; struct ubifs_orphan *dnext; ino_t inum; From 9ca2d732644484488db31123ecd3bf122b551566 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Fri, 5 Apr 2019 00:34:38 +0200 Subject: [PATCH 09/13] ubifs: Limit number of xattrs per inode Since we have to write one deletion inode per xattr into the journal, limit the max number of xattrs. In theory UBIFS supported up to 65535 xattrs per inode. But this never worked correctly, expect no powercuts happened. Now we support only as many xattrs as we can store in 50% of a LEB. Even for tiny flashes this allows dozens of xattrs per inode, which is for an embedded filesystem still fine. In case someone has existing inodes with much more xattrs, it is still possible to delete them. UBIFS will fall back to an non-atomic deletion mode. Reported-by: Stefan Agner Fixes: 1e51764a3c2ac ("UBIFS: add new flash file system") Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 15 +++++++++- fs/ubifs/journal.c | 12 ++++++++ fs/ubifs/misc.h | 8 ++++++ fs/ubifs/super.c | 2 ++ fs/ubifs/ubifs.h | 1 + fs/ubifs/xattr.c | 71 +++++++++++++++++++++++++++++++++++++++++----- 6 files changed, 101 insertions(+), 8 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index c3311ea68fe5..d2f5a50f5d83 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -796,6 +796,10 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry) if (err) return err; + err = ubifs_purge_xattrs(inode); + if (err) + return err; + sz_change = CALC_DENT_SIZE(fname_len(&nm)); ubifs_assert(c, inode_is_locked(dir)); @@ -900,6 +904,10 @@ static int ubifs_rmdir(struct inode *dir, struct dentry *dentry) if (err) return err; + err = ubifs_purge_xattrs(inode); + if (err) + return err; + sz_change = CALC_DENT_SIZE(fname_len(&nm)); err = ubifs_budget_space(c, &req); @@ -1282,9 +1290,14 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry, old_dentry, old_inode->i_ino, old_dir->i_ino, new_dentry, new_dir->i_ino, flags); - if (unlink) + if (unlink) { ubifs_assert(c, inode_is_locked(new_inode)); + err = ubifs_purge_xattrs(new_inode); + if (err) + return err; + } + if (unlink && is_dir) { err = ubifs_check_dir_empty(new_inode); if (err) diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 4f74d443ca44..74a7306978d0 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -892,6 +892,11 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) struct inode *xino; struct ubifs_dent_node *xent, *pxent = NULL; + if (ui->xattr_cnt >= ubifs_xattr_max_cnt(c)) { + ubifs_err(c, "Cannot delete inode, it has too much xattrs!"); + goto out_release; + } + lowest_xent_key(c, &key, inode->i_ino); while (1) { xent = ubifs_tnc_next_ent(c, &key, &nm); @@ -907,6 +912,13 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) fname_len(&nm) = le16_to_cpu(xent->nlen); xino = ubifs_iget(c->vfs_sb, xent->inum); + if (IS_ERR(xino)) { + err = PTR_ERR(xino); + ubifs_err(c, "dead directory entry '%s', error %d", + xent->name, err); + ubifs_ro_mode(c, err); + goto out_release; + } ubifs_assert(c, ubifs_inode(xino)->xattr); clear_nlink(xino); diff --git a/fs/ubifs/misc.h b/fs/ubifs/misc.h index 6f87237fdbf4..78a6e97f846e 100644 --- a/fs/ubifs/misc.h +++ b/fs/ubifs/misc.h @@ -288,6 +288,14 @@ static inline int ubifs_next_log_lnum(const struct ubifs_info *c, int lnum) return lnum; } +static inline int ubifs_xattr_max_cnt(struct ubifs_info *c) +{ + int max_xattrs = (c->leb_size / 2) / UBIFS_INO_NODE_SZ; + + ubifs_assert(c, max_xattrs < c->max_orphans); + return max_xattrs; +} + const char *ubifs_assert_action_name(struct ubifs_info *c); #endif /* __UBIFS_MISC_H__ */ diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 12628184772c..300458a4f518 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1548,6 +1548,8 @@ static int mount_ubifs(struct ubifs_info *c) c->bud_bytes, c->bud_bytes >> 10, c->bud_bytes >> 20); dbg_gen("max. seq. number: %llu", c->max_sqnum); dbg_gen("commit number: %llu", c->cmt_no); + dbg_gen("max. xattrs per inode: %d", ubifs_xattr_max_cnt(c)); + dbg_gen("max orphans: %d", c->max_orphans); return 0; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index d28102829f6c..cf4b10f24b6d 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -2017,6 +2017,7 @@ int ubifs_xattr_set(struct inode *host, const char *name, const void *value, size_t size, int flags, bool check_lock); ssize_t ubifs_xattr_get(struct inode *host, const char *name, void *buf, size_t size); +int ubifs_purge_xattrs(struct inode *host); #ifdef CONFIG_UBIFS_FS_XATTR void ubifs_evict_xattr_inode(struct ubifs_info *c, ino_t xattr_inum); diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index f5ad1ede7990..acab3181ab35 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -60,12 +60,6 @@ #include #include -/* - * Limit the number of extended attributes per inode so that the total size - * (@xattr_size) is guaranteeded to fit in an 'unsigned int'. - */ -#define MAX_XATTRS_PER_INODE 65535 - /* * Extended attribute type constants. * @@ -106,7 +100,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host, .new_ino_d = ALIGN(size, 8), .dirtied_ino = 1, .dirtied_ino_d = ALIGN(host_ui->data_len, 8) }; - if (host_ui->xattr_cnt >= MAX_XATTRS_PER_INODE) { + if (host_ui->xattr_cnt >= ubifs_xattr_max_cnt(c)) { ubifs_err(c, "inode %lu already has too many xattrs (%d), cannot create more", host->i_ino, host_ui->xattr_cnt); return -ENOSPC; @@ -507,6 +501,69 @@ out_cancel: return err; } +int ubifs_purge_xattrs(struct inode *host) +{ + union ubifs_key key; + struct ubifs_info *c = host->i_sb->s_fs_info; + struct ubifs_dent_node *xent, *pxent = NULL; + struct inode *xino; + struct fscrypt_name nm = {0}; + int err; + + if (ubifs_inode(host)->xattr_cnt < ubifs_xattr_max_cnt(c)) + return 0; + + ubifs_warn(c, "inode %lu has too many xattrs, doing a non-atomic deletion", + host->i_ino); + + lowest_xent_key(c, &key, host->i_ino); + while (1) { + xent = ubifs_tnc_next_ent(c, &key, &nm); + if (IS_ERR(xent)) { + err = PTR_ERR(xent); + break; + } + + fname_name(&nm) = xent->name; + fname_len(&nm) = le16_to_cpu(xent->nlen); + + xino = ubifs_iget(c->vfs_sb, xent->inum); + if (IS_ERR(xino)) { + err = PTR_ERR(xino); + ubifs_err(c, "dead directory entry '%s', error %d", + xent->name, err); + ubifs_ro_mode(c, err); + kfree(pxent); + return err; + } + + ubifs_assert(c, ubifs_inode(xino)->xattr); + + clear_nlink(xino); + err = remove_xattr(c, host, xino, &nm); + if (err) { + kfree(pxent); + iput(xino); + ubifs_err(c, "cannot remove xattr, error %d", err); + return err; + } + + iput(xino); + + kfree(pxent); + pxent = xent; + key_read(c, &xent->key, &key); + } + + kfree(pxent); + if (err != -ENOENT) { + ubifs_err(c, "cannot find next direntry, error %d", err); + return err; + } + + return 0; +} + /** * ubifs_evict_xattr_inode - Evict an xattr inode. * @c: UBIFS file-system description object From eea2c05d927b031034e222110a4fc34914d97ca4 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 26 Mar 2019 08:52:31 +0100 Subject: [PATCH 10/13] ubifs: Remove #ifdef around CONFIG_FS_ENCRYPTION ifdefs reduce readablity and compile coverage. This removes the ifdefs around CONFIG_FS_ENCRYPTION by using IS_ENABLED and relying on static inline wrappers. A new static inline wrapper for setting sb->s_cop is introduced to allow filesystems to unconditionally compile in their s_cop operations. Signed-off-by: Sascha Hauer Signed-off-by: Richard Weinberger --- fs/ubifs/ioctl.c | 4 ---- fs/ubifs/sb.c | 7 ++++--- fs/ubifs/super.c | 4 +--- include/linux/fscrypt.h | 11 +++++++++++ 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c index cd1d4212fc45..6b05b3ec500e 100644 --- a/fs/ubifs/ioctl.c +++ b/fs/ubifs/ioctl.c @@ -193,7 +193,6 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return err; } case FS_IOC_SET_ENCRYPTION_POLICY: { -#ifdef CONFIG_FS_ENCRYPTION struct ubifs_info *c = inode->i_sb->s_fs_info; err = ubifs_enable_encryption(c); @@ -201,9 +200,6 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return err; return fscrypt_ioctl_set_policy(file, (const void __user *)arg); -#else - return -EOPNOTSUPP; -#endif } case FS_IOC_GET_ENCRYPTION_POLICY: return fscrypt_ioctl_get_policy(file, (void __user *)arg); diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 67fac1e8adfb..2afc8b1d4c3b 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -748,14 +748,12 @@ int ubifs_read_superblock(struct ubifs_info *c) goto out; } -#ifndef CONFIG_FS_ENCRYPTION - if (c->encrypted) { + if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION) && c->encrypted) { ubifs_err(c, "file system contains encrypted files but UBIFS" " was built without crypto support."); err = -EINVAL; goto out; } -#endif /* Automatically increase file system size to the maximum size */ c->old_leb_cnt = c->leb_cnt; @@ -943,6 +941,9 @@ int ubifs_enable_encryption(struct ubifs_info *c) int err; struct ubifs_sb_node *sup = c->sup_node; + if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION)) + return -EOPNOTSUPP; + if (c->encrypted) return 0; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 300458a4f518..d0c1228635f0 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -2146,9 +2146,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent) #ifdef CONFIG_UBIFS_FS_XATTR sb->s_xattr = ubifs_xattr_handlers; #endif -#ifdef CONFIG_FS_ENCRYPTION - sb->s_cop = &ubifs_crypt_operations; -#endif + fscrypt_set_ops(sb, &ubifs_crypt_operations); mutex_lock(&c->umount_mutex); err = mount_ubifs(c); diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index e5194fc3983e..9a5792dac16a 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -230,6 +230,11 @@ extern int __fscrypt_encrypt_symlink(struct inode *inode, const char *target, extern const char *fscrypt_get_symlink(struct inode *inode, const void *caddr, unsigned int max_size, struct delayed_call *done); +static inline void fscrypt_set_ops(struct super_block *sb, + const struct fscrypt_operations *s_cop) +{ + sb->s_cop = s_cop; +} #else /* !CONFIG_FS_ENCRYPTION */ static inline bool fscrypt_has_encryption_key(const struct inode *inode) @@ -446,6 +451,12 @@ static inline const char *fscrypt_get_symlink(struct inode *inode, { return ERR_PTR(-EOPNOTSUPP); } + +static inline void fscrypt_set_ops(struct super_block *sb, + const struct fscrypt_operations *s_cop) +{ +} + #endif /* !CONFIG_FS_ENCRYPTION */ /** From e3d73dead4988f3eb8483b28087f3ec0ff9d1398 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 26 Mar 2019 08:52:32 +0100 Subject: [PATCH 11/13] ubifs: Remove ifdefs around CONFIG_UBIFS_ATIME_SUPPORT ifdefs reduce readability and compile coverage. This removes the ifdefs around CONFIG_UBIFS_ATIME_SUPPORT by replacing them with IS_ENABLED() where applicable. The fs layer would fall back to generic_update_time() when .update_time doesn't exist. We do this fallback explicitly now. Signed-off-by: Sascha Hauer Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 2 -- fs/ubifs/file.c | 16 +++++++--------- fs/ubifs/super.c | 16 ++++++++-------- fs/ubifs/ubifs.h | 2 -- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index d2f5a50f5d83..80c062c03857 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1653,9 +1653,7 @@ const struct inode_operations ubifs_dir_inode_operations = { #ifdef CONFIG_UBIFS_FS_XATTR .listxattr = ubifs_listxattr, #endif -#ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, -#endif .tmpfile = ubifs_tmpfile, }; diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 5d2ffb1a45fc..512e7d9c60cd 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1375,7 +1375,6 @@ static inline int mctime_update_needed(const struct inode *inode, return 0; } -#ifdef CONFIG_UBIFS_ATIME_SUPPORT /** * ubifs_update_time - update time of inode. * @inode: inode to update @@ -1392,6 +1391,9 @@ int ubifs_update_time(struct inode *inode, struct timespec64 *time, int iflags = I_DIRTY_TIME; int err, release; + if (!IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT)) + return generic_update_time(inode, time, flags); + err = ubifs_budget_space(c, &req); if (err) return err; @@ -1414,7 +1416,6 @@ int ubifs_update_time(struct inode *inode, struct timespec64 *time, ubifs_release_budget(c, &req); return 0; } -#endif /** * update_mctime - update mtime and ctime of an inode. @@ -1623,9 +1624,10 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) if (err) return err; vma->vm_ops = &ubifs_file_vm_ops; -#ifdef CONFIG_UBIFS_ATIME_SUPPORT - file_accessed(file); -#endif + + if (IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT)) + file_accessed(file); + return 0; } @@ -1663,9 +1665,7 @@ const struct inode_operations ubifs_file_inode_operations = { #ifdef CONFIG_UBIFS_FS_XATTR .listxattr = ubifs_listxattr, #endif -#ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, -#endif }; const struct inode_operations ubifs_symlink_inode_operations = { @@ -1675,9 +1675,7 @@ const struct inode_operations ubifs_symlink_inode_operations = { #ifdef CONFIG_UBIFS_FS_XATTR .listxattr = ubifs_listxattr, #endif -#ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, -#endif }; const struct file_operations ubifs_file_operations = { diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index d0c1228635f0..ed026ad6f455 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -129,9 +129,10 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum) goto out_ino; inode->i_flags |= S_NOCMTIME; -#ifndef CONFIG_UBIFS_ATIME_SUPPORT - inode->i_flags |= S_NOATIME; -#endif + + if (!IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT)) + inode->i_flags |= S_NOATIME; + set_nlink(inode, le32_to_cpu(ino->nlink)); i_uid_write(inode, le32_to_cpu(ino->uid)); i_gid_write(inode, le32_to_cpu(ino->gid)); @@ -2248,11 +2249,10 @@ static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags, goto out_deact; /* We do not support atime */ sb->s_flags |= SB_ACTIVE; -#ifndef CONFIG_UBIFS_ATIME_SUPPORT - sb->s_flags |= SB_NOATIME; -#else - ubifs_msg(c, "full atime support is enabled."); -#endif + if (IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT)) + ubifs_msg(c, "full atime support is enabled."); + else + sb->s_flags |= SB_NOATIME; } /* 'fill_super()' opens ubi again so we must close it here */ diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index cf4b10f24b6d..379b9f791ff6 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1999,9 +1999,7 @@ int ubifs_calc_dark(const struct ubifs_info *c, int spc); /* file.c */ int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync); int ubifs_setattr(struct dentry *dentry, struct iattr *attr); -#ifdef CONFIG_UBIFS_ATIME_SUPPORT int ubifs_update_time(struct inode *inode, struct timespec64 *time, int flags); -#endif /* dir.c */ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir, From a65d10f3ce657aa4542b5de78933053f6d1a9e97 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 26 Mar 2019 09:05:32 +0100 Subject: [PATCH 12/13] ubifs: Drop unnecessary setting of zbr->znode in dbg_walk_index ubifs_load_znode is used to load the znode behind a zbranch. ubifs_load_znode links the new child znode to the zbranch, so doing it again is unnecessary. Signed-off-by: Sascha Hauer Signed-off-by: Richard Weinberger --- fs/ubifs/debug.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index c49ff50fdceb..3a2613038e88 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -1603,7 +1603,6 @@ int dbg_walk_index(struct ubifs_info *c, dbg_leaf_callback leaf_cb, err = PTR_ERR(child); goto out_unlock; } - zbr->znode = child; } znode = child; From 04d37e5a8b1fad2d625727af3d738c6fd9491720 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Sat, 2 Mar 2019 13:59:07 -0600 Subject: [PATCH 13/13] ubi: wl: Fix uninitialized variable There is a potential execution path in which variable *err* is compared against UBI_IO_BITFLIPS without being properly initialized previously. Fix this by initializing variable *err* to 0. Addresses-Coverity-ID: 1477298 "(Uninitialized scalar variable") Fixes: 663586c0a892 ("ubi: Expose the bitrot interface") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/wl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 2709dc02fc24..1f56c655832b 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -1475,7 +1475,7 @@ static bool scrub_possible(struct ubi_device *ubi, struct ubi_wl_entry *e) */ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force) { - int err; + int err = 0; struct ubi_wl_entry *e; if (pnum < 0 || pnum >= ubi->peb_count) {