Skip to content

Commit 3af2d3a

Browse files
Zhihao Chengrichardweinberger
Zhihao Cheng
authored and
richardweinberger
committed
ubifs: Fix unattached inode when powercut happens in creating
For selinux or encryption scenarios, UBIFS could become inconsistent while creating new files in powercut case. Encryption/selinux related xattrs will be created before creating file dentry, which makes creation process is not atomic, details are shown as: Encryption case: ubifs_create ubifs_new_inode fscrypt_set_context ubifs_xattr_set create_xattr ubifs_jnl_update // Disk: xentry xinode inode(LAST_OF_NODE_GROUP) >> power cut << ubifs_jnl_update // Disk: dentry inode parent_inode(LAST_OF_NODE_GROUP) Selinux case: ubifs_create ubifs_new_inode ubifs_init_security security_inode_init_security ubifs_xattr_set create_xattr ubifs_jnl_update // Disk: xentry xinode inode(LAST_OF_NODE_GROUP) >> power cut << ubifs_jnl_update // Disk: dentry inode parent_inode(LAST_OF_NODE_GROUP) Above process will make chk_fs failed in next mounting: UBIFS error (ubi0:0 pid 7995): dbg_check_filesystem [ubifs]: inode 66 nlink is 1, but calculated nlink is 0 Fix it by allocating orphan inode for each non-xattr file creation, then removing orphan list in journal writing process, which ensures that both xattr and dentry be effective in atomic when powercut happens. Fixes: d7f0b70 ("UBIFS: Add security.* XATTR support for the UBIFS") Fixes: d475a50 ("ubifs: Add skeleton for fscrypto") Link: https://bugzilla.kernel.org/show_bug.cgi?id=218309 Suggested-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Signed-off-by: Richard Weinberger <richard@nod.at>
1 parent b25e6a5 commit 3af2d3a

File tree

3 files changed

+51
-26
lines changed

3 files changed

+51
-26
lines changed

‎fs/ubifs/dir.c

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,13 @@ static int inherit_flags(const struct inode *dir, umode_t mode)
7171
* @is_xattr: whether the inode is xattr inode
7272
*
7373
* This function finds an unused inode number, allocates new inode and
74-
* initializes it. Returns new inode in case of success and an error code in
75-
* case of failure.
74+
* initializes it. Non-xattr new inode may be written with xattrs(selinux/
75+
* encryption) before writing dentry, which could cause inconsistent problem
76+
* when powercut happens between two operations. To deal with it, non-xattr
77+
* new inode is initialized with zero-nlink and added into orphan list, caller
78+
* should make sure that inode is relinked later, and make sure that orphan
79+
* removing and journal writing into an committing atomic operation. Returns
80+
* new inode in case of success and an error code in case of failure.
7681
*/
7782
struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir,
7883
umode_t mode, bool is_xattr)
@@ -163,9 +168,25 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir,
163168
ui->creat_sqnum = ++c->max_sqnum;
164169
spin_unlock(&c->cnt_lock);
165170

171+
if (!is_xattr) {
172+
set_nlink(inode, 0);
173+
err = ubifs_add_orphan(c, inode->i_ino);
174+
if (err) {
175+
ubifs_err(c, "ubifs_add_orphan failed: %i", err);
176+
goto out_iput;
177+
}
178+
down_read(&c->commit_sem);
179+
ui->del_cmtno = c->cmt_no;
180+
up_read(&c->commit_sem);
181+
}
182+
166183
if (encrypted) {
167184
err = fscrypt_set_context(inode, NULL);
168185
if (err) {
186+
if (!is_xattr) {
187+
set_nlink(inode, 1);
188+
ubifs_delete_orphan(c, inode->i_ino);
189+
}
169190
ubifs_err(c, "fscrypt_set_context failed: %i", err);
170191
goto out_iput;
171192
}
@@ -320,12 +341,13 @@ static int ubifs_create(struct mnt_idmap *idmap, struct inode *dir,
320341
if (err)
321342
goto out_inode;
322343

344+
set_nlink(inode, 1);
323345
mutex_lock(&dir_ui->ui_mutex);
324346
dir->i_size += sz_change;
325347
dir_ui->ui_size = dir->i_size;
326348
inode_set_mtime_to_ts(dir,
327349
inode_set_ctime_to_ts(dir, inode_get_ctime(inode)));
328-
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 0);
350+
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 1);
329351
if (err)
330352
goto out_cancel;
331353
mutex_unlock(&dir_ui->ui_mutex);
@@ -340,8 +362,8 @@ static int ubifs_create(struct mnt_idmap *idmap, struct inode *dir,
340362
dir->i_size -= sz_change;
341363
dir_ui->ui_size = dir->i_size;
342364
mutex_unlock(&dir_ui->ui_mutex);
365+
set_nlink(inode, 0);
343366
out_inode:
344-
make_bad_inode(inode);
345367
iput(inode);
346368
out_fname:
347369
fscrypt_free_filename(&nm);
@@ -386,7 +408,6 @@ static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry)
386408
return inode;
387409

