Bug 266142 - SNDSTIOC_ADD_USER_DEVS ioctl on /dev/sndstat passes unchecked size from user to malloc() -> potential crash
Summary: SNDSTIOC_ADD_USER_DEVS ioctl on /dev/sndstat passes unchecked size from user ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-31 20:30 UTC by Robert Morris
Modified: 2024-05-22 13:24 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2022-08-31 20:30:49 UTC
User code specifies a 64-bit length to the SNDSTIOC_ADD_USER_DEVS
ioctl, which sndstat_add_user_devs() passes to
sndstat_unpack_user_nvlbuf() (as arg->nbytes), which is then passed as
the size to malloc(). One way this can go wrong is if the user passes
in (size_t)-1, which this line in malloc_large() turns into zero:

        size = roundup(size, PAGE_SIZE);

which either causes the MPASS(size > 0) in vmem_alloc() to fail,
or (if not INVARIANTS) causes a later kernel page fault.

A demo:

int main() {
  int fd = open("/dev/sndstat", 2);
  if(fd < 0) { perror("/dev/sndstat"); exit(1); }
  char buf[128];
  memset(buf, 0xff, sizeof(buf));
  ioctl(fd, 0xc0104466, buf); // SNDSTIOC_ADD_USER_DEVS
}

# uname -a
FreeBSD  14.0-CURRENT FreeBSD 14.0-CURRENT #71 main-n250928-b8170f38ccc7-dirty: Wed Aug 31 16:05:23 EDT 2022     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv
# cc x.c
# ./a.out
panic: Fatal page fault at 0xffffffc00039b2be: 0x27fffd000809460
cpuid = 0
time = 1661976398
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+0x170
panic() at panic+0x2a
page_fault_handler() at page_fault_handler+0x1d6
do_trap_supervisor() at do_trap_supervisor+0x76
cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x70
--- exception 13, tval = 0x27fffd000809460
vmem_alloc() at vmem_alloc+0x88
kmem_malloc_domain() at kmem_malloc_domain+0x6a
kmem_malloc_domainset() at kmem_malloc_domainset+0x36
malloc_large() at malloc_large+0x44
malloc() at malloc+0xf8
sndstat_unpack_user_nvlbuf() at sndstat_unpack_user_nvlbuf+0x26
sndstat_add_user_devs() at sndstat_add_user_devs+0x4a
sndstat_ioctl() at sndstat_ioctl+0xae
devfs_ioctl() at devfs_ioctl+0xbe
VOP_IOCTL_APV() at VOP_IOCTL_APV+0x30
VOP_IOCTL() at VOP_IOCTL+0x36
vn_ioctl() at vn_ioctl+0xba
fo_ioctl() at fo_ioctl+0xa
kern_ioctl() at kern_ioctl+0x242
sys_ioctl() at sys_ioctl+0x120
syscallenter() at syscallenter+0xec
ecall_handler() at ecall_handler+0x18
do_trap_user() at do_trap_user+0xea
cpu_exception_handler_user() at cpu_exception_handler_user+0x72
devfs_ioctl_f() at devfs_ioctl_f+0x20
Comment 1 Christos Margiolis freebsd_committer freebsd_triage 2024-05-14 13:47:27 UTC
Thanks for the report Robert, I will take this.
Comment 2 Christos Margiolis freebsd_committer freebsd_triage 2024-05-17 21:37:09 UTC
Submitted a fix: https://reviews.freebsd.org/D45236
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-05-20 14:19:22 UTC
A commit in branch main references this bug:

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

commit 074d337ad618f9cc2a1d5ab18b484928e57bd72b
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-05-20 14:18:28 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-05-20 14:18:28 +0000

    sound: Check user-supplied size passed to SNDSTIOC_ADD_USER_DEVS*

    SNDSTIOC_ADD_USER_DEVS* expects a user-supplied sndstioc_nv_arg->nbytes,
    however we currently do not check whether this size is actually valid,
    which results in a panic when SNDSTIOC_ADD_USER_DEVS* is called with an
    invalid size. sndstat_add_user_devs() calls
    sndstat_unpack_user_nvlbuf(), which then calls malloc() with that size.

    PR:             266142
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 day
    Reviewed by:    brooks
    Differential Revision:  https://reviews.freebsd.org/D45236

 sys/dev/sound/pcm/sndstat.c | 5 +++++
 sys/sys/sndstat.h           | 5 +++++
 2 files changed, 10 insertions(+)
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-05-21 17:54:12 UTC
A commit in branch stable/14 references this bug:

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

commit 5830a00c2c5485ec17900558e4f29c459c6a1f3e
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-05-20 14:18:28 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-05-21 17:45:49 +0000

    sound: Check user-supplied size passed to SNDSTIOC_ADD_USER_DEVS*

    SNDSTIOC_ADD_USER_DEVS* expects a user-supplied sndstioc_nv_arg->nbytes,
    however we currently do not check whether this size is actually valid,
    which results in a panic when SNDSTIOC_ADD_USER_DEVS* is called with an
    invalid size. sndstat_add_user_devs() calls
    sndstat_unpack_user_nvlbuf(), which then calls malloc() with that size.

    PR:             266142
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 day
    Reviewed by:    brooks
    Differential Revision:  https://reviews.freebsd.org/D45236

    (cherry picked from commit 074d337ad618f9cc2a1d5ab18b484928e57bd72b)

 sys/dev/sound/pcm/sndstat.c | 5 +++++
 sys/sys/sndstat.h           | 5 +++++
 2 files changed, 10 insertions(+)
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-05-22 13:24:18 UTC
A commit in branch releng/14.1 references this bug:

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

commit 18f80d6d463495703f9eb2ccfd92bae93c7889a3
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-05-20 14:18:28 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-05-22 13:22:26 +0000

    sound: Check user-supplied size passed to SNDSTIOC_ADD_USER_DEVS*

    SNDSTIOC_ADD_USER_DEVS* expects a user-supplied sndstioc_nv_arg->nbytes,
    however we currently do not check whether this size is actually valid,
    which results in a panic when SNDSTIOC_ADD_USER_DEVS* is called with an
    invalid size. sndstat_add_user_devs() calls
    sndstat_unpack_user_nvlbuf(), which then calls malloc() with that size.

    PR:             266142
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 day
    Reviewed by:    brooks
    Differential Revision:  https://reviews.freebsd.org/D45236

    (cherry picked from commit 074d337ad618f9cc2a1d5ab18b484928e57bd72b)
    (cherry picked from commit 5830a00c2c5485ec17900558e4f29c459c6a1f3e)

    Approved by:    re (cperciva)

 sys/dev/sound/pcm/sndstat.c | 5 +++++
 sys/sys/sndstat.h           | 5 +++++
 2 files changed, 10 insertions(+)