[Bug 216541] FUZZ: general protection fault, KASAN: null-ptr-deref at fs/ext4/ialloc.c:ext4_read_inode_bitmap() when mount a corrupted image

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=216541

Theodore Tso (tytso@mit.edu) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tytso@mit.edu

--- Comment #1 from Theodore Tso (tytso@mit.edu) ---
I've done some analysis on this failure and what is going on is the following.

1)   The journal inode in the fuzzed image is the normal journal inode, #8. 
HOWEVER, after the journal is replayed, the journal overwrites the superblock
with new one where the journal inode is different; it is now #32.

2)  Next, we set up the set of static metadata blocks ("the system zone") that
should never be used by any data blocks in fs/ext4/block_validity.c.    This
includes the blocks used by the journal inode (which never change while the
file system is mounted).  In order to reserve those blocks in the system zone,
ext4_protect_reserved_inode() fetches the journal inode using ext4_iget(), and
then later releases it using iput().

3)  This would be fine for a valid file system journal, but after the journal
replay, the s_journal_inum now has 32.  And inode 32 has an i_links_count of 0.
  That's a problem, because now when we call iput(), since the VFS layers sees
that the links count is zero, it calls evict() so that the inode can be
deallocated.   And at this point in the file system mount operation, we're not
set up to deallocate any blocks or inodes.  And this is what triggers the NULL
pointer dereference.


Fixes:

FIX A)  In ext4_iget(), if we are getting a special inode, the links count must
be > 0.  If not, when that special inode (whether it is the root directory, the
journal inode, or the quota inode) is finally released using iput, the system
will attempt to deallocate the special inode, with the resulting hilarity
ensuing.   So if i_links_count is 0, we should set the returned inode to be the
bad inode, and return -EFSCORRUPTED.

FIX B)   In ext4_check_blockref(), we skip all of the checks if the inode in
question is the journal inode.   We shouldn't check to see if the journal's
blocks overlaps with the metadata blocks (which include the journal inode, so
it will always overlap with itself) --- but we should check to make sure the
block number is valid and does not exceed the file system limits.   This is not
critical for fixing the bug shown here, but it does add a missing check which
was unnecessarily exempted by commit 170417c8c7bb2 ("ext4: fix block validity
checks for journal inodes using indirect blocks").

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.