Re: [Xen-devel] [PATCH v2 01/13] x86emul/fuzz: add rudimentary limit checking

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.10.17 at 17:21, <george.dunlap@xxxxxxxxxx> wrote:
> On Mon, Sep 25, 2017 at 3:26 PM, George Dunlap <george.dunlap@xxxxxxxxxx> 
> wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> 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.
> 
> Something seems to be missing here -- what's mandatory for DstBitBase,
> and what are the two  behaviors allowed by the specification for
> two-part accesses?

"it" in the sentence above refers to "adjustments" (for DstBitBase
it's just one adjustment, so the singular/plural mismatch is not
really helpful). Similarly "either behavior" refers to the code with
and without the changes done. Would

"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 the DstBitBase one is really mandatory,
 the specification allows for either original or new 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, which our
 code is now being brought in line with. While adding truncate_ea()
 invocations there, also convert open coded instances of it."

be any better?

Jan


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