qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] exec: Stop using memory after free


From: Gonglei (Arei)
Subject: Re: [Qemu-devel] [PATCH] exec: Stop using memory after free
Date: Wed, 2 Dec 2015 07:59:15 +0000

> Subject: RE: [PATCH] exec: Stop using memory after free
> 
> > From: Paolo Bonzini [mailto:address@hidden
> >
> > On 30/11/2015 23:11, Don Slutz wrote:
> > > memory_region_unref(mr) can free memory.
> > >
> > > For example I got:
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > > [Switching to Thread 0x7f43280d4700 (LWP 4462)]
> > > 0x00007f43323283c0 in phys_section_destroy (mr=0x7f43259468b0)
> > >     at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
> > > 1023        if (mr->subpage) {
> > > (gdb) bt
> > >     at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
> > >     at /home/don/xen/tools/qemu-xen-dir/exec.c:1034
> > >     at /home/don/xen/tools/qemu-xen-dir/exec.c:2205
> > > (gdb) p mr
> > > $1 = (MemoryRegion *) 0x7f43259468b0
> > >
> > > And this change prevents this.
> >
> > Great, thanks!  I think this fixes also the problem that Gonglei was
> > seeing a few months ago.  I'll queue it for 2.5.
> >
> > BTW, since I have your attention, have you noticed my refresh/rewrite
> > of your
> > SAS1068 patches?  A review would be welcome.
> >
> 
> Nice catch! I will check the issue tomorrow.
> 
> Thanks for reminding and CC'ing me, Paolo.
> 
> Regards,
> -Gonglei
> 
> > Paolo
> >
> > > Signed-off-by: Don Slutz <address@hidden>
> > > ---
> > >  exec.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/exec.c b/exec.c
> > > index de1cf19..0bf0a6e 100644
> > > --- a/exec.c
> > > +++ b/exec.c
> > > @@ -1064,9 +1064,11 @@ static uint16_t
> phys_section_add(PhysPageMap
> > > *map,
> > >
> > >  static void phys_section_destroy(MemoryRegion *mr)  {
> > > +    bool have_sub_page = mr->subpage;
> > > +
> > >      memory_region_unref(mr);
> > >
> > > -    if (mr->subpage) {
> > > +    if (have_sub_page) {
> > >          subpage_t *subpage = container_of(mr, subpage_t, iomem);

Can we use the *mr* here again?

IMO we should invoke memory_region_unref(mr) after the if check.

Regards,
-Gonglei

> > >          object_unref(OBJECT(&subpage->iomem));
> > >          g_free(subpage);
> > >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]