Re: [PATCH 3/3] hw/display/artist: rewrite vram access mode handling

[Sven Schnelle <svens@stackframe.org>]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> +static void artist_vram_write4(ARTISTState *s, struct vram_buffer *buf,
>> +                               uint32_t offset, uint32_t data)
>
>> +static int get_vram_offset(ARTISTState *s, struct vram_buffer *buf,
>> +                           int pos, int posy)
>
>> +    case 0x13a0:
>> +        artist_vram_write4(s, buf, get_vram_offset(s, buf, pos >> 2, posy),
>> +                           data);
>
> That is asking for trouble.
>
> You should pass around offsets not pointers.  An offset can trivially be
> checked whenever it is within the valid range (i.e. smaller than vram
> size), or it can be masked to strip off high bits when accessing virtual
> vram.  You need that for robustness and security reasons (i.e. make sure
> the guest can't write to host memory by tricking your get_vram_offset
> calculations).

I'm not sure i understand the problem. get_vram_offset() returns an
offset, which is passed to artist_vram_write4() which itself doesn't
do anything on the buffer. artist_rop8() in the end accesses the buffer,
and that function checks whether it's < buf->size. Can you elaborate
a bit more? Maybe it's just so obvious that i don't see it.

Thanks,
Sven