388410
out_inode:
389-
make_bad_inode(inode);
390411
iput(inode);
391412
out_free:
392413
ubifs_err(c, "cannot create whiteout file, error %d", err);
@@ -470,6 +491,7 @@ static int ubifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
470491
if (err)
471492
goto out_inode;
472493

494+
set_nlink(inode, 1);
473495
mutex_lock(&ui->ui_mutex);
474496
insert_inode_hash(inode);
475497
d_tmpfile(file, inode);
@@ -479,7 +501,7 @@ static int ubifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
479501
mutex_unlock(&ui->ui_mutex);
480502

481503
lock_2_inodes(dir, inode);
482-
err = ubifs_jnl_update(c, dir, &nm, inode, 1, 0, 0);
504+
err = ubifs_jnl_update(c, dir, &nm, inode, 1, 0, 1);
483505
if (err)
484506
goto out_cancel;
485507
unlock_2_inodes(dir, inode);
@@ -492,7 +514,6 @@ static int ubifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
492514
out_cancel:
493515
unlock_2_inodes(dir, inode);
494516
out_inode:
495-
make_bad_inode(inode);
496517
if (!instantiated)
497518
iput(inode);
498519
out_budg:
@@ -1011,6 +1032,7 @@ static int ubifs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
10111032
if (err)
10121033
goto out_inode;
10131034

1035+
set_nlink(inode, 1);
10141036
mutex_lock(&dir_ui->ui_mutex);
10151037
insert_inode_hash(inode);
10161038
inc_nlink(inode);
@@ -1019,7 +1041,7 @@ static int ubifs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
10191041
dir_ui->ui_size = dir->i_size;
10201042
inode_set_mtime_to_ts(dir,
10211043
inode_set_ctime_to_ts(dir, inode_get_ctime(inode)));
1022-
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 0);
1044+
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 1);
10231045
if (err) {
10241046
ubifs_err(c, "cannot create directory, error %d", err);
10251047
goto out_cancel;
@@ -1036,8 +1058,8 @@ static int ubifs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
10361058
dir_ui->ui_size = dir->i_size;
10371059
drop_nlink(dir);
10381060
mutex_unlock(&dir_ui->ui_mutex);
1061+
set_nlink(inode, 0);
10391062
out_inode:
1040-
make_bad_inode(inode);
10411063
iput(inode);
10421064
out_fname:
10431065
fscrypt_free_filename(&nm);
@@ -1107,13 +1129,14 @@ static int ubifs_mknod(struct mnt_idmap *idmap, struct inode *dir,
11071129
ui = ubifs_inode(inode);
11081130
ui->data = dev;
11091131
ui->data_len = devlen;
1132+
set_nlink(inode, 1);
11101133

11111134
mutex_lock(&dir_ui->ui_mutex);
11121135
dir->i_size += sz_change;
11131136
dir_ui->ui_size = dir->i_size;
11141137
inode_set_mtime_to_ts(dir,
11151138
inode_set_ctime_to_ts(dir, inode_get_ctime(inode)));
1116-
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 0);
1139+
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 1);
11171140
if (err)
11181141
goto out_cancel;
11191142
mutex_unlock(&dir_ui->ui_mutex);
@@ -1128,8 +1151,8 @@ static int ubifs_mknod(struct mnt_idmap *idmap, struct inode *dir,
11281151
dir->i_size -= sz_change;
11291152
dir_ui->ui_size = dir->i_size;
11301153
mutex_unlock(&dir_ui->ui_mutex);
1154+
set_nlink(inode, 0);
11311155
out_inode:
1132-
make_bad_inode(inode);
11331156
iput(inode);
11341157
out_fname:
11351158
fscrypt_free_filename(&nm);
@@ -1208,13 +1231,14 @@ static int ubifs_symlink(struct mnt_idmap *idmap, struct inode *dir,
12081231
*/
12091232
ui->data_len = disk_link.len - 1;
12101233
inode->i_size = ubifs_inode(inode)->ui_size = disk_link.len - 1;
1234+
set_nlink(inode, 1);
12111235

12121236
mutex_lock(&dir_ui->ui_mutex);
12131237
dir->i_size += sz_change;
12141238
dir_ui->ui_size = dir->i_size;
12151239
inode_set_mtime_to_ts(dir,
12161240
inode_set_ctime_to_ts(dir, inode_get_ctime(inode)));
1217-
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 0);
1241+
err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0, 1);
12181242
if (err)
12191243
goto out_cancel;
12201244
mutex_unlock(&dir_ui->ui_mutex);
@@ -1228,10 +1252,10 @@ static int ubifs_symlink(struct mnt_idmap *idmap, struct inode *dir,
12281252
dir->i_size -= sz_change;
12291253
dir_ui->ui_size = dir->i_size;
12301254
mutex_unlock(&dir_ui->ui_mutex);
1255+
set_nlink(inode, 0);
12311256
out_inode:
12321257
/* Free inode->i_link before inode is marked as bad. */
12331258
fscrypt_free_inode(inode);
1234-
make_bad_inode(inode);
12351259
iput(inode);
12361260
out_fname:
12371261
fscrypt_free_filename(&nm);
@@ -1399,14 +1423,10 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
13991423
*/
14001424
err = ubifs_budget_space(c, &wht_req);
14011425
if (err) {
1402-
/*
1403-
* Whiteout inode can not be written on flash by
1404-
* ubifs_jnl_write_inode(), because it's neither
1405-
* dirty nor zero-nlink.
1406-
*/
14071426
iput(whiteout);
14081427
goto out_release;
14091428
}
1429+
set_nlink(whiteout, 1);
14101430

14111431
/* Add the old_dentry size to the old_dir size. */
14121432
old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
@@ -1485,7 +1505,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
14851505
}
14861506

