Bug 205938 - [ext2fs][patch][panic] EXT4: reading mmaped file causes panic because struct buf leaks
Summary: [ext2fs][patch][panic] EXT4: reading mmaped file causes panic because struct ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Pedro F. Giffuni
URL:
Keywords: crash, patch
Depends on:
Blocks:
 
Reported: 2016-01-05 23:53 UTC by Damjan Jovanovic
Modified: 2016-01-10 22:35 UTC (History)
3 users (show)

See Also:


Attachments
Fix a kernel panic when reading mmaped files from EXT4 (1.49 KB, patch)
2016-01-05 23:53 UTC, Damjan Jovanovic
no flags Details | Diff
Fix reading mmaped files from EXT4 (642 bytes, patch)
2016-01-07 21:32 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-05 23:53:26 UTC
Created attachment 165127 [details]
Fix a kernel panic when reading mmaped files from EXT4

Calling mmap() on any sizeable file on an EXT4 filesystem, and then attempting to read that memory (can be easily tested using the "cmp file file" tool), causes a reproducible kernel panic:

userret: returning with the following locks held:
exclusive lockmgr bufwait (bufwait) r = 0 (0xfffffe001d90c220) locked @ /usr/src/sys/kern/vfs_bio.c:1454
panic: witness_warn
cpuid = 0
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace-self_wrapper+0x2b/frame 0xfffffe002b7e67f0
vpanic() at vpanic+0x182/frame 0xfffffe002b7e6870
kassert_panic() at kassert_panic+0x126/frame 0xfffffe002b7e68e0
witness_warn() at witness_warn+0x3c6/frame 0xfffffe002b7e69b0
userret() at userret+0x98/frame 0xfffffe002b7e69e0
trap() at trap+0x3f4/frame 0xfffffe002b7e6bf0
calltrap() at calltrap+0x8/frame 0xfffffe002b7e6bf0
--- trap 0xc, rip = 0x4019c0, rsp = 0x7fffffffe940, rbp = 0x7ffffffffeea30 ---
KDB: enter: panic
[ thread pid 909 tid 100082 ]
Stopped at      kdb_enter+0x3b: movq    $0,kdb_why


The problem comes from ext4_bmapext() in sys/fs/ext2fs/ext2_bmap.c never calling brelse(), meaning the "struct buf" returned in path.ep_bp from ext4_ext_find_extent() is never released/unlocked, something userret() catches later and panics from.

The attached patch always calls brelse(path.ep_bp), fixing reading EXT4 files using mmap().

This affects all versions of FreeBSD.
Comment 1 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-06 16:10:01 UTC
Patch looks good.
Add Zheng Liu (author of original code) for review.
Take.
Comment 2 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-07 21:32:04 UTC
Created attachment 165231 [details]
Fix reading mmaped files from EXT4

Small change: it doesn't seem like defining "ret" in necessary or useful.
Please review.
Comment 3 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-07 21:34:59 UTC
Comment on attachment 165127 [details]
Fix a kernel panic when reading mmaped files from EXT4

Nevermind... I got that wrong.
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-01-07 21:44:05 UTC
A commit references this bug:

Author: pfg
Date: Thu Jan  7 21:43:43 UTC 2016
New revision: 293370
URL: https://svnweb.freebsd.org/changeset/base/293370

Log:
  ext2fs: reading mmaped file in Ext4 causes panic

  Always call brelse(path.ep_bp), fixing reading EXT4 files using mmap().

  Patch by Damjan Jovanovic.

  PR:		205938
  MFC after:	1 week

Changes:
  head/sys/fs/ext2fs/ext2_bmap.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-01-10 22:31:05 UTC
A commit references this bug:

Author: pfg
Date: Sun Jan 10 22:30:40 UTC 2016
New revision: 293646
URL: https://svnweb.freebsd.org/changeset/base/293646

Log:
  MFC	r293370:
  ext2fs: reading mmaped file in Ext4 causes panic

  Always call brelse(path.ep_bp), fixing reading EXT4 files using mmap().

  Patch by Damjan Jovanovic.

  PR:		205938

Changes:
_U  stable/10/
  stable/10/sys/fs/ext2fs/ext2_bmap.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-01-10 22:32:07 UTC
A commit references this bug:

Author: pfg
Date: Sun Jan 10 22:31:39 UTC 2016
New revision: 293647
URL: https://svnweb.freebsd.org/changeset/base/293647

Log:
  MFC	r293370:
  ext2fs: reading mmaped file in Ext4 causes panic

  Always call brelse(path.ep_bp), fixing reading EXT4 files using mmap().

  Patch by Damjan Jovanovic.

  PR:		205938

Changes:
_U  stable/9/sys/
_U  stable/9/sys/fs/
  stable/9/sys/fs/ext2fs/ext2_bmap.c
Comment 7 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-01-10 22:35:44 UTC
MFC'd a little early since avoiding panics is good.

Thanks!