KVM segfaults with 3.5 while installing ubuntu 12.04

Commit Message

Avi Kivity Aug. 6, 2012, 12:37 p.m. UTC

On 08/06/2012 03:12 PM, Avi Kivity wrote:
> On 08/06/2012 11:46 AM, Stefan Priebe - Profihost AG wrote:
> 
>> But still i got the segfault and core dump - this is my main problem? I
>> mean qemu-kvm master isn't declared as stable. So i don't care about the
>> slowness here.
>> 
>> What can we do about the core dump and crash?
> 
> Okay, I reproduced it; it seems aio=native is the culprit.  You can try
> aio=threads as a workaround.
> 
> Copying some relevant people (context: aio=native on qemu-kvm-1.1.1
> segfaults pretty early during guest install)
> 

The following ought to fix it:


From: Avi Kivity <[e-mail]>
Date: Mon, 6 Aug 2012 15:35:02 +0300
Subject: [PATCH] virtio-mlk: fix use-after-free while handling scsi commands

The scsi passthrough handler falls through after completing a
request into the failure path, resulting in a use after free.

Reprducible by running a guest with aio=native on a block device.

Reported-by: Stefan Priebe <[e-mail]>
Signed-off-by: Avi Kivity <[e-mail]>

Comments

Avi Kivity Aug. 6, 2012, 12:48 p.m. UTC | #1

On 08/06/2012 03:37 PM, Avi Kivity wrote:
> On 08/06/2012 03:12 PM, Avi Kivity wrote:
>> On 08/06/2012 11:46 AM, Stefan Priebe - Profihost AG wrote:
>> 
>>> But still i got the segfault and core dump - this is my main problem? I
>>> mean qemu-kvm master isn't declared as stable. So i don't care about the
>>> slowness here.
>>> 
>>> What can we do about the core dump and crash?
>> 
>> Okay, I reproduced it; it seems aio=native is the culprit.  You can try
>> aio=threads as a workaround.
>> 
>> Copying some relevant people (context: aio=native on qemu-kvm-1.1.1
>> segfaults pretty early during guest install)
>> 
> 
> The following ought to fix it:

It does.

Kevin: despite aio=native, I get iothreads and pwrites, something is broken.

Stefan Priebe - Profihost AG Aug. 6, 2012, 1:11 p.m. UTC | #2

can confirm - this fixed it!
Am 06.08.2012 14:37, schrieb Avi Kivity:
> On 08/06/2012 03:12 PM, Avi Kivity wrote:
>> On 08/06/2012 11:46 AM, Stefan Priebe - Profihost AG wrote:
>>
>>> But still i got the segfault and core dump - this is my main problem? I
>>> mean qemu-kvm master isn't declared as stable. So i don't care about the
>>> slowness here.
>>>
>>> What can we do about the core dump and crash?
>>
>> Okay, I reproduced it; it seems aio=native is the culprit.  You can try
>> aio=threads as a workaround.
>>
>> Copying some relevant people (context: aio=native on qemu-kvm-1.1.1
>> segfaults pretty early during guest install)
>>
>
> The following ought to fix it:
>
>
> From: Avi Kivity <[e-mail]>
> Date: Mon, 6 Aug 2012 15:35:02 +0300
> Subject: [PATCH] virtio-mlk: fix use-after-free while handling scsi commands
>
> The scsi passthrough handler falls through after completing a
> request into the failure path, resulting in a use after free.
>
> Reprducible by running a guest with aio=native on a block device.
>
> Reported-by: Stefan Priebe <[e-mail]>
> Signed-off-by: Avi Kivity <[e-mail]>
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index f21757e..552b3b6 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -254,6 +254,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>
>       virtio_blk_req_complete(req, status);
>       g_free(req);
> +    return;
>   #else
>       abort();
>   #endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Priebe - Profihost AG Aug. 8, 2012, 5:51 a.m. UTC | #3

Any news? Was this applied upstream?