14871507
err = ubifs_jnl_rename(c, old_dir, old_inode, &old_nm, new_dir,
1488-
new_inode, &new_nm, whiteout, sync);
1508+
new_inode, &new_nm, whiteout, sync, !!whiteout);
14891509
if (err)
14901510
goto out_cancel;
14911511

@@ -1538,6 +1558,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
15381558
unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
15391559
if (whiteout) {
15401560
ubifs_release_budget(c, &wht_req);
1561+
set_nlink(whiteout, 0);
15411562
iput(whiteout);
15421563
}
15431564
out_release:

‎fs/ubifs/journal.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ static void set_dent_cookie(struct ubifs_info *c, struct ubifs_dent_node *dent)
643643
* @inode: inode to update
644644
* @deletion: indicates a directory entry deletion i.e unlink or rmdir
645645
* @xent: non-zero if the directory entry is an extended attribute entry
646-
* @delete_orphan: indicates an orphan entry deletion for @inode
646+
* @in_orphan: indicates whether the @inode is in orphan list
647647
*
648648
* This function updates an inode by writing a directory entry (or extended
649649
* attribute entry), the inode itself, and the parent directory inode (or the
@@ -665,7 +665,7 @@ static void set_dent_cookie(struct ubifs_info *c, struct ubifs_dent_node *dent)
665665
*/
666666
int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
667667
const struct fscrypt_name *nm, const struct inode *inode,
668-
int deletion, int xent, int delete_orphan)
668+
int deletion, int xent, int in_orphan)
669669
{
670670
int err, dlen, ilen, len, lnum, ino_offs, dent_offs, orphan_added = 0;
671671
int aligned_dlen, aligned_ilen, sync = IS_DIRSYNC(dir);
@@ -751,7 +751,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
751751
if (err)
752752
goto out_release;
753753

754-
if (last_reference) {
754+
if (last_reference && !in_orphan) {
755755
err = ubifs_add_orphan(c, inode->i_ino);
756756
if (err) {
757757
release_head(c, BASEHD);
@@ -807,7 +807,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
807807
if (err)
808808
goto out_ro;
809809

810-
if (delete_orphan)
810+
if (in_orphan && inode->i_nlink)
811811
ubifs_delete_orphan(c, inode->i_ino);
812812

813813
finish_reservation(c);
@@ -1340,6 +1340,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
13401340
* @new_nm: new name of the new directory entry
13411341
* @whiteout: whiteout inode
13421342
* @sync: non-zero if the write-buffer has to be synchronized
1343+
* @delete_orphan: indicates an orphan entry deletion for @whiteout
13431344
*
13441345
* This function implements the re-name operation which may involve writing up
13451346
* to 4 inodes(new inode, whiteout inode, old and new parent directory inodes)
@@ -1352,7 +1353,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
13521353
const struct inode *new_dir,
13531354
const struct inode *new_inode,
13541355
const struct fscrypt_name *new_nm,
1355-
const struct inode *whiteout, int sync)
1356+
const struct inode *whiteout, int sync, int delete_orphan)
13561357
{
13571358
void *p;
13581359
union ubifs_key key;
@@ -1569,6 +1570,9 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
15691570
goto out_ro;
15701571
}
15711572

1573+
if (delete_orphan)
1574+
ubifs_delete_orphan(c, whiteout->i_ino);
1575+
15721576
finish_reservation(c);
15731577
if (new_inode) {
15741578
mark_inode_clean(c, new_ui);

‎fs/ubifs/ubifs.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,7 +1800,7 @@ int ubifs_consolidate_log(struct ubifs_info *c);
18001800
/* journal.c */
18011801
int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
18021802
const struct fscrypt_name *nm, const struct inode *inode,
1803-
int deletion, int xent, int delete_orphan);
1803+
int deletion, int xent, int in_orphan);
18041804
int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
18051805
const union ubifs_key *key, const void *buf, int len);
18061806
int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode);
@@ -1817,7 +1817,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
18171817
const struct inode *new_dir,
18181818
const struct inode *new_inode,
18191819
const struct fscrypt_name *new_nm,
1820-
const struct inode *whiteout, int sync);
1820+
const struct inode *whiteout, int sync, int delete_orphan);
18211821
int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
18221822
loff_t old_size, loff_t new_size);
18231823
int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,

0 commit comments

Comments
 (0)