On Thu, 18 Jan 2018 18:33:51 +0100
Claudio Imbrenda <imbre...@linux.vnet.ibm.com> wrote:
> On Thu, 18 Jan 2018 18:25:47 +0100
> Cornelia Huck <coh...@redhat.com> wrote:
>
> ...
> [snip]
>
> > > > > > > diff --git a/hw/s390x/s390-stattrib-kvm.c
> > > > > > > b/hw/s390x/s390-stattrib-kvm.c index 41770a7..480551c 100644
> > > > > > > --- a/hw/s390x/s390-stattrib-kvm.c
> > > > > > > +++ b/hw/s390x/s390-stattrib-kvm.c
> > > > > > > @@ -116,7 +116,7 @@ static void
> > > > > > > kvm_s390_stattrib_synchronize(S390StAttribState *sa) for
> > > > > > > (cx = 0; cx + len <= max; cx += len) { clog.start_gfn = cx;
> > > > > > > clog.count = len;
> > > > > > > - clog.values = (uint64_t)(sas->incoming_buffer
> > > > > > > + cx
> > > > > > > * len);
> > > > > >
> > > > > > Hm, doesn't that even imply that you reference an area beyond
> > > > > > the buffer, as the <= max check does not catch this?
> > > > >
> > > > > what do you mean?
> > > > >
> > > > > cx + len <= max catches the cases where you would write beyond
> > > > > the end of the buffer. if cx + len == max then we are filling
> > > > > the buffer to the last byte. and we will get out at the next
> > > > > iteration.
> > > >
> > > > Yes, but the problem is that your offset is too long, isn't it?
> > > > (Where cx + len <= max, but you use an offset of cx * len which
> > > > may be > max.)
> > >
> > > which is exactly why I'm removing that line. look at the very
> > > beginning of the line, there is a -
> > >
> > > the replacement line (the one that starts with a +) has only cx
> >
> > Err, yes :) I simply wanted to comment that this looks worse than "not
> > migrated completely".
>
> yeah, that's true :) but the offset ended up big enough to always
> get -EFAULT from the kernel and get ignored by qemu afterwards, which
> then resulted in not all values being migrated.
So the moral is: If you're wrong, be really wrong? :)
>
> should I change the description to explain the issue in more detail?
Just mentioning something like out-of-bounds due to wrong offset or so
would be good.
