qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] ide: ahci: unparent child


From: John Snow
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] ide: ahci: unparent children buses before freeing their memory
Date: Mon, 28 Aug 2017 18:41:01 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1


On 08/28/2017 12:34 PM, Igor Mammedov wrote:
> Fixes read after freeing error reported
>   https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04243.html
>   Message-Id: <address@hidden>
> 
> ich9-ahci device creates ide buses and attaches them as QOM children
> at realize time, however it forgets to properly clean them up
> at unrealize time and frees memory containing these children,
> with following call-chain:
> 
>    qdev_device_add()
>      object_property_set_bool('realized', true)
>        device_set_realized()
>           ...
>           pci_qdev_realize() -> pci_ich9_ahci_realize() -> ahci_realize()
>                ...
>                s->dev = g_new0(AHCIDevice, ports);
>                ...
>                   AHCIDevice *ad = &s->dev[i];
>                   ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
>                   ^^^ creates bus in memory allocated by above gnew()
>                       and adds it as child propety to ahci device
>           ...
>           hotplug_handler_plug(); -> goto post_realize_fail;
>           pci_qdev_unrealize() -> pci_ich9_uninit() -> ahci_uninit()
>               ...
>                g_free(s->dev);
>                ^^^ free memory that holds children busses
> 
>           return with error from device_set_realized()
> 
> As result later when qdev_device_add() tries to unparent ich9-ahci
> after failed device_set_realized(),
>     object_unparent() -> object_property_del_child()
> iterates over existing QOM children including buses added by
> ide_bus_new() and tries to unparent them, which causes access to
> freed memory where they where located.
> 
> Reported-by: Thomas Huth <address@hidden>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>  hw/ide/ahci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 406a1b5..ccbe091 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1495,6 +1495,7 @@ void ahci_uninit(AHCIState *s)
>  
>              ide_exit(s);
>          }
> +        object_unparent(OBJECT(&ad->port));
>      }
>  
>      g_free(s->dev);
> 

Nice, Thank you.

Reviewed-by: John Snow <address@hidden>

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js



reply via email to

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