[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] hw/display/artist: rewrite vram access mode handling
From: |
Sven Schnelle |
Subject: |
Re: [PATCH 3/3] hw/display/artist: rewrite vram access mode handling |
Date: |
Tue, 25 Jan 2022 17:29:53 +0100 |
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