Re: [PATCH] hw/nvme: fix mmio read

[Klaus Jensen]

On Jul 13 12:34, Klaus Jensen wrote:
> On Jul 13 11:31, Peter Maydell wrote:
> > On Tue, 13 Jul 2021 at 11:19, Klaus Jensen <its@irrelevant.dk> wrote:
> > >
> > > On Jul 13 11:07, Peter Maydell wrote:
> > > > Looking at the surrounding code, I notice that we guard this "read size 
> > > > bytes
> > > > from &n->bar + addr" with
> > > >     if (addr < sizeof(n->bar)) {
> > > >
> > > > but that doesn't account for 'size', so if the guest asks to read
> > > > 4 bytes starting at offset sizeof(n->bar)-1 then we'll still read
> > > > 3 bytes beyond the end of the buffer...
> > >
> > > The buffer is at least sizeof(n->bar) + 8 bytes (there are two doorbell
> > > registers following the controller registers). It is wrong for the host
> > > to read those, but as per the spec it is undefined behavior.
> > 
> > I don't know about the doorbell registers, but with this code
> > (or the old memcpy()) you'll access whatever the next thing after
> > "NvmeBar bar" in the NvmeCtrl struct is, which looks like it's the
> > first part of 'struct NvmeParams".
> > 
> 
> Sorry, you are of course right. I remembered how the bar was allocated
> incorrectly.

I fixed this properly by holding all bar values in little endian as per
the spec.

I'll clean it up and send it tonight.

signature.asc
Description: PGP signature