Re: [PATCH v3 7/8] hw/display/artist: Refactor artist_rop8() to avoid buffer over-run

[Alexander Bulekov]

On 200809 0717, Helge Deller wrote:
> Hello Alexander,
> 
> On 06.08.20 17:46, Alexander Bulekov wrote:
> > On 200805 2244, Helge Deller wrote:
> >> * Alexander Bulekov <alxndr@bu.edu>:
> >>> On 200804 2320, Helge Deller wrote:
> >>>> * Alexander Bulekov <alxndr@bu.edu>:
> >>>>> I applied this series and it fixes most of the problems I saw before.
> >>>>> I still see a few crashes - I made issues for them on launchpad:
> >>>>> https://bugs.launchpad.net/qemu/+bug/1890310
> >>>>> https://bugs.launchpad.net/qemu/+bug/1890311
> >>>>> https://bugs.launchpad.net/qemu/+bug/1890312
> >>> Hi Helge, I can still reproduce this one  ^^^
> >>> I'll fuzz it some more, but so far I am not finding anything else.
> >>
> >> I've now updated the patch which does address all issues you found
> >> so far. It's attached below.
> >>
> > I pulled from your repo, but I can still reproduce LP1890312
> > (I build with --enable-sanitizers). Maybe I am missing something? With
> > this cleared up, I'm happy to leave an Acked-by for the artist patches
> > in this series.
> >
> > git show --summary
> > commit 1657a7a95adc15552138c2b4d310a06128093892 (HEAD, hppa/target-hppa, 
> > target-hppa)
> > Author: Helge Deller <deller@gmx.de>
> > Date:   Tue Aug 4 15:35:38 2020 +0200
> 
> The current tree at
> https://github.com/hdeller/qemu-hppa/commits/target-hppa
> does survive all your tests and in addition fixes an artist bug which
> made the dtwm window manager rendering wrong on HP-UX.
> Please completely revert your tree before pulling again.
> I'll send out a complete patch series shortly.
> 

Hi Helge,
This still causes a segfault for me:

cat << EOF | ./hppa-softmmu/qemu-system-hppa -m 64 -display none \
-qtest stdio -accel qtest
writew 0xf8118001 0x105a
readq 0xf900f8ff
EOF

I think the problem is that there is a bounds check missing on the
artist_vram_read with s->cmap_bm_access path. The bounds check is
present for artist_vram_write, so I just copied it over, and that fixed
the issue (applied on top of the current qemu-hppa/target-hppa):

diff --git a/hw/display/artist.c b/hw/display/artist.c
index 720db179ab..678023bb3a 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -1216,8 +1216,10 @@ static uint64_t artist_vram_read(void *opaque, hwaddr 
addr, unsigned size)
 
     if (s->cmap_bm_access) {
         buf = &s->vram_buffer[ARTIST_BUFFER_CMAP];
-        val = *(uint32_t *)(buf->data + addr);
-        trace_artist_vram_read(size, addr, 0, 0, val);
+        if (addr + 3 < buf->size) {
+            val = *(uint32_t *)(buf->data + addr);
+            trace_artist_vram_read(size, addr, 0, 0, val);
+        }
         return 0;
     }
 

-Alex


> Thanks!
> Helge