On Thu, Mar 23, 2017 at 1:37 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
>
>
> On 23/03/2017 17:50, Pranith Kumar wrote:
>> On Thu, Mar 23, 2017 at 6:27 AM, Paolo Bonzini <pbonz...@redhat.com> wrote:
>>>
>>>
>>> On 22/03/2017 21:01, Richard Henderson wrote:
>>>>>
>>>>> Ah, OK. Thanks for the explanation. May be we should check the size of
>>>>> the instruction while decoding the prefixes and error out once we
>>>>> exceed the limit. We would not generate any IR code.
>>>>
>>>> Yes.
>>>>
>>>> It would not enforce a true limit of 15 bytes, since you can't know that
>>>> until you've done the rest of the decode. But you'd be able to say that
>>>> no more than 14 prefix + 1 opc + 6 modrm+sib+ofs + 4 immediate = 25
>>>> bytes is used.
>>>>
>>>> Which does fix the bug.
>>>
>>> Yeah, that would work for 2.9 if somebody wants to put together a patch.
>>> Ensuring that all instruction fetching happens before translation side
>>> effects is a little harder, but perhaps it's also the opportunity to get
>>> rid of s->rip_offset which is a little ugly.
>>
>> How about the following?
>>
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 72c1b03a2a..67c58b8900 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -4418,6 +4418,11 @@ static target_ulong disas_insn(CPUX86State
>> *env, DisasContext *s,
>> s->vex_l = 0;
>> s->vex_v = 0;
>> next_byte:
>> + /* The prefixes can atmost be 14 bytes since x86 has an upper
>> + limit of 15 bytes for the instruction */
>> + if (s->pc - pc_start > 14) {
>> + goto illegal_op;
>> + }
>> b = cpu_ldub_code(env, s->pc);
>> s->pc++;
>> /* Collect prefixes. */
>
> Please make the comment more verbose, based on Richard's remark. We
> should apply it to 2.9.
>
> Also, QEMU usually formats comments with stars on every line.
OK. I'll send a proper patch with updated comment.
Thanks,
--
Pranith