Am 06.08.2012 14:37, schrieb Avi Kivity:
> On 08/06/2012 03:12 PM, Avi Kivity wrote:
>> On 08/06/2012 11:46 AM, Stefan Priebe - Profihost AG wrote:
>>
>>> But still i got the segfault and core dump - this is my main problem? I
>>> mean qemu-kvm master isn't declared as stable. So i don't care about the
>>> slowness here.
>>>
>>> What can we do about the core dump and crash?
>>
>> Okay, I reproduced it; it seems aio=native is the culprit.  You can try
>> aio=threads as a workaround.
>>
>> Copying some relevant people (context: aio=native on qemu-kvm-1.1.1
>> segfaults pretty early during guest install)
>>
>
> The following ought to fix it:
>
>
> From: Avi Kivity <[e-mail]>
> Date: Mon, 6 Aug 2012 15:35:02 +0300
> Subject: [PATCH] virtio-mlk: fix use-after-free while handling scsi commands
>
> The scsi passthrough handler falls through after completing a
> request into the failure path, resulting in a use after free.
>
> Reprducible by running a guest with aio=native on a block device.
>
> Reported-by: Stefan Priebe <[e-mail]>
> Signed-off-by: Avi Kivity <[e-mail]>
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index f21757e..552b3b6 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -254,6 +254,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>
>       virtio_blk_req_complete(req, status);
>       g_free(req);
> +    return;
>   #else
>       abort();
>   #endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Hajnoczi Aug. 8, 2012, 8:06 a.m. UTC | #4

On Wed, Aug 08, 2012 at 07:51:07AM +0200, Stefan Priebe wrote:
> Any news? Was this applied upstream?

Kevin is ill.  He has asked me to review and test patches in his
absence.  When he gets back later this week this will get picked up (and
included in QEMU 1.2).

Here is the tree, it includes this patch:

https://github.com/stefanha/qemu/commits/for-kevin

Stefan

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Priebe - Profihost AG Aug. 8, 2012, 8:29 a.m. UTC | #5

ah OK - thanks. Will there be a fixed 1.1.2 as well?

Stefan

Am 08.08.2012 10:06, schrieb Stefan Hajnoczi:
> On Wed, Aug 08, 2012 at 07:51:07AM +0200, Stefan Priebe wrote:
>> Any news? Was this applied upstream?
>
> Kevin is ill.  He has asked me to review and test patches in his
> absence.  When he gets back later this week this will get picked up (and
> included in QEMU 1.2).
>
> Here is the tree, it includes this patch:
>
> https://github.com/stefanha/qemu/commits/for-kevin
>
> Stefan
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Hajnoczi Aug. 10, 2012, 3:14 p.m. UTC | #6

On Wed, Aug 8, 2012 at 9:29 AM, Stefan Priebe <[e-mail]> wrote:
> ah OK - thanks. Will there be a fixed 1.1.2 as well?

mdroth: Kevin has the fix in his block branch, which means qemu.git
will get it soon.  Here's the commit:

http://repo.or.cz/w/qemu/kevin.git/commit/730a9c53b4e52681fcfe31cf38854cbf91e132c7

>
> Am 08.08.2012 10:06, schrieb Stefan Hajnoczi:
>
>> On Wed, Aug 08, 2012 at 07:51:07AM +0200, Stefan Priebe wrote:
>>>
>>> Any news? Was this applied upstream?
>>
>>
>> Kevin is ill.  He has asked me to review and test patches in his
>> absence.  When he gets back later this week this will get picked up (and
>> included in QEMU 1.2).
>>
>> Here is the tree, it includes this patch:
>>
>> https://github.com/stefanha/qemu/commits/for-kevin
>>
>> Stefan
>>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f21757e..552b3b6 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -254,6 +254,7 @@  static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 
     virtio_blk_req_complete(req, status);
     g_free(req);
+    return;
 #else
     abort();
 #endif