Re: [PATCH v3] ati-vga: check mm_index before recursive call (CVE-2020-13800)

[BALATON Zoltan]

On Thu, 4 Jun 2020, Gerd Hoffmann wrote:
> > +        } else if (s->regs.mm_index > MM_DATA + 3) {
> >              val = ati_mm_read(s, s->regs.mm_index + addr - MM_DATA, size);
>
> MM_INDEX is 0
> MM_DATA  is 4
> "normal" registers start at 8.
>
> So we want allow indirect access for offset 8 and above and deny offsets
> 0-7.  mm_index is interpreted with an offset, see "- MM_DATA" in the
> call above.

MM_INDEX is the register to read, addr - MM_DATA is an offset for 
unaligned access (when guest reads MM_DATA + 1, size=2 then we need to 
return regs[valueof(MM_INDEX) + 1], size=2.

> Not clear to me why this offset is 4, that doesn't make sense to me.
> I'd expect either no offset or offset being 8.  BALATON, can you
> double-check that with the specs?

We check that valueof(MM_INDEX) is at least MM_DATA + 4 = 8

> Assuming offset 4 is correct we must require mm_index being larger than
> MM_DATA + MM_DATA + 3 ( == 11) to compensate for the offset.

I don't get this, I think you're confusing value of MM_INDEX and offset of 
reading MM_DATA reg itself which together define what register is read 
with what offset during recursion. We don't want to recurse if clients 
tries to access either MM_INDEX or MM_DATA via these regs themselves to 
avoid infinite recursion.

Regards,
BALATON Zoltan