Bug 263220 - invalid fusefs error numbers can cause kernel crash
Summary: invalid fusefs error numbers can cause kernel crash
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL: https://reviews.freebsd.org/D34931
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-11 13:55 UTC by Robert Morris
Modified: 2022-08-20 00:55 UTC (History)
3 users (show)

See Also:
asomers: mfc-stable13+
asomers: mfc-stable12-


Attachments
a fuse daemon that crashes the kernel by setting error=2 (9.22 KB, text/plain)
2022-04-11 13:55 UTC, Robert Morris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2022-04-11 13:55:25 UTC
Created attachment 233146 [details]
a fuse daemon that crashes the kernel by setting error=2

If a fuse daemon responds to a FUSE_LOOKUP with error set to 2,
fuse_device_write() negates this to -2, which is EJUSTRETURN. As a
result, vfs_lookup() will return 0 for success, but leave ni_vp NULL.
kern_statat() assumes that a zero return from namei() implies that
nd.ni_vp is valid, so it crashes in VOP_STAT(nd.ni_vp, ...).

I've included a demo:

# uname -a
FreeBSD  14.0-CURRENT FreeBSD 14.0-CURRENT #187 main-n250915-a8123f770b1e-dirty: Mon Apr 11 07:07:40 EDT 2022     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM  riscv
# pkg install fusefs-libs
# cc -I/usr/local/include/fuse -o futo0 futo0.c -L/usr/local/lib -lfuse
# ./futo0
...
running touch /mnt/z
...
panic: Fatal page fault at 0xffffffc00039f47a: 0x00000000000010
cpuid = 1
time = 1649684546
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_trace_self_wrapper() at db_trace_self_wrapper+0x38
kdb_backtrace() at kdb_backtrace+0x2c
vpanic() at vpanic+0x16e
panic() at panic+0x2a
page_fault_handler() at page_fault_handler+0x1aa
do_trap_supervisor() at do_trap_supervisor+0x76
cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x70
--- exception 13, tval = 0x10
VOP_STAT() at VOP_STAT+0x24
kern_statat() at kern_statat+0x10a
sys_fstatat() at sys_fstatat+0x1e
syscallenter() at syscallenter+0xf4
ecall_handler() at ecall_handler+0x18
do_trap_user() at do_trap_user+0xea
cpu_exception_handler_user() at cpu_exception_handler_user+0x72
--- exception 8, tval = 0
KDB: enter: panic
[ thread pid 100 tid 100051 ]
Stopped at      breakpoint+0xa: c.ldsp  s0,0(sp)
db>
Comment 1 Alan Somers freebsd_committer freebsd_triage 2022-04-11 16:35:12 UTC
Thanks for the good bug report.  I'll try to get this fixed soon.
Comment 2 Alan Somers freebsd_committer freebsd_triage 2022-04-15 19:09:32 UTC
Fix in code review
Comment 3 commit-hook freebsd_committer freebsd_triage 2022-04-15 19:59:20 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=155ac516c60f20573d15c54bafabfd0e52d21fa6

commit 155ac516c60f20573d15c54bafabfd0e52d21fa6
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-15 19:04:24 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-04-15 19:57:32 +0000

    fusefs: validate servers' error values

    Formerly fusefs would pass up the stack any error value returned by the
    fuse server.  However, some values aren't valid for userland, but have
    special meanings within the kernel.  One of these, EJUSTRETURN, could
    cause a kernel page fault if the server returned it in response to
    FUSE_LOOKUP.  Fix by validating all errors returned by the server.

    Also, fix a data lifetime bug in the FUSE_DESTROY test.

    PR:             263220
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    MFC after:      3 weeks
    Sponsored by:   Axcient
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D34931

 sys/fs/fuse/fuse_device.c     | 14 ++++++++++++--
 tests/sys/fs/fusefs/lookup.cc | 21 +++++++++++++++++++++
 tests/sys/fs/fusefs/mockfs.cc |  9 ++++++++-
 tests/sys/fs/fusefs/mockfs.hh |  3 +++
 tests/sys/fs/fusefs/utils.cc  |  2 +-
 5 files changed, 45 insertions(+), 4 deletions(-)
Comment 4 Alan Somers freebsd_committer freebsd_triage 2022-04-15 20:00:27 UTC
Ok Robert, the fix is in.  But I'm curious.  How did you find it?  I assume that you were either writing a new fuse file system or porting an existing one. If it's open source, I'd like to know.  It's always good to hear how people are using fusefs.
Comment 5 Ed Maste freebsd_committer freebsd_triage 2022-04-23 02:26:54 UTC
(In reply to Alan Somers from comment #4)
A comment from another bug might give a hint about how this was found: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=257193#c4
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-05-12 20:39:01 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ef0e12d5656ad471dcc93a15c63a6af0d1d62bd9

commit ef0e12d5656ad471dcc93a15c63a6af0d1d62bd9
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-15 19:04:24 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-05-12 20:37:24 +0000

    fusefs: validate servers' error values

    Formerly fusefs would pass up the stack any error value returned by the
    fuse server.  However, some values aren't valid for userland, but have
    special meanings within the kernel.  One of these, EJUSTRETURN, could
    cause a kernel page fault if the server returned it in response to
    FUSE_LOOKUP.  Fix by validating all errors returned by the server.

    Also, fix a data lifetime bug in the FUSE_DESTROY test.

    PR:             263220
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Sponsored by:   Axcient
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D34931

    (cherry picked from commit 155ac516c60f20573d15c54bafabfd0e52d21fa6)

 sys/fs/fuse/fuse_device.c     | 14 ++++++++++++--
 tests/sys/fs/fusefs/lookup.cc | 21 +++++++++++++++++++++
 tests/sys/fs/fusefs/mockfs.cc |  9 ++++++++-
 tests/sys/fs/fusefs/mockfs.hh |  3 +++
 tests/sys/fs/fusefs/utils.cc  |  2 +-
 5 files changed, 45 insertions(+), 4 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-08-20 00:55:05 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=73e02a370e524eb1e5719d9c37098e29c0d9be78

commit 73e02a370e524eb1e5719d9c37098e29c0d9be78
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-15 19:04:24 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-08-20 00:49:23 +0000

    fusefs: validate servers' error values

    Formerly fusefs would pass up the stack any error value returned by the
    fuse server.  However, some values aren't valid for userland, but have
    special meanings within the kernel.  One of these, EJUSTRETURN, could
    cause a kernel page fault if the server returned it in response to
    FUSE_LOOKUP.  Fix by validating all errors returned by the server.

    Also, fix a data lifetime bug in the FUSE_DESTROY test.

    PR:             263220
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Sponsored by:   Axcient
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D34931

    (cherry picked from commit 155ac516c60f20573d15c54bafabfd0e52d21fa6)

 sys/fs/fuse/fuse_device.c     | 14 ++++++++++++--
 tests/sys/fs/fusefs/lookup.cc | 21 +++++++++++++++++++++
 tests/sys/fs/fusefs/mockfs.cc |  9 ++++++++-
 tests/sys/fs/fusefs/mockfs.hh |  3 +++
 tests/sys/fs/fusefs/utils.cc  |  2 +-
 5 files changed, 45 insertions(+), 4 deletions(-)