Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50()

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Vegard Nossum <vegard.nossum@oracle.com> writes:

> On 11/25/2015 10:54 PM, OGAWA Hirofumi wrote:
>> Vegard Nossum <vegard.nossum@oracle.com> writes:
>>
>>> On 11/23/2015 11:21 PM, Richard Weinberger wrote:
>>>> Am 23.11.2015 um 08:55 schrieb Vegard Nossum:
>>>>> With the attached vfat disk image (fuzzed), I get the following WARNING:
>>>>>
>>>>> WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50()
>
> [...]
>
>>
>> Can you try this one?
>>
>
> That seems to fix the problem here, thanks!

Andrew, please queue this up for next chance.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>


[PATCH] fat: Add simple validation for directory inode


This detects simple corruption cases of directory, and try to avoid
further damage to user data.

And performance impact of this validation should be very low, or not
measurable.

Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Tested-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/inode.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff -puN fs/fat/inode.c~fat-validate-dir fs/fat/inode.c
--- linux/fs/fat/inode.c~fat-validate-dir	2015-11-26 06:31:39.666959958 +0900
+++ linux-hirofumi/fs/fat/inode.c	2015-11-26 06:31:39.670959945 +0900
@@ -449,6 +449,24 @@ static int fat_calc_dir_size(struct inod
 	return 0;
 }
 
+static int fat_validate_dir(struct inode *dir)
+{
+	struct super_block *sb = dir->i_sb;
+
+	if (dir->i_nlink < 2) {
+		/* Directory should have "."/".." entries at least. */
+		fat_fs_error(sb, "corrupted directory (invalid entries)");
+		return -EIO;
+	}
+	if (MSDOS_I(dir)->i_start == 0 ||
+	    MSDOS_I(dir)->i_start == MSDOS_SB(sb)->root_cluster) {
+		/* Directory should point valid cluster. */
+		fat_fs_error(sb, "corrupted directory (invalid i_start)");
+		return -EIO;
+	}
+	return 0;
+}
+
 /* doesn't deal with root inode */
 int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 {
@@ -475,6 +493,10 @@ int fat_fill_inode(struct inode *inode,
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 
 		set_nlink(inode, fat_subdirs(inode));
+
+		error = fat_validate_dir(inode);
+		if (error < 0)
+			return error;
 	} else { /* not a directory */
 		inode->i_generation |= 1;
 		inode->i_mode = fat_make_mode(sbi, de->attr,
_