* WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50()
@ 2015-11-23 7:55 Vegard Nossum
2015-11-23 22:21 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Vegard Nossum @ 2015-11-23 7:55 UTC (permalink / raw)
To: OGAWA Hirofumi; +Cc: Richard Weinberger, LKML
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
Hi,
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()
CPU: 0 PID: 913 Comm: a.out Not tainted 4.2.5+ #39
Stack:
e0931b50 60075412 e13981e8 00000009
00000000 605cc684 e0931b60 605cf637
e0931bc0 60040f6d e0931ba0 6011e12b
Call Trace:
[<60029f3b>] show_stack+0xdb/0x1a0
[<605cf637>] dump_stack+0x2a/0x2c
[<60040f6d>] warn_slowpath_common+0x9d/0xf0
[<6004114c>] warn_slowpath_null+0x1c/0x20
[<6011e12b>] drop_nlink+0x4b/0x50
[<601e9e2f>] vfat_rename+0x56f/0x800
[<60113dd2>] vfs_rename+0x9a2/0x9d0
[<60114439>] SyS_renameat2+0x639/0x690
[<601144d0>] SyS_rename+0x20/0x30
[<6002c5ce>] handle_syscall+0x6e/0xa0
[<6003a911>] userspace+0x4f1/0x5e0
To trigger it, you have to do a rename("/mnt/a/b/1", "/mnt/1"), where
/mnt is your mountpoint.
Also happens on 3.13.0 Ubuntu trusty kernel.
Vegard
[-- Attachment #2: vfat2.img.bz2 --]
[-- Type: application/x-bzip, Size: 615 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-23 7:55 WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() Vegard Nossum @ 2015-11-23 22:21 ` Richard Weinberger 2015-11-24 8:08 ` Vegard Nossum 0 siblings, 1 reply; 9+ messages in thread From: Richard Weinberger @ 2015-11-23 22:21 UTC (permalink / raw) To: Vegard Nossum, OGAWA Hirofumi; +Cc: LKML Am 23.11.2015 um 08:55 schrieb Vegard Nossum: > Hi, > > 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() > CPU: 0 PID: 913 Comm: a.out Not tainted 4.2.5+ #39 > Stack: > e0931b50 60075412 e13981e8 00000009 > 00000000 605cc684 e0931b60 605cf637 > e0931bc0 60040f6d e0931ba0 6011e12b > Call Trace: > [<60029f3b>] show_stack+0xdb/0x1a0 > [<605cf637>] dump_stack+0x2a/0x2c > [<60040f6d>] warn_slowpath_common+0x9d/0xf0 > [<6004114c>] warn_slowpath_null+0x1c/0x20 > [<6011e12b>] drop_nlink+0x4b/0x50 > [<601e9e2f>] vfat_rename+0x56f/0x800 > [<60113dd2>] vfs_rename+0x9a2/0x9d0 > [<60114439>] SyS_renameat2+0x639/0x690 > [<601144d0>] SyS_rename+0x20/0x30 > [<6002c5ce>] handle_syscall+0x6e/0xa0 > [<6003a911>] userspace+0x4f1/0x5e0 > > To trigger it, you have to do a rename("/mnt/a/b/1", "/mnt/1"), where > /mnt is your mountpoint. Not here. All I get is: FAT-fs (ubdb): Corrupted directory (i_pos 244) Did you something before the rename()? Thanks, //richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-23 22:21 ` Richard Weinberger @ 2015-11-24 8:08 ` Vegard Nossum 2015-11-25 21:54 ` OGAWA Hirofumi 0 siblings, 1 reply; 9+ messages in thread From: Vegard Nossum @ 2015-11-24 8:08 UTC (permalink / raw) To: Richard Weinberger, OGAWA Hirofumi; +Cc: LKML 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() [...] >> >> To trigger it, you have to do a rename("/mnt/a/b/1", "/mnt/1"), where >> /mnt is your mountpoint. > > Not here. All I get is: > FAT-fs (ubdb): Corrupted directory (i_pos 244) > > Did you something before the rename()? No, nothing before the rename. Did you use mv to generate the rename()? Then you may have to do 'mv /mnt/a/b/1 /mnt/', otherwise it ends up doing rename("/mnt/a/b/1", "/mnt/1/1") which only shows the message you saw. Let me know if this helps. Thanks for having a look, Vegard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-24 8:08 ` Vegard Nossum @ 2015-11-25 21:54 ` OGAWA Hirofumi 2015-11-26 8:12 ` Vegard Nossum 0 siblings, 1 reply; 9+ messages in thread From: OGAWA Hirofumi @ 2015-11-25 21:54 UTC (permalink / raw) To: Vegard Nossum; +Cc: Richard Weinberger, LKML 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() > [...] >>> >>> To trigger it, you have to do a rename("/mnt/a/b/1", "/mnt/1"), where >>> /mnt is your mountpoint. >> >> Not here. All I get is: >> FAT-fs (ubdb): Corrupted directory (i_pos 244) >> >> Did you something before the rename()? > > No, nothing before the rename. > > Did you use mv to generate the rename()? Then you may have to do 'mv > /mnt/a/b/1 /mnt/', otherwise it ends up doing rename("/mnt/a/b/1", > "/mnt/1/1") which only shows the message you saw. Let me know if this helps. > > Thanks for having a look, Can you try this one? -- 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. 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, _ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-25 21:54 ` OGAWA Hirofumi @ 2015-11-26 8:12 ` Vegard Nossum 2015-11-26 8:26 ` OGAWA Hirofumi 2015-11-26 8:30 ` OGAWA Hirofumi 0 siblings, 2 replies; 9+ messages in thread From: Vegard Nossum @ 2015-11-26 8:12 UTC (permalink / raw) To: OGAWA Hirofumi; +Cc: Richard Weinberger, LKML 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! The last potential issue I'm seeing (completely unrelated to your patch) is this: [ 340.610000] VFS: Lookup of '1' in vfat loop0 would have caused loop [ 354.360000] d_splice_alias: 1104 callbacks suppressed [ 354.360000] VFS: Lookup of '1' in vfat loop0 would have caused loop Is that worth investigating closer? Thanks, Vegard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-26 8:12 ` Vegard Nossum @ 2015-11-26 8:26 ` OGAWA Hirofumi 2015-11-26 8:30 ` OGAWA Hirofumi 1 sibling, 0 replies; 9+ messages in thread From: OGAWA Hirofumi @ 2015-11-26 8:26 UTC (permalink / raw) To: Vegard Nossum; +Cc: Richard Weinberger, LKML 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! > > The last potential issue I'm seeing (completely unrelated to your patch) > is this: > > [ 340.610000] VFS: Lookup of '1' in vfat loop0 would have caused loop > [ 354.360000] d_splice_alias: 1104 callbacks suppressed > [ 354.360000] VFS: Lookup of '1' in vfat loop0 would have caused loop > > Is that worth investigating closer? It looks like corruption detected with ratelimited printk (at vfs level. dir is hardlink of ancestor). IOW, it looks like intended behavior. Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-26 8:12 ` Vegard Nossum 2015-11-26 8:26 ` OGAWA Hirofumi @ 2015-11-26 8:30 ` OGAWA Hirofumi 2015-12-13 22:19 ` Vegard Nossum 1 sibling, 1 reply; 9+ messages in thread From: OGAWA Hirofumi @ 2015-11-26 8:30 UTC (permalink / raw) To: Andrew Morton; +Cc: Vegard Nossum, Richard Weinberger, LKML 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, _ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-11-26 8:30 ` OGAWA Hirofumi @ 2015-12-13 22:19 ` Vegard Nossum 2015-12-14 22:19 ` Andrew Morton 0 siblings, 1 reply; 9+ messages in thread From: Vegard Nossum @ 2015-12-13 22:19 UTC (permalink / raw) To: OGAWA Hirofumi, Andrew Morton; +Cc: Richard Weinberger, LKML On 11/26/2015 09:30 AM, OGAWA Hirofumi wrote: > 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. Sorry to bug you, I didn't see this merged yet and just making sure this doesn't slip through the cracks. Or should I expect it only for the next merge window? Vegard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() 2015-12-13 22:19 ` Vegard Nossum @ 2015-12-14 22:19 ` Andrew Morton 0 siblings, 0 replies; 9+ messages in thread From: Andrew Morton @ 2015-12-14 22:19 UTC (permalink / raw) To: Vegard Nossum; +Cc: OGAWA Hirofumi, Richard Weinberger, LKML On Sun, 13 Dec 2015 23:19:31 +0100 Vegard Nossum <vegard.nossum@oracle.com> wrote: > On 11/26/2015 09:30 AM, OGAWA Hirofumi wrote: > > 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. > > Sorry to bug you, I didn't see this merged yet and just making sure this > doesn't slip through the cracks. Or should I expect it only for the next > merge window? I merged this into -mm on Nov 11. I'd assumed it was 4.5 material. Was that wrong? ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-12-14 22:19 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-23 7:55 WARNING: CPU: 0 PID: 913 at fs/inode.c:275 drop_nlink+0x4b/0x50() Vegard Nossum 2015-11-23 22:21 ` Richard Weinberger 2015-11-24 8:08 ` Vegard Nossum 2015-11-25 21:54 ` OGAWA Hirofumi 2015-11-26 8:12 ` Vegard Nossum 2015-11-26 8:26 ` OGAWA Hirofumi 2015-11-26 8:30 ` OGAWA Hirofumi 2015-12-13 22:19 ` Vegard Nossum 2015-12-14 22:19 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox