Re: [Xen-devel] [PATCH v3] x86emul/fuzz: add rudimentary limit checking

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.07.17 at 12:57, <george.dunlap@xxxxxxxxxx> wrote:
> On 07/06/2017 10:20 AM, Jan Beulich wrote:
>> 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 <george.dunlap@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> v3: Add more truncate_ea().
>> v2: Correct system segment related assert()-s.
> 
> Still getting crashes in protmode_load_seg(), line 1824.  (See attached
> for an example stack trace; but basically any place that calls
> protmode_load_seg()).

Ah, this is one I indeed forgot about. We shouldn't deal with this in
the emulator though, so slightly relaxing the assert() seems like the
only option: We'd need to permit reads up to 0x10007 instead of
0xffff (which would never pass limit checks).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel