Re: [Qemu-devel] [BUG??] ARM CP accesses rely on guest matching host data types

[Peter Crosthwaite]

Ok so read a bit further,

I was assuming MRC and MRRC were able to access the same register
subset but it turns out that the code is enforcing mutually
exclusivity of two sets via the ENCODE_CP_REG hash table. Sorry for
the noise.

Regards,
Peter

On Thu, Dec 19, 2013 at 3:17 PM, Peter Crosthwaite
<address@hidden> wrote:
> Hi Peter,
>
> I've been reviewing the ARM CP TCG handing code trying to get my head
> around it, and I found this a little suspicious (translate.c):
>
> 6476 static int disas_coproc_insn(CPUARMState * env, DisasContext *s,
> uint32_t insn)
> 6477 {
> ...
> 6502     is64 = (insn & (1 << 25)) == 0;
> ...
> 6552             if (is64) {
> 6553                 TCGv_i64 tmp64;
> 6554                 TCGv_i32 tmp;
> 6555                 if (ri->type & ARM_CP_CONST) {
> ...
> 6557                 } else if (ri->readfn) {
> ...
> 6564                 } else {
> 6565                     tmp64 = tcg_temp_new_i64();
> 6566                     tcg_gen_ld_i64(tmp64, cpu_env, ri->fieldoffset);
> 6567                 }
> ...
> 6577                 TCGv_i32 tmp;
> 6578                 if (ri->type & ARM_CP_CONST) {
> ...
> 6580                 } else if (ri->readfn) {
> ...
> 6587                 } else {
> 6588                     tmp = load_cpu_offset(ri->fieldoffset);
> 6589                 }
>
> Depending on "is64" you will either get a tcg_gen_ld_i64 or gen_ld_i32
> (via load_cpu_offset()) of fieldoffset. The values pointed to by
> ->fieldoffset are however a heterogenous collection of uint32_t and
> uint64_t, which is not checked by this code. This means the guest
> could do a 64bit read of a 32bit reg and we would leak nearby info
> from the CPU env to the guest.
>
> Should we be explicitly defining CPs as 32/64 in ri->type and
> enforcing the correct tcg_gen_ldXX here accordingly?
>
> Regards,
> Peter