Skip to content

Commit

Permalink
ocfs2: fix readonly issue in ocfs2_unlink()
Browse files Browse the repository at this point in the history
While deleting a file with ocfs2_unlink(), there is a bug in this
function.  This bug will result in filesystem read-only.

After calling ocfs2_orphan_add(), the file which will be deleted is
added into orphan dir.  If ocfs2_delete_entry() fails, the file still
exists in the parent dir.  And this scenario introduces a conflict of
metadata.

If a file is added into orphan dir, when we put inode of the file with
iput(), the inode i_flags is setted (~OCFS2_VALID_FL) in
ocfs2_remove_inode(), and then write back to disk.

But as previously mentioned, the file still exists in the parent dir.
On other nodes, the file can be still accessed.  When first read the
file with ocfs2_read_blocks() from disk, It will check and avalidate
inode using ocfs2_validate_inode_block().  So File system will be
readonly because the inode is invalid.  In other words, the inode
i_flags has been set (~OCFS2_VALID_FL).

[akpm@linux-foundation.org: cleanups]
[jeff.liu@oracle.com: s/inode_is_unlinkable/ocfs2_inode_is_unlinkable/]
Signed-off-by: Younger Liu <younger.liu@huawei.com>
Signed-off-by: Jensen <shencanquan@huawei.com>
Cc: Jie Liu <jeff.liu@oracle.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Sunil Mushran <sunil.mushran@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Younger Liu authored and Linus Torvalds committed Jul 3, 2013
1 parent 25e2892 commit b5a8bb7
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions fs/ocfs2/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ static int ocfs2_remote_dentry_delete(struct dentry *dentry)
return ret;
}

static inline int inode_is_unlinkable(struct inode *inode)
static inline int ocfs2_inode_is_unlinkable(struct inode *inode)
{
if (S_ISDIR(inode->i_mode)) {
if (inode->i_nlink == 2)
Expand All @@ -791,6 +791,7 @@ static int ocfs2_unlink(struct inode *dir,
{
int status;
int child_locked = 0;
bool is_unlinkable = false;
struct inode *inode = dentry->d_inode;
struct inode *orphan_dir = NULL;
struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
Expand Down Expand Up @@ -865,14 +866,15 @@ static int ocfs2_unlink(struct inode *dir,
goto leave;
}

if (inode_is_unlinkable(inode)) {
if (ocfs2_inode_is_unlinkable(inode)) {
status = ocfs2_prepare_orphan_dir(osb, &orphan_dir,
OCFS2_I(inode)->ip_blkno,
orphan_name, &orphan_insert);
if (status < 0) {
mlog_errno(status);
goto leave;
}
is_unlinkable = true;
}

handle = ocfs2_start_trans(osb, ocfs2_unlink_credits(osb->sb));
Expand All @@ -892,15 +894,6 @@ static int ocfs2_unlink(struct inode *dir,

fe = (struct ocfs2_dinode *) fe_bh->b_data;

if (inode_is_unlinkable(inode)) {
status = ocfs2_orphan_add(osb, handle, inode, fe_bh, orphan_name,
&orphan_insert, orphan_dir);
if (status < 0) {
mlog_errno(status);
goto leave;
}
}

/* delete the name from the parent dir */
status = ocfs2_delete_entry(handle, dir, &lookup);
if (status < 0) {
Expand All @@ -923,6 +916,14 @@ static int ocfs2_unlink(struct inode *dir,
mlog_errno(status);
if (S_ISDIR(inode->i_mode))
inc_nlink(dir);
goto leave;
}

if (is_unlinkable) {
status = ocfs2_orphan_add(osb, handle, inode, fe_bh,
orphan_name, &orphan_insert, orphan_dir);
if (status < 0)
mlog_errno(status);
}

leave:
Expand Down

0 comments on commit b5a8bb7

Please sign in to comment.