Ping! This looks like it should be an easy one-liner fix
for a Coverity-detected read-from-bogus-memory bug --
could one of the CXL folks have a look at it and send
a patch, please ?
thanks
-- PMM
On Fri, 3 May 2024 at 13:45, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Mon, 26 Jun 2023 at 13:28, Michael S. Tsirkin <m...@redhat.com> wrote:
> >
> > From: Jonathan Cameron <jonathan.came...@huawei.com>
> >
> > Current implementation is very simple so many of the corner
> > cases do not exist (e.g. fragmenting larger poison list entries)
>
> Hi; Coverity has just spotted what looks like a bug in this
> function (CID 1544772) where we write bogus data from the host
> stack into guest memory):
>
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index ab600735eb..d751803188 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -947,6 +947,42 @@ static void set_lsa(CXLType3Dev *ct3d, const void
> > *buf, uint64_t size,
> > */
> > }
> >
> > +static bool set_cacheline(CXLType3Dev *ct3d, uint64_t dpa_offset, uint8_t
> > *data)
> > +{
> > + MemoryRegion *vmr = NULL, *pmr = NULL;
> > + AddressSpace *as;
> > +
> > + if (ct3d->hostvmem) {
> > + vmr = host_memory_backend_get_memory(ct3d->hostvmem);
> > + }
> > + if (ct3d->hostpmem) {
> > + pmr = host_memory_backend_get_memory(ct3d->hostpmem);
> > + }
> > +
> > + if (!vmr && !pmr) {
> > + return false;
> > + }
> > +
> > + if (dpa_offset + CXL_CACHE_LINE_SIZE > ct3d->cxl_dstate.mem_size) {
> > + return false;
> > + }
> > +
> > + if (vmr) {
> > + if (dpa_offset < memory_region_size(vmr)) {
> > + as = &ct3d->hostvmem_as;
> > + } else {
> > + as = &ct3d->hostpmem_as;
> > + dpa_offset -= memory_region_size(vmr);
> > + }
> > + } else {
> > + as = &ct3d->hostpmem_as;
> > + }
> > +
> > + address_space_write(as, dpa_offset, MEMTXATTRS_UNSPECIFIED, &data,
> > + CXL_CACHE_LINE_SIZE);
>
> We've passed '&data' to address_space_write(), which means
> "read from the address on the stack where the function
> argument 'data' lives", so instead of writing 64 bytes of
> data to the guest , we'll write 64 bytes which start with
> a host pointer value and then continue with whatever happens
> to be on the host stack after that.
>
> I assume the intention was "data", not "&data"...
thanks
-- PMM
