2016-12-26 12:35:18

by Zhang Yi

[permalink] [raw]
Subject: [RFC PATCH] ext4: increase the protection of drop nlink and ext4 inode destroy

Because of the disk and hardware issue, the ext4 filesystem have
many errors, the inode->i_nlink of ext4 becomes zero abnormally
but the dentry is still positive, it will cause memory corruption
after the following process:

1) Due to the inode->i_nlink is 0, this inode will be added into
the orhpan list,
2) ext4_rename() cover this inode, and drop_nlink() will reverse
the inode->i_nlink to 0xFFFFFFFF,
3) iput() add this inode to LRU,
4) evict() will call destroy_inode() to destroy this inode but
skip removing it from the orphan list,
5) after this, the inode's memory address space will be used by
other module, when the ext4 filesystem change the orphan list, it will
trample other module's data and then may cause oops.

Although we cannot avoid hardware and disk errors, we can control the
softwore error in the ext4 module, do not affect other modules and
increase the difficulty of locating problems.

This patch avoid inode->i_nlink reverse and remove the inode form the
orphan list when destroy it if the list is not empty.
Signed-off-by: yi zhang <[email protected]>
---
fs/ext4/super.c | 1 +
fs/inode.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 52b0530..617327e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -975,6 +975,7 @@ static void ext4_destroy_inode(struct inode *inode)
EXT4_I(inode), sizeof(struct ext4_inode_info),
true);
dump_stack();
+ ext4_orphan_del(NULL, inode);
}
call_rcu(&inode->i_rcu, ext4_i_callback);
}
diff --git a/fs/inode.c b/fs/inode.c
index 88110fd..079d383 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -279,7 +279,10 @@ static void destroy_inode(struct inode *inode)
*/
void drop_nlink(struct inode *inode)
{
- WARN_ON(inode->i_nlink == 0);
+ if (WARN(inode->i_nlink == 0, "inode %lu nlink"
+ " is already 0", inode->i_ino))
+ return;
+
inode->__i_nlink--;
if (!inode->i_nlink)
atomic_long_inc(&inode->i_sb->s_remove_count);
--
2.5.0


2016-12-26 18:32:47

by Andreas Dilger

[permalink] [raw]
Subject: Re: [RFC PATCH] ext4: increase the protection of drop nlink and ext4 inode destroy

On Dec 26, 2016, at 5:34 AM, yi zhang <[email protected]> wrote:
>
> Because of the disk and hardware issue, the ext4 filesystem have
> many errors, the inode->i_nlink of ext4 becomes zero abnormally
> but the dentry is still positive, it will cause memory corruption
> after the following process:
>
> 1) Due to the inode->i_nlink is 0, this inode will be added into
> the orhpan list,
> 2) ext4_rename() cover this inode, and drop_nlink() will reverse
> the inode->i_nlink to 0xFFFFFFFF,
> 3) iput() add this inode to LRU,
> 4) evict() will call destroy_inode() to destroy this inode but
> skip removing it from the orphan list,
> 5) after this, the inode's memory address space will be used by
> other module, when the ext4 filesystem change the orphan list, it will
> trample other module's data and then may cause oops.
>
> Although we cannot avoid hardware and disk errors, we can control the
> softwore error in the ext4 module, do not affect other modules and
> increase the difficulty of locating problems.
>
> This patch avoid inode->i_nlink reverse and remove the inode form the

(typo) s/form/from/

> orphan list when destroy it if the list is not empty.
> Signed-off-by: yi zhang <[email protected]>
> ---
> fs/ext4/super.c | 1 +
> fs/inode.c | 5 ++++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 52b0530..617327e 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -975,6 +975,7 @@ static void ext4_destroy_inode(struct inode *inode)
> EXT4_I(inode), sizeof(struct ext4_inode_info),
> true);
> dump_stack();
> + ext4_orphan_del(NULL, inode);
> }
> call_rcu(&inode->i_rcu, ext4_i_callback);
> }
> diff --git a/fs/inode.c b/fs/inode.c
> index 88110fd..079d383 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -279,7 +279,10 @@ static void destroy_inode(struct inode *inode)
> */
> void drop_nlink(struct inode *inode)
> {
> - WARN_ON(inode->i_nlink == 0);
> + if (WARN(inode->i_nlink == 0, "inode %lu nlink"
> + " is already 0", inode->i_ino))

(style) the string should be kept on a single line instead of being
split, especially since it can fit easily.

(defect) this needs to have a newline.

if (WARN(inode->i_nlink == 0,
"inode %lu nlink is already 0\n", inode->i_ino))

Cheers, Andreas

> + return;
> +
> inode->__i_nlink--;
> if (!inode->i_nlink)
> atomic_long_inc(&inode->i_sb->s_remove_count);
> --
> 2.5.0
>


Cheers, Andreas






Attachments:
signature.asc (833.00 B)
Message signed with OpenPGP using GPGMail

2016-12-31 22:59:41

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [RFC PATCH] ext4: increase the protection of drop nlink and ext4 inode destroy

On Mon, 26 Dec 2016 20:34:17 +0800, yi zhang said:
> Because of the disk and hardware issue, the ext4 filesystem have
> many errors, the inode->i_nlink of ext4 becomes zero abnormally
> but the dentry is still positive, it will cause memory corruption
> after the following process:
>
> 1) Due to the inode->i_nlink is 0, this inode will be added into
> the orhpan list,

> + if (WARN(inode->i_nlink == 0, "inode %lu nlink"
> + " is already 0", inode->i_ino))

Can we get the filesystem? Or at least the device major/minor? If a system
has multiple large ext4 filesystems, it would be helpful to know which
one is having the problem.


Attachments:
(No filename) (484.00 B)