From patchwork Thu Jul 6 09:20:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 9827795 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 97EAC60361 for ; Thu, 6 Jul 2017 09:23:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 875E6285DA for ; Thu, 6 Jul 2017 09:23:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7B823285FC; Thu, 6 Jul 2017 09:23:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8F347285DA for ; Thu, 6 Jul 2017 09:23:32 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dT2xy-0000h2-Ot; Thu, 06 Jul 2017 09:20:54 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dT2xx-0000gw-QK for xen-devel@lists.xenproject.org; Thu, 06 Jul 2017 09:20:53 +0000 Received: from [85.158.139.211] by server-9.bemta-5.messagelabs.com id 87/EA-01994-4F00E595; Thu, 06 Jul 2017 09:20:52 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrCIsWRWlGSWpSXmKPExsXS6fjDS/cLQ1y kwbqLwhbft0xmcmD0OPzhCksAYxRrZl5SfkUCa8bnt3tZCqa5Vxza+5+pgXGRcRcjJ4eQQJ7E vImL2EBsXgE7ifkPPjOC2BIChhKnF95kAbFZBFQlnr57zwxiswmoS7Q9287axcjBISJgIHHua BKIySwQL3F1nT1IhbCAg8SCCV+ZIKbbSUyd9JIZpIRXQFDi7w5hkDCzgJbEw1+3WCBsbYllC1 8zQ0yRllj+j2MCI+8shIZZSBpmIWmYhdCwgJFlFaN6cWpRWWqRroleUlFmekZJbmJmjq6hgal ebmpxcWJ6ak5iUrFecn7uJkZgeDEAwQ7GW33OhxglOZiURHnFD8dGCvEl5adUZiQWZ8QXleak Fh9i1ODgENi2a/UFRimWvPy8VCUJ3tL/QHWCRanpqRVpmTnACIApleDgURLhjfkBlOYtLkjML c5Mh0idYjTm2LB6/RcmjlcT/n9jEgKbJCXO6w0ySQCkNKM0D24QLDIvMcpKCfMyAp0pxFOQWp SbWYIq/4pRnINRSZj3M8gUnsy8Erh9r4BOYQI6RbExBuSUkkSElFQDI7NZZaCYp3H2/fcpcpU Cj8SN8h+mpb1V/lX4q1m473DL1qNnrTJbNzyIMN2wbtKJjlN2Kiw/ni3KdrD6eGrDsmcSPS// P08PMyxr7XX5J2x/jz/zC4+Q6a/8bnelh0YzSkMys62DUkR1fzdaPctl+OR1fHv+/d9nPG68z jZ3k3esPTK9ctp9JZbijERDLeai4kQASSS3JscCAAA= X-Env-Sender: JBeulich@suse.com X-Msg-Ref: server-4.tower-206.messagelabs.com!1499332850!101632786!1 X-Originating-IP: [137.65.248.74] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.25; banners=-,-,- X-VirusChecked: Checked Received: (qmail 39045 invoked from network); 6 Jul 2017 09:20:51 -0000 Received: from prv-mh.provo.novell.com (HELO prv-mh.provo.novell.com) (137.65.248.74) by server-4.tower-206.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 6 Jul 2017 09:20:51 -0000 Received: from INET-PRV-MTA by prv-mh.provo.novell.com with Novell_GroupWise; Thu, 06 Jul 2017 03:20:49 -0600 Message-Id: <595E1D100200007800169103@prv-mh.provo.novell.com> X-Mailer: Novell GroupWise Internet Agent 14.2.2 Date: Thu, 06 Jul 2017 03:20:48 -0600 From: "Jan Beulich" To: "xen-devel" References: <595E1D100200007800169103@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Disposition: inline Cc: George Dunlap , Andrew Cooper Subject: [Xen-devel] [PATCH v3] x86emul/fuzz: add rudimentary limit checking X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP fuzz_insn_fetch() is the only data access helper where it is possible to see offsets larger than 4Gb in 16- or 32-bit modes, as we leave the incoming rIP untouched in the emulator itself. The check is needed here as otherwise, after successfully fetching insn bytes, we may end up zero-extending EIP soon after complete_insn, which collides with the X86EMUL_EXCEPTION-conditional respective ASSERT() in x86_emulate_wrapper(). (NB: put_rep_prefix() is what allows complete_insn to be reached with rc set to other than X86EMUL_OKAY or X86EMUL_DONE. See also commit 53f87c03b4 ["x86emul: generalize exception handling for rep_* hooks"].) Add assert()-s for all other (data) access routines, as effective address generation in the emulator ought to guarantee in-range values. For them to not trigger, several adjustments to the emulator's address calculations are needed: While for DstBitBase it is really mandatory, the specification allows for either behavior for two-part accesses. Observed behavior on real hardware, however, is for such accesses to silently wrap at the 2^^32 boundary in other than 64-bit mode, just like they do at the 2^^64 boundary in 64-bit mode. While adding truncate_ea() invocations there, also convert open coded instances of it. Reported-by: George Dunlap Signed-off-by: Jan Beulich --- v3: Add more truncate_ea(). v2: Correct system segment related assert()-s. --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c @@ -139,7 +139,17 @@ static int fuzz_read( struct x86_emulate_ctxt *ctxt) { /* Reads expected for all user and system segments. */ - assert(is_x86_user_segment(seg) || is_x86_system_segment(seg)); + if ( is_x86_user_segment(seg) ) + assert(ctxt->addr_size == 64 || !(offset >> 32)); + else if ( seg == x86_seg_tr ) + /* + * The TSS is special in that accesses below the segment base are + * possible, as the Interrupt Redirection Bitmap starts 32 bytes + * ahead of the I/O Bitmap, regardless of the value of the latter. + */ + assert((long)offset < 0 ? (long)offset > -32 : !(offset >> 17)); + else + assert(is_x86_system_segment(seg) && !(offset >> 16)); return data_read(ctxt, seg, "read", p_data, bytes); } @@ -162,6 +172,13 @@ static int fuzz_insn_fetch( { assert(seg == x86_seg_cs); + /* Minimal segment limit checking, until full one is being put in place. */ + if ( ctxt->addr_size < 64 && (offset >> 32) ) + { + x86_emul_hw_exception(13, 0, ctxt); + return X86EMUL_EXCEPTION; + } + /* * Zero-length instruction fetches are made at the destination of jumps, * to perform segmentation checks. No data needs returning. @@ -232,6 +249,7 @@ static int fuzz_rep_ins( struct x86_emulate_ctxt *ctxt) { assert(dst_seg == x86_seg_es); + assert(ctxt->addr_size == 64 || !(dst_offset >> 32)); return _fuzz_rep_read(ctxt, "rep_ins", reps); } @@ -247,6 +265,7 @@ static int fuzz_rep_movs( { assert(is_x86_user_segment(src_seg)); assert(dst_seg == x86_seg_es); + assert(ctxt->addr_size == 64 || !((src_offset | dst_offset) >> 32)); return _fuzz_rep_read(ctxt, "rep_movs", reps); } @@ -260,6 +279,7 @@ static int fuzz_rep_outs( struct x86_emulate_ctxt *ctxt) { assert(is_x86_user_segment(src_seg)); + assert(ctxt->addr_size == 64 || !(src_offset >> 32)); return _fuzz_rep_write(ctxt, "rep_outs", reps); } @@ -277,6 +297,7 @@ static int fuzz_rep_stos( * for CLZERO. */ assert(is_x86_user_segment(seg)); + assert(ctxt->addr_size == 64 || !(offset >> 32)); return _fuzz_rep_write(ctxt, "rep_stos", reps); } @@ -290,6 +311,7 @@ static int fuzz_write( { /* Writes not expected for any system segments. */ assert(is_x86_user_segment(seg)); + assert(ctxt->addr_size == 64 || !(offset >> 32)); return maybe_fail(ctxt, "write", true); } @@ -306,8 +328,10 @@ static int fuzz_cmpxchg( * Cmpxchg expected for user segments, and setting accessed/busy bits in * GDT/LDT enties, but not expected for any IDT or TR accesses. */ - assert(is_x86_user_segment(seg) || - seg == x86_seg_gdtr || seg == x86_seg_ldtr); + if ( is_x86_user_segment(seg) ) + assert(ctxt->addr_size == 64 || !(offset >> 32)); + else + assert((seg == x86_seg_gdtr || seg == x86_seg_ldtr) && !(offset >> 16)); return maybe_fail(ctxt, "cmpxchg", true); } @@ -319,6 +343,7 @@ static int fuzz_invlpg( { /* invlpg(), unlike all other hooks, may be called with x86_seg_none. */ assert(is_x86_user_segment(seg) || seg == x86_seg_none); + assert(ctxt->addr_size == 64 || !(offset >> 32)); return maybe_fail(ctxt, "invlpg", false); } --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1249,10 +1249,10 @@ static void __put_rep_prefix( /* Clip maximum repetitions so that the index register at most just wraps. */ #define truncate_ea_and_reps(ea, reps, bytes_per_rep) ({ \ - unsigned long todo__, ea__ = truncate_word(ea, ad_bytes); \ + unsigned long todo__, ea__ = truncate_ea(ea); \ if ( !(_regs.eflags & X86_EFLAGS_DF) ) \ - todo__ = truncate_word(-(ea), ad_bytes) / (bytes_per_rep); \ - else if ( truncate_word((ea) + (bytes_per_rep) - 1, ad_bytes) < ea__ )\ + todo__ = truncate_ea(-ea__) / (bytes_per_rep); \ + else if ( truncate_ea(ea__ + (bytes_per_rep) - 1) < ea__ ) \ todo__ = 1; \ else \ todo__ = ea__ / (bytes_per_rep) + 1; \ @@ -3128,6 +3128,7 @@ x86_emulate( op_bytes + (((-src.val - 1) >> 3) & ~(op_bytes - 1L)); else ea.mem.off += (src.val >> 3) & ~(op_bytes - 1L); + ea.mem.off = truncate_ea(ea.mem.off); } /* Bit index always truncated to within range. */ @@ -3346,7 +3347,7 @@ x86_emulate( unsigned long src_val2; int lb, ub, idx; generate_exception_if(src.type != OP_MEM, EXC_UD); - if ( (rc = read_ulong(src.mem.seg, src.mem.off + op_bytes, + if ( (rc = read_ulong(src.mem.seg, truncate_ea(src.mem.off + op_bytes), &src_val2, op_bytes, ctxt, ops)) ) goto done; ub = (op_bytes == 2) ? (int16_t)src_val2 : (int32_t)src_val2; @@ -3897,7 +3898,7 @@ x86_emulate( seg = (b & 1) * 3; /* es = 0, ds = 3 */ les: generate_exception_if(src.type != OP_MEM, EXC_UD); - if ( (rc = read_ulong(src.mem.seg, src.mem.off + src.bytes, + if ( (rc = read_ulong(src.mem.seg, truncate_ea(src.mem.off + src.bytes), &dst.val, 2, ctxt, ops)) != X86EMUL_OKAY ) goto done; ASSERT(is_x86_user_segment(seg)); @@ -4931,7 +4932,8 @@ x86_emulate( case 5: /* jmp (far, absolute indirect) */ generate_exception_if(src.type != OP_MEM, EXC_UD); - if ( (rc = read_ulong(src.mem.seg, src.mem.off + op_bytes, + if ( (rc = read_ulong(src.mem.seg, + truncate_ea(src.mem.off + op_bytes), &imm2, 2, ctxt, ops)) ) goto done; imm1 = src.val; @@ -5115,8 +5117,8 @@ x86_emulate( } if ( (rc = ops->write(ea.mem.seg, ea.mem.off, &sreg.limit, 2, ctxt)) != X86EMUL_OKAY || - (rc = ops->write(ea.mem.seg, ea.mem.off + 2, &sreg.base, - op_bytes, ctxt)) != X86EMUL_OKAY ) + (rc = ops->write(ea.mem.seg, truncate_ea(ea.mem.off + 2), + &sreg.base, op_bytes, ctxt)) != X86EMUL_OKAY ) goto done; break; case 2: /* lgdt */ @@ -5125,9 +5127,9 @@ x86_emulate( generate_exception_if(ea.type != OP_MEM, EXC_UD); fail_if(ops->write_segment == NULL); memset(&sreg, 0, sizeof(sreg)); - if ( (rc = read_ulong(ea.mem.seg, ea.mem.off+0, + if ( (rc = read_ulong(ea.mem.seg, ea.mem.off, &limit, 2, ctxt, ops)) || - (rc = read_ulong(ea.mem.seg, ea.mem.off+2, + (rc = read_ulong(ea.mem.seg, truncate_ea(ea.mem.off + 2), &base, mode_64bit() ? 8 : 4, ctxt, ops)) ) goto done; generate_exception_if(!is_canonical_address(base), EXC_GP, 0);