Bug 206056 - [ext2fs][patch][panic] EXT4: mount panic from freeing invalid pointers
Summary: [ext2fs][patch][panic] EXT4: mount panic from freeing invalid pointers
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: crash, patch
Depends on:
Blocks:
 
Reported: 2016-01-09 05:16 UTC by Damjan Jovanovic
Modified: 2016-01-14 01:54 UTC (History)
2 users (show)

See Also:
pfg: mfc-stable10+
pfg: mfc-stable9+


Attachments
Preventing a panic when pointers from struct ext2mount's um_e2fs are freed (816 bytes, patch)
2016-01-09 05:16 UTC, Damjan Jovanovic
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-09 05:16:32 UTC
Created attachment 165290 [details]
Preventing a panic when pointers from struct ext2mount's um_e2fs are freed

On Linux I made a 500MB EXT4 filesystem for testing, and when I tried to mount it in FreeBSD with:

mdconfig -a /path/to/filesystem
mount -t ext2fs -o ro /dev/md0 /path/to/mountpoint

the following error got printed out, followed immediately by a panic:

ext2fs: no space for extra inode timestamps

Fatal trap 12: page fault while in kernel mode
cpuid = 0; apicid = 00
fault_virtual_address = 0x4
fault code            = supervisor read, page not present
instruction pointer   = 0x20:0xc0b1f1cc
stack pointer         = 0x28:0xcebee898
frame pointer         = 0x28:0xcebee8c0
code segment          = base rx0, limit 0xfffff, type 0x1b
                      = DPL 0, pres 1, def32 1, gran 1
processor eflags      = interrupt enabled, resume, IOPL = 0
current process       = 777 (mount)
[ thread pid 777 tid 100065 ]
Stopped at      free+0x5c:    movl    0x4(%eax),%eax

db> bt
Tracing pid 777 tid 100065 td 0xc4e0c620
free(aa,c54ab298,2a3,2a1,0,...) at free+0x5c/frame 0xcebee8c0
ext2_mount(c4e16a80,c54ab208,c5374380,c4e10800,c4c40a70,...) at ext2_mount+0x1604/frame 0xceebe9e8
vfs_donmount(c4e4c620,1,0,c4c11b00,c4c11b00,...) at vfs_donmount+0xdc6/frame 0xceebebf0
sys_nmount(c4e0c620,ceebeca8,c506890c,c4e0c620,c506890c,...) at sys_nmount+0x78/frame 0xceebec18
syscall(ceebece8) at syscall+0x4a6/frame 0xceebecdc
Xint0x80_syscall() at Xint0x80_syscall+0x21/frame 0xceebecdc
--- syscall (378, FreeBSD ELF32, sys_nmount), eip = 0x280e013b, esp = 0xbfbfdd20, ebp = 0xbfbfe278


The "ext2fs: no space for extra inode timestamps" message comes from compute_sb_data() in ext2_vfsops.c, which returns EINVAL after printing it, never reaching the subsequent lines that initialize fs->e2fs_gd and fs->e2fs_contigdirs. When ext2_mountfs() calls compute_sb_data(), it does a "goto out" on error, and in "out" it attempts to free() those 2 fields. Since the memory for the struct those fields are in wasn't initialized when it was allocated, free() is being passed invalid pointers, resulting in a panic.

The attached patch initializes the struct with those fields to zeroes on allocation, preventing the panic.

I'll investigate the original error that caused this buggy error path to be taken in a separate issue.
Comment 1 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-11 18:58:23 UTC
Take
Comment 2 commit-hook freebsd_committer freebsd_triage 2016-01-11 19:25:53 UTC
A commit references this bug:

Author: pfg
Date: Mon Jan 11 19:25:44 UTC 2016
New revision: 293683
URL: https://svnweb.freebsd.org/changeset/base/293683

Log:
  ext4: mount panic from freeing invalid pointers

  Initialize the struct with those fields to zeroes on allocation,
  preventing the panic.

  Patch by:	Damjan Jovanovic.

  PR:		206056
  MFC after:	3 days

Changes:
  head/sys/fs/ext2fs/ext2_vfsops.c
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2016-01-11 20:17:08 UTC
Leaving mfc-stableX flags ? until committed in those branches as per bug 205816 Commetn 14
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2016-01-11 20:17:28 UTC
I mean bug 205816 Comment 14
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-01-14 01:50:41 UTC
A commit references this bug:

Author: pfg
Date: Thu Jan 14 01:50:06 UTC 2016
New revision: 293866
URL: https://svnweb.freebsd.org/changeset/base/293866

Log:
  MFC	r293683:
  ext4: mount panic from freeing invalid pointers

  Initialize the struct with those fields to zeroes on allocation,
  preventing the panic.

  Patch by:	Damjan Jovanovic.

  PR:		206056

Changes:
_U  stable/10/
  stable/10/sys/fs/ext2fs/ext2_vfsops.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-01-14 01:51:44 UTC
A commit references this bug:

Author: pfg
Date: Thu Jan 14 01:51:18 UTC 2016
New revision: 293867
URL: https://svnweb.freebsd.org/changeset/base/293867

Log:
  MFC	r293683:
  ext4: mount panic from freeing invalid pointers

  Initialize the struct with those fields to zeroes on allocation,
  preventing the panic.

  Patch by:	Damjan Jovanovic.

  PR:		206056

Changes:
_U  stable/9/sys/
_U  stable/9/sys/fs/
  stable/9/sys/fs/ext2fs/ext2_vfsops.c
Comment 7 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-14 01:54:40 UTC
Committed and MFCd.
Thanks!