Bug 206530 - ext2fs: fsck.ext3 reports "Inode 157938 has INDEX_FL flag set but is not a directory"
Summary: ext2fs: fsck.ext3 reports "Inode 157938 has INDEX_FL flag set but is not a di...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Pedro F. Giffuni
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-01-23 16:03 UTC by Damjan Jovanovic
Modified: 2016-01-24 02:45 UTC (History)
2 users (show)

See Also:


Attachments
Initialize the inode's i_flag to 0 during allocation (846 bytes, patch)
2016-01-23 16:03 UTC, Damjan Jovanovic
no flags Details | Diff
Alternative workaround (1.34 KB, patch)
2016-01-23 21:01 UTC, Pedro F. Giffuni
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Damjan Jovanovic 2016-01-23 16:03:22 UTC
Created attachment 166011 [details]
Initialize the inode's i_flag to 0 during allocation

fsck.ext3 on a cleanly unmounted EXT3 filesystem that went through heavy file creation (OpenOffice build, about 80000 new files) gives a dozen or so of these errors during "Pass 1: Checking inodes, blocks, and sizes":

Inode 157938 has INDEX_FL flag set but is not a directory.
Clear HTree index?

This error is benign to our ext2fs driver, as it requires the inode's mode to be directory for this flag to have any effect. However it's not benign to other EXT3 implementations - it breaks at least fsck.ext3 itself if the "-n" option to it is used or "no" is answered to that question, since it misinterprets the inode as a directory, giving further false errors because that pseudo-directory will appear corrupt.

Adding this hack:

  if (!S_ISDIR(ip->i_mode) && (ip->i_flag & IN_E4INDEX) != 0)
    panic("non-directory has index!?\n");

to ext2_i2ei() to catch wrong use of this flag just before the inode is written to disk, produces this revealing stack trace:

KDB: stack backtrace:
db_trace_self_wrapper()
vpanic()
panic()
ext2_i2ei()
ext2_update()
ext2_makeinode()
ext2_create()
VOP_CREATE_APV()
vn_open_cred()
kern_openat()
amd64_syscall()
Xfast_syscall()

Reading through those functions shows ext2_makeinode() calls ext2_valloc() which apparently reuses inodes from disk without initializing their i_flag field, hence if a previously deleted directory's inode is reused for a file, the IN_E4INDEX flag from it will still be set, and wrongly written to the file's inode!

I am attaching a patch that initializes i_flag to 0. With it, fsck.ext3 reports a clean scan after the same test.
Comment 1 Pedro F. Giffuni freebsd_committer 2016-01-23 17:57:43 UTC
Take ...
Comment 2 Pedro F. Giffuni freebsd_committer 2016-01-23 18:47:10 UTC
(In reply to Damjan Jovanovic from comment #0)

While your patch is not wrong, the root cause is elsewhere.
IN_E4INDEX and IN_E4EXTENTS are Ext4 attributes that we hide in i_flag
but they should never get written since we don't have Ext4 write support.

Curiously, it appears the situation also happened at some point on linux:
http://sourceforge.net/p/e2fsprogs/bugs/41/
Comment 3 Pedro F. Giffuni freebsd_committer 2016-01-23 21:01:34 UTC
Created attachment 166027 [details]
Alternative workaround

I still have to check if we are handling i_flag correctly but please test this workaround for now.
Comment 4 Pedro F. Giffuni freebsd_committer 2016-01-23 21:21:53 UTC
Add Zheng Liu
Comment 5 Damjan Jovanovic 2016-01-24 00:22:01 UTC
Why do you think INDEX_FL is an EXT4-only flag?

Linux has an EXT3_INDEX_FL (http://lxr.free-electrons.com/ident?v=3.14;i=EXT3_INDEX_FL).
Comment 6 Pedro F. Giffuni freebsd_committer 2016-01-24 02:20:07 UTC
(In reply to Damjan Jovanovic from comment #5)
Ah .. nice!

We basically added those attributes when looking at ext4 and I had missed the EXT3 attribute completely.

It has the same value as the ext4 one so we should rename IN_E4INDEX.

Thanks!
Comment 7 commit-hook freebsd_committer 2016-01-24 02:26:44 UTC
A commit references this bug:

Author: pfg
Date: Sun Jan 24 02:25:42 UTC 2016
New revision: 294652
URL: https://svnweb.freebsd.org/changeset/base/294652

Log:
  ext2: Initialize i_flag after allocation.

  We use i_flag to carry some flags like IN_E4INDEX which newer
  ext2fs variants uses internally.

  fsck.ext3 rightfully complains after our implementation tags
  non-directory inodes with INDEX_FL.

  Initializing i_flag during allocation removes the noise factor
  and quiets down fsck.

  Patch from:	Damjan Jovanovic
  PR:		206530

Changes:
  head/sys/fs/ext2fs/ext2_alloc.c
Comment 8 Pedro F. Giffuni freebsd_committer 2016-01-24 02:45:30 UTC
Committed, thanks!