From owner-svn-src-head@freebsd.org Wed Sep 30 05:24:24 2015 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 85615A0C075; Wed, 30 Sep 2015 05:24:24 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 732DF12F0; Wed, 30 Sep 2015 05:24:24 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.70]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id t8U5OOXd097264; Wed, 30 Sep 2015 05:24:24 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id t8U5ONsG097254; Wed, 30 Sep 2015 05:24:23 GMT (envelope-from markj@FreeBSD.org) Message-Id: <201509300524.t8U5ONsG097254@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Wed, 30 Sep 2015 05:24:23 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r288415 - in head: cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/privs cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/scalars sys/cddl/contrib/opensolaris/uts/common/dtrace s... X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Sep 2015 05:24:24 -0000 Author: markj Date: Wed Sep 30 05:24:22 2015 New Revision: 288415 URL: https://svnweb.freebsd.org/changeset/base/288415 Log: MFV r288408: 6266 harden dtrace_difo_chunksize() with respect to malicious DIF illumos/illumos-gate@395c7a3dcfc66b8b671dc4b3c4a2f0ca26449922 Reviewed by: Alex Wilson Reviewed by: Dan McDonald Approved by: Garrett D'Amore Author: Bryan Cantrill MFC after: 1 week Added: head/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh - copied unchanged from r288408, vendor/illumos/dist/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh head/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/scalars/err.bigglobal.d - copied unchanged from r288408, vendor/illumos/dist/cmd/dtrace/test/tst/common/scalars/err.bigglobal.d head/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/scalars/err.biglocal.d - copied unchanged from r288408, vendor/illumos/dist/cmd/dtrace/test/tst/common/scalars/err.biglocal.d Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c head/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h Directory Properties: head/cddl/contrib/opensolaris/ (props changed) head/sys/cddl/contrib/opensolaris/ (props changed) Copied: head/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh (from r288408, vendor/illumos/dist/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh) ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh Wed Sep 30 05:24:22 2015 (r288415, copy of r288408, vendor/illumos/dist/cmd/dtrace/test/tst/common/privs/tst.kpriv.ksh) @@ -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 <mrbig.toomany[0] = '!'; + exit(0); +} Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c Wed Sep 30 05:19:16 2015 (r288414) +++ head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c Wed Sep 30 05:24:22 2015 (r288415) @@ -155,7 +155,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 = 128; @@ -699,13 +699,33 @@ dtrace_canstore_statvar(uint64_t addr, s 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); } @@ -4455,7 +4475,8 @@ dtrace_dif_subr(uint_t subr, uint_t rd, if (!dtrace_destructive_disallow && dtrace_priv_proc_control(state) && - !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); @@ -4470,7 +4491,8 @@ dtrace_dif_subr(uint_t subr, uint_t rd, if (!dtrace_destructive_disallow && dtrace_priv_proc_control(state) && - !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); @@ -6458,6 +6480,11 @@ dtrace_dif_emulate(dtrace_difo_t *difo, 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]; } @@ -9915,9 +9942,10 @@ dtrace_difo_validate(dtrace_difo_t *dp, 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; } } @@ -10261,6 +10289,9 @@ dtrace_difo_chunksize(dtrace_difo_t *dp, if (srd == 0) return; + if (sval > LONG_MAX) + return; + tupregs[ttop++].dttk_size = sval; } @@ -10322,6 +10353,19 @@ dtrace_difo_chunksize(dtrace_difo_t *dp, */ 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; } @@ -13942,6 +13986,8 @@ dtrace_dstate_init(dtrace_dstate_t *dsta 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; @@ -13982,6 +14028,9 @@ dtrace_dstate_init(dtrace_dstate_t *dsta ((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; @@ -14007,7 +14056,7 @@ dtrace_dstate_init(dtrace_dstate_t *dsta 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 + @@ -14016,6 +14065,8 @@ dtrace_dstate_init(dtrace_dstate_t *dsta 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; } Modified: head/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h Wed Sep 30 05:19:16 2015 (r288414) +++ head/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h Wed Sep 30 05:24:22 2015 (r288415) @@ -1317,16 +1317,19 @@ extern void dtrace_copystr(uintptr_t, ui /* * 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__)))