On Tue, 26 Mar 2019 at 12:26, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Mon, 25 Mar 2019 at 18:13, Juan Quintela <quint...@redhat.com> wrote:
> >
> > This way we can change the packet size in the future and everything
> > will work. We choose an arbitrary big number (100 times configured
> > size) as a limit about how big we will reallocate.
> >
> > Signed-off-by: Juan Quintela <quint...@redhat.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Signed-off-by: Juan Quintela <quint...@redhat.com>
> > --
>
> Hi; Coverity reports a use-after-free in this code
> (CID 1400442):
Ping? Have we got a fix for this yet ?
thanks
-- PMM
>
> > @@ -832,12 +832,24 @@ static int
> > multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
> > p->flags = be32_to_cpu(packet->flags);
> >
> > packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
> > - if (packet->pages_alloc > page_count) {
> > + /*
> > + * If we recevied a packet that is 100 times bigger than expected
> > + * just stop migration. It is a magic number.
> > + */
> > + if (packet->pages_alloc > pages_max * 100) {
> > error_setg(errp, "multifd: received packet "
> > - "with size %d and expected maximum size %d",
> > - packet->pages_alloc, page_count) ;
> > + "with size %d and expected a maximum size of %d",
> > + packet->pages_alloc, pages_max * 100) ;
> > return -1;
> > }
> > + /*
> > + * We received a packet that is bigger than expected but inside
> > + * reasonable limits (see previous comment). Just reallocate.
> > + */
> > + if (packet->pages_alloc > p->pages->allocated) {
> > + multifd_pages_clear(p->pages);
>
> multifd_pages_clear() calls g_free() on the pointer it is passed...
>
> > + multifd_pages_init(packet->pages_alloc);
> > + }
> >
> > p->pages->used = be32_to_cpu(packet->pages_used);
>
> ...but here we fall through and dereference p->pages, which
> we might have just freed.
>
> > if (p->pages->used > packet->pages_alloc) {
> > --
> > 2.20.1
>
> thanks
> -- PMM
