Re: [QEMU-SECURITY] Out-of-bounds read in xlnx_dp_read

[Alistair]

On Sun, Jul 11, 2021, at 6:17 PM, Qiang Liu wrote:
> On Fri, Jul 9, 2021 at 1:47 PM P J P <pjp@fedoraproject.org> wrote:
> >
> > Hello Qiang,
> >
> > On Wednesday, 7 July, 2021, 11:38:09 am IST, Qiang Liu 
> > <cyruscyliu@gmail.com> wrote:
> > >I found an out-of-bounds read in xlxn_dp_read and here are the details.
> > >
> > >## Root Cause Analysis
> > >
> > >1. xlnx_dp_read allows one-element-off read at its default branch
> > >because of an improper index.
> > >
> > >```
> > >#define DP_CORE_REG_ARRAY_SIZE (0x3AF >> 2)
> > >struct XlnxDPState {
> > >    uint32_t core_registers[DP_CORE_REG_ARRAY_SIZE];
> > >    MemoryRegion core_iomem;
> > >}
> > >static uint64_t xlnx_dp_read(void *opaque, hwaddr ...
> > >    switch (offset) {
> > >        default:                            // (0x3AF >> 2) is equal
> > >to (0x3AC >> 2)
> > >            assert(offset <= (0x3AC >> 2));  // the op should be <
> > >            ret = s->core_registers[offset]; // one-element-off access
> > >            break;
> > >
> > >According to
> > >https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html,
> > >offset 0x3AC is allowed.
> > >
> > >```
> > >#!/bin/bash -x
> > >export QEMU=/root/qemu/build-ubsan/qemu-system-aarch64
> > >export BUILDROOT=./ cat << EOF | $QEMU \
> > >-machine xlnx-zcu102,accel=qtest -qtest stdio -monitor none -serial none \
> > >-display none -nodefaults -qtest stdio
> > >readl 0xfd4a03ac
> > >EOF
> > >```
> > >
> > >Let me know if I need to provide more information.
> >
> > * Thank you so much for reporting this issue and providing the details. I 
> > appreciate it.
> >
> > * It does not look like the issue maybe reproducible with KVM 
> > virtualization use case.
> >   The machine 'xlnx-zcu102' looks to be a SoC system used in industrial 
> > applications.
> >
> > * Is it reproducible with the KVM virtualization support?
> No, it is not. I got an aarch64 host yesterday and tested this
> machine. The answer is no.
> 
> I'm sorry to bother you. I will test arm/aarch64 machines more with
> the aarch64 host in the future.

This should still be fixed though. Your proposed fix looks correct. Do you want 
to send a patch to the QEMU mailing list to fix the issue?

As it doesn't effect KVM I don't think we need a CVE or to merge the patch 
quietly, so it should be ok to just send to the list. Someone let me know if 
they think otherwise though.

Alistair

> 
> Best,
> Qiang
>