Skip to content

Commit

Permalink
6266 harden dtrace_difo_chunksize() with respect to malicious DIF
Browse files Browse the repository at this point in the history
Reviewed by: Alex Wilson <alex.wilson@joyent.com>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Approved by: Garrett D'Amore <garrett@damore.org>
  • Loading branch information
bcantrill authored and rmustacc committed Sep 29, 2015
1 parent 6a9ca70 commit 395c7a3
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 16 deletions.
112 changes: 112 additions & 0 deletions usr/src/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
#
# This file and its contents are supplied under the terms of the
# Common Development and Distribution License ("CDDL"), version 1.0.
# You may only use this file in accordance with the terms of version
# 1.0 of the CDDL.
#
# A full copy of the text of the CDDL should have accompanied this
# source. A copy of the CDDL is also available via the Internet at
# http://www.illumos.org/license/CDDL.
#

#
# Copyright (c) 2015, Joyent, Inc. All rights reserved.
#

err=/tmp/err.$$

ppriv -s A=basic,dtrace_user $$

#
# When we lack dtrace_kernel, we expect to not be able to get at kernel memory
# via any subroutine or other vector.
#
# trace(func((void *)&\`utsname)); }
/usr/sbin/dtrace -wq -Cs /dev/stdin 2> $err <<EOF
#define FAIL \
printf("able to read kernel memory via %s!\n", badsubr); \
exit(2);
#define CANTREAD1(func) \
BEGIN { badsubr = "func()"; func((void *)&\`utsname); FAIL }
#define CANTREAD2(func, arg1) \
BEGIN { badsubr = "func()"; func((void *)&\`utsname, arg1); FAIL }
#define CANTREAD2ARG1(func, arg0) \
BEGIN { badsubr = "func() (arg1)"; func(arg0, (void *)&\`utsname); FAIL }
#define CANTREAD3(func, arg1, arg2) \
BEGIN { badsubr = "func()"; func((void *)&\`utsname, arg1, arg2); FAIL }
CANTREAD1(mutex_owned)
CANTREAD1(mutex_owner)
CANTREAD1(mutex_type_adaptive)
CANTREAD1(mutex_type_spin)
CANTREAD1(rw_read_held)
CANTREAD1(rw_write_held)
CANTREAD1(rw_iswriter)
CANTREAD3(bcopy, alloca(1), 1)
CANTREAD1(msgsize)
CANTREAD1(msgdsize)
CANTREAD1(strlen)
CANTREAD2(strchr, '!')
CANTREAD2(strrchr, '!')
CANTREAD2(strstr, "doogle")
CANTREAD2ARG1(strstr, "doogle")
CANTREAD2(index, "bagnoogle")
CANTREAD2ARG1(index, "bagnoogle")
CANTREAD2(rindex, "bagnoogle")
CANTREAD2ARG1(rindex, "bagnoogle")
CANTREAD2(strtok, "doogle")
CANTREAD2ARG1(strtok, "doogle")
CANTREAD2(json, "doogle")
CANTREAD2ARG1(json, "doogle")
CANTREAD1(toupper)
CANTREAD1(tolower)
CANTREAD2(ddi_pathname, 1)
CANTREAD2(strjoin, "doogle")
CANTREAD2ARG1(strjoin, "doogle")
CANTREAD1(strtoll)
CANTREAD1(dirname)
CANTREAD1(basename)
CANTREAD1(cleanpath)
#if defined(__amd64)
CANTREAD3(copyout, uregs[R_R9], 1)
CANTREAD3(copyoutstr, uregs[R_R9], 1)
#else
#if defined(__i386)
CANTREAD3(copyout, uregs[R_ESP], 1)
CANTREAD3(copyoutstr, uregs[R_ESP], 1)
#endif
#endif
BEGIN
{
exit(0);
}
ERROR
/arg4 != DTRACEFLT_KPRIV/
{
printf("bad error code via %s (expected %d, found %d)\n",
badsubr, DTRACEFLT_KPRIV, arg4);
exit(3);
}
ERROR
/arg4 == DTRACEFLT_KPRIV/
{
printf("illegal kernel access properly prevented from %s\n", badsubr);
}
EOF

status=$?

if [[ $status -eq 1 ]]; then
cat $err
fi

exit $status
26 changes: 26 additions & 0 deletions usr/src/cmd/dtrace/test/tst/common/scalars/err.bigglobal.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* This file and its contents are supplied under the terms of the
* Common Development and Distribution License ("CDDL"), version 1.0.
* You may only use this file in accordance with the terms of version
* 1.0 of the CDDL.
*
* A full copy of the text of the CDDL should have accompanied this
* source. A copy of the CDDL is also available via the Internet at
* http://www.illumos.org/license/CDDL.
*/

/*
* Copyright (c) 2015, Joyent, Inc. All rights reserved.
*/

struct mrbig {
char toomany[100000];
};

struct mrbig mrbig;

BEGIN
{
mrbig.toomany[0] = '!';
exit(0);
}
26 changes: 26 additions & 0 deletions usr/src/cmd/dtrace/test/tst/common/scalars/err.biglocal.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* This file and its contents are supplied under the terms of the
* Common Development and Distribution License ("CDDL"), version 1.0.
* You may only use this file in accordance with the terms of version
* 1.0 of the CDDL.
*
* A full copy of the text of the CDDL should have accompanied this
* source. A copy of the CDDL is also available via the Internet at
* http://www.illumos.org/license/CDDL.
*/

/*
* Copyright (c) 2015, Joyent, Inc. All rights reserved.
*/

struct mrbig {
char toomany[100000];
};

this struct mrbig mrbig;

BEGIN
{
this->mrbig.toomany[0] = '!';
exit(0);
}
3 changes: 3 additions & 0 deletions usr/src/pkg/manifests/system-dtrace-tests.mf
Original file line number Diff line number Diff line change
Expand Up @@ -1393,6 +1393,7 @@ file path=opt/SUNWdtrt/tst/common/printf/tst.wp.d.out mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.fds.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.func_access.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.getf.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.kpriv.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.noprivdrop.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.noprivrestrict.ksh mode=0444
file path=opt/SUNWdtrt/tst/common/privs/tst.op_access.ksh mode=0444
Expand Down Expand Up @@ -1572,6 +1573,8 @@ file path=opt/SUNWdtrt/tst/common/scalars/err.D_OP_INCOMPAT.dupltype.d \
file path=opt/SUNWdtrt/tst/common/scalars/err.D_OP_INCOMPAT.dupttype.d \
mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/err.D_SYNTAX.declare.d mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/err.bigglobal.d mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/err.biglocal.d mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/tst.basicvar.d mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/tst.basicvar.d.out mode=0444
file path=opt/SUNWdtrt/tst/common/scalars/tst.localvar.d mode=0444
Expand Down
67 changes: 59 additions & 8 deletions usr/src/uts/common/dtrace/dtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ int dtrace_destructive_disallow = 0;
dtrace_optval_t dtrace_nonroot_maxsize = (16 * 1024 * 1024);
size_t dtrace_difo_maxsize = (256 * 1024);
dtrace_optval_t dtrace_dof_maxsize = (8 * 1024 * 1024);
size_t dtrace_global_maxsize = (16 * 1024);
size_t dtrace_statvar_maxsize = (16 * 1024);
size_t dtrace_actions_max = (16 * 1024);
size_t dtrace_retain_max = 1024;
dtrace_optval_t dtrace_helper_actions_max = 1024;
Expand Down Expand Up @@ -592,13 +592,33 @@ dtrace_canstore_statvar(uint64_t addr, size_t sz,
dtrace_statvar_t **svars, int nsvars)
{
int i;
size_t maxglobalsize, maxlocalsize;

if (nsvars == 0)
return (0);

maxglobalsize = dtrace_statvar_maxsize;
maxlocalsize = (maxglobalsize + sizeof (uint64_t)) * NCPU;

for (i = 0; i < nsvars; i++) {
dtrace_statvar_t *svar = svars[i];
uint8_t scope;
size_t size;

if (svar == NULL || svar->dtsv_size == 0)
if (svar == NULL || (size = svar->dtsv_size) == 0)
continue;

scope = svar->dtsv_var.dtdv_scope;

/*
* We verify that our size is valid in the spirit of providing
* defense in depth: we want to prevent attackers from using
* DTrace to escalate an orthogonal kernel heap corruption bug
* into the ability to store to arbitrary locations in memory.
*/
VERIFY((scope == DIFV_SCOPE_GLOBAL && size < maxglobalsize) ||
(scope == DIFV_SCOPE_LOCAL && size < maxlocalsize));

if (DTRACE_INRANGE(addr, sz, svar->dtsv_data, svar->dtsv_size))
return (1);
}
Expand Down Expand Up @@ -4165,7 +4185,8 @@ dtrace_dif_subr(uint_t subr, uint_t rd, uint64_t *regs,

if (!dtrace_destructive_disallow &&
dtrace_priv_proc_control(state, mstate) &&
!dtrace_istoxic(kaddr, size)) {
!dtrace_istoxic(kaddr, size) &&
dtrace_canload(kaddr, size, mstate, vstate)) {
DTRACE_CPUFLAG_SET(CPU_DTRACE_NOFAULT);
dtrace_copyout(kaddr, uaddr, size, flags);
DTRACE_CPUFLAG_CLEAR(CPU_DTRACE_NOFAULT);
Expand All @@ -4180,7 +4201,8 @@ dtrace_dif_subr(uint_t subr, uint_t rd, uint64_t *regs,

if (!dtrace_destructive_disallow &&
dtrace_priv_proc_control(state, mstate) &&
!dtrace_istoxic(kaddr, size)) {
!dtrace_istoxic(kaddr, size) &&
dtrace_strcanload(kaddr, size, mstate, vstate)) {
DTRACE_CPUFLAG_SET(CPU_DTRACE_NOFAULT);
dtrace_copyoutstr(kaddr, uaddr, size, flags);
DTRACE_CPUFLAG_CLEAR(CPU_DTRACE_NOFAULT);
Expand Down Expand Up @@ -6078,6 +6100,11 @@ dtrace_dif_emulate(dtrace_difo_t *difo, dtrace_mstate_t *mstate,
regs[r2] ? regs[r2] :
dtrace_strsize_default) + 1;
} else {
if (regs[r2] > LONG_MAX) {
*flags |= CPU_DTRACE_ILLOP;
break;
}

tupregs[ttop].dttk_size = regs[r2];
}

Expand Down Expand Up @@ -9344,9 +9371,10 @@ dtrace_difo_validate(dtrace_difo_t *dp, dtrace_vstate_t *vstate, uint_t nregs,
break;
}

if (v->dtdv_scope == DIFV_SCOPE_GLOBAL &&
vt->dtdt_size > dtrace_global_maxsize) {
err += efunc(i, "oversized by-ref global\n");
if ((v->dtdv_scope == DIFV_SCOPE_GLOBAL ||
v->dtdv_scope == DIFV_SCOPE_LOCAL) &&
vt->dtdt_size > dtrace_statvar_maxsize) {
err += efunc(i, "oversized by-ref static\n");
break;
}
}
Expand Down Expand Up @@ -9683,6 +9711,9 @@ dtrace_difo_chunksize(dtrace_difo_t *dp, dtrace_vstate_t *vstate)
if (srd == 0)
return;

if (sval > LONG_MAX)
return;

tupregs[ttop++].dttk_size = sval;
}

Expand Down Expand Up @@ -9744,6 +9775,19 @@ dtrace_difo_chunksize(dtrace_difo_t *dp, dtrace_vstate_t *vstate)
*/
size = P2ROUNDUP(size, sizeof (uint64_t));

/*
* Before setting the chunk size, check that we're not going
* to set it to a negative value...
*/
if (size > LONG_MAX)
return;

/*
* ...and make certain that we didn't badly overflow.
*/
if (size < ksize || size < sizeof (dtrace_dynvar_t))
return;

if (size > vstate->dtvs_dynvars.dtds_chunksize)
vstate->dtvs_dynvars.dtds_chunksize = size;
}
Expand Down Expand Up @@ -13184,6 +13228,8 @@ dtrace_dstate_init(dtrace_dstate_t *dstate, size_t size)
if ((dstate->dtds_chunksize = chunksize) == 0)
dstate->dtds_chunksize = DTRACE_DYNVAR_CHUNKSIZE;

VERIFY(dstate->dtds_chunksize < LONG_MAX);

if (size < (min = dstate->dtds_chunksize + sizeof (dtrace_dynhash_t)))
size = min;

Expand Down Expand Up @@ -13224,6 +13270,9 @@ dtrace_dstate_init(dtrace_dstate_t *dstate, size_t size)
((uintptr_t)base + hashsize * sizeof (dtrace_dynhash_t));
limit = (uintptr_t)base + size;

VERIFY((uintptr_t)start < limit);
VERIFY((uintptr_t)start >= (uintptr_t)base);

maxper = (limit - (uintptr_t)start) / NCPU;
maxper = (maxper / dstate->dtds_chunksize) * dstate->dtds_chunksize;

Expand All @@ -13245,7 +13294,7 @@ dtrace_dstate_init(dtrace_dstate_t *dstate, size_t size)
start = (dtrace_dynvar_t *)limit;
}

ASSERT(limit <= (uintptr_t)base + size);
VERIFY(limit <= (uintptr_t)base + size);

for (;;) {
next = (dtrace_dynvar_t *)((uintptr_t)dvar +
Expand All @@ -13254,6 +13303,8 @@ dtrace_dstate_init(dtrace_dstate_t *dstate, size_t size)
if ((uintptr_t)next + dstate->dtds_chunksize >= limit)
break;

VERIFY((uintptr_t)dvar >= (uintptr_t)base &&
(uintptr_t)dvar <= (uintptr_t)base + size);
dvar->dtdv_next = next;
dvar = next;
}
Expand Down
19 changes: 11 additions & 8 deletions usr/src/uts/common/sys/dtrace_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1290,16 +1290,19 @@ extern void dtrace_copystr(uintptr_t, uintptr_t, size_t, volatile uint16_t *);
/*
* DTrace Assertions
*
* DTrace calls ASSERT from probe context. To assure that a failed ASSERT
* does not induce a markedly more catastrophic failure (e.g., one from which
* a dump cannot be gleaned), DTrace must define its own ASSERT to be one that
* may safely be called from probe context. This header file must thus be
* included by any DTrace component that calls ASSERT from probe context, and
* _only_ by those components. (The only exception to this is kernel
* debugging infrastructure at user-level that doesn't depend on calling
* ASSERT.)
* DTrace calls ASSERT and VERIFY from probe context. To assure that a failed
* ASSERT or VERIFY does not induce a markedly more catastrophic failure (e.g.,
* one from which a dump cannot be gleaned), DTrace must define its own ASSERT
* and VERIFY macros to be ones that may safely be called from probe context.
* This header file must thus be included by any DTrace component that calls
* ASSERT and/or VERIFY from probe context, and _only_ by those components.
* (The only exception to this is kernel debugging infrastructure at user-level
* that doesn't depend on calling ASSERT.)
*/
#undef ASSERT
#undef VERIFY
#define VERIFY(EX) ((void)((EX) || \
dtrace_assfail(#EX, __FILE__, __LINE__)))
#ifdef DEBUG
#define ASSERT(EX) ((void)((EX) || \
dtrace_assfail(#EX, __FILE__, __LINE__)))
Expand Down

0 comments on commit 395c7a3

Please sign in to comment.