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.