* [Qemu-devel] [PATCH] vhost: fix log base address
@ 2015-04-15 9:37 Michael S. Tsirkin
2015-04-15 9:56 ` zhanghailiang
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2015-04-15 9:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell
VHOST_SET_LOG_BASE got an incorrect address, causing
migration errors and potentially even memory corruption.
Cc: Peter Maydell <peter.maydell@linaro.org>
Reported-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Could you please confirm this fixes the problem for you?
hw/virtio/vhost.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 8dd2f59..02c5604 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
}
if (hdev->log_enabled) {
+ uint64_t log_base;
+
hdev->log_size = vhost_get_log_size(hdev);
hdev->log = hdev->log_size ?
g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
- r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, hdev->log);
+ log_base = (uint64_t)(unsigned long)log_base;
+ r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, &log_base);
if (r < 0) {
r = -errno;
goto fail_log;
--
MST
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-15 9:37 [Qemu-devel] [PATCH] vhost: fix log base address Michael S. Tsirkin
@ 2015-04-15 9:56 ` zhanghailiang
2015-04-15 10:08 ` Wen Congyang
2015-04-16 21:26 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: zhanghailiang @ 2015-04-15 9:56 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel; +Cc: Peter Maydell, peter.huangpeng
On 2015/4/15 17:37, Michael S. Tsirkin wrote:
> VHOST_SET_LOG_BASE got an incorrect address, causing
> migration errors and potentially even memory corruption.
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Could you please confirm this fixes the problem for you?
>
> hw/virtio/vhost.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 8dd2f59..02c5604 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> }
>
> if (hdev->log_enabled) {
> + uint64_t log_base;
> +
> hdev->log_size = vhost_get_log_size(hdev);
> hdev->log = hdev->log_size ?
> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, hdev->log);
> + log_base = (uint64_t)(unsigned long)log_base;
^^^^^^^^
s/log_base/hdev->log ?
> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, &log_base);
> if (r < 0) {
> r = -errno;
> goto fail_log;
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-15 9:56 ` zhanghailiang
@ 2015-04-15 10:08 ` Wen Congyang
2015-04-16 21:26 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: Wen Congyang @ 2015-04-15 10:08 UTC (permalink / raw)
To: zhanghailiang, Michael S. Tsirkin, qemu-devel
Cc: Peter Maydell, peter.huangpeng
On 04/15/2015 05:56 PM, zhanghailiang wrote:
> On 2015/4/15 17:37, Michael S. Tsirkin wrote:
>> VHOST_SET_LOG_BASE got an incorrect address, causing
>> migration errors and potentially even memory corruption.
>>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>
>> Could you please confirm this fixes the problem for you?
>>
>> hw/virtio/vhost.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 8dd2f59..02c5604 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>> }
>>
>> if (hdev->log_enabled) {
>> + uint64_t log_base;
>> +
>> hdev->log_size = vhost_get_log_size(hdev);
>> hdev->log = hdev->log_size ?
>> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
>> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, hdev->log);
>> + log_base = (uint64_t)(unsigned long)log_base;
> ^^^^^^^^
>
> s/log_base/hdev->log ?
I test the patch with this modification. It works for me.
Thanks
Wen Congyang
>
>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, &log_base);
>> if (r < 0) {
>> r = -errno;
>> goto fail_log;
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-15 9:56 ` zhanghailiang
2015-04-15 10:08 ` Wen Congyang
@ 2015-04-16 21:26 ` Paolo Bonzini
2015-04-17 4:02 ` Wen Congyang
2015-04-17 14:40 ` Peter Maydell
1 sibling, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2015-04-16 21:26 UTC (permalink / raw)
To: zhanghailiang, Michael S. Tsirkin, qemu-devel
Cc: Peter Maydell, peter.huangpeng
On 15/04/2015 11:56, zhanghailiang wrote:
> On 2015/4/15 17:37, Michael S. Tsirkin wrote:
>> VHOST_SET_LOG_BASE got an incorrect address, causing
>> migration errors and potentially even memory corruption.
>>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>
>> Could you please confirm this fixes the problem for you?
>>
>> hw/virtio/vhost.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 8dd2f59..02c5604 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev,
>> VirtIODevice *vdev)
>> }
>>
>> if (hdev->log_enabled) {
>> + uint64_t log_base;
>> +
>> hdev->log_size = vhost_get_log_size(hdev);
>> hdev->log = hdev->log_size ?
>> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
>> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
>> hdev->log);
>> + log_base = (uint64_t)(unsigned long)log_base;
> ^^^^^^^^
>
> s/log_base/hdev->log ?
Also s/unsigned long/uintptr_t/ please. The subsequent cast to uint64_t
is not necessary.
Paolo
>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
>> &log_base);
>> if (r < 0) {
>> r = -errno;
>> goto fail_log;
>>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-16 21:26 ` Paolo Bonzini
@ 2015-04-17 4:02 ` Wen Congyang
2015-04-17 14:40 ` Peter Maydell
1 sibling, 0 replies; 7+ messages in thread
From: Wen Congyang @ 2015-04-17 4:02 UTC (permalink / raw)
To: Paolo Bonzini, zhanghailiang, Michael S. Tsirkin, qemu-devel
Cc: Peter Maydell, peter.huangpeng
On 04/17/2015 05:26 AM, Paolo Bonzini wrote:
>
>
> On 15/04/2015 11:56, zhanghailiang wrote:
>> On 2015/4/15 17:37, Michael S. Tsirkin wrote:
>>> VHOST_SET_LOG_BASE got an incorrect address, causing
>>> migration errors and potentially even memory corruption.
>>>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>
>>> Could you please confirm this fixes the problem for you?
>>>
>>> hw/virtio/vhost.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>> index 8dd2f59..02c5604 100644
>>> --- a/hw/virtio/vhost.c
>>> +++ b/hw/virtio/vhost.c
>>> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev,
>>> VirtIODevice *vdev)
>>> }
>>>
>>> if (hdev->log_enabled) {
>>> + uint64_t log_base;
>>> +
>>> hdev->log_size = vhost_get_log_size(hdev);
>>> hdev->log = hdev->log_size ?
>>> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
>>> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
>>> hdev->log);
>>> + log_base = (uint64_t)(unsigned long)log_base;
>> ^^^^^^^^
>>
>> s/log_base/hdev->log ?
>
> Also s/unsigned long/uintptr_t/ please. The subsequent cast to uint64_t
> is not necessary.
Should we also update vhost_dev_log_resize()?
Thanks
Wen Congyang
>
> Paolo
>
>>> + r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
>>> &log_base);
>>> if (r < 0) {
>>> r = -errno;
>>> goto fail_log;
>>>
>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-16 21:26 ` Paolo Bonzini
2015-04-17 4:02 ` Wen Congyang
@ 2015-04-17 14:40 ` Peter Maydell
2015-04-17 14:56 ` Michael S. Tsirkin
1 sibling, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2015-04-17 14:40 UTC (permalink / raw)
To: Paolo Bonzini
Cc: QEMU Developers, Huangpeng (Peter), zhanghailiang,
Michael S. Tsirkin
On 16 April 2015 at 22:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 15/04/2015 11:56, zhanghailiang wrote:
>> On 2015/4/15 17:37, Michael S. Tsirkin wrote:
>>> VHOST_SET_LOG_BASE got an incorrect address, causing
>>> migration errors and potentially even memory corruption.
>>>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>
>>> Could you please confirm this fixes the problem for you?
>>>
>>> hw/virtio/vhost.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>> index 8dd2f59..02c5604 100644
>>> --- a/hw/virtio/vhost.c
>>> +++ b/hw/virtio/vhost.c
>>> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev,
>>> VirtIODevice *vdev)
>>> }
>>>
>>> if (hdev->log_enabled) {
>>> + uint64_t log_base;
>>> +
>>> hdev->log_size = vhost_get_log_size(hdev);
>>> hdev->log = hdev->log_size ?
>>> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
>>> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
>>> hdev->log);
>>> + log_base = (uint64_t)(unsigned long)log_base;
>> ^^^^^^^^
>>
>> s/log_base/hdev->log ?
>
> Also s/unsigned long/uintptr_t/ please. The subsequent cast to uint64_t
> is not necessary.
I think this is our remaining for-2.3 bug; would somebody like
to produce and test a patch with all the fixes mentioned in
this thread?
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] vhost: fix log base address
2015-04-17 14:40 ` Peter Maydell
@ 2015-04-17 14:56 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2015-04-17 14:56 UTC (permalink / raw)
To: Peter Maydell
Cc: Paolo Bonzini, Huangpeng (Peter), zhanghailiang, QEMU Developers
On Fri, Apr 17, 2015 at 03:40:50PM +0100, Peter Maydell wrote:
> On 16 April 2015 at 22:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> >
> > On 15/04/2015 11:56, zhanghailiang wrote:
> >> On 2015/4/15 17:37, Michael S. Tsirkin wrote:
> >>> VHOST_SET_LOG_BASE got an incorrect address, causing
> >>> migration errors and potentially even memory corruption.
> >>>
> >>> Cc: Peter Maydell <peter.maydell@linaro.org>
> >>> Reported-by: Wen Congyang <wency@cn.fujitsu.com>
> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>> ---
> >>>
> >>> Could you please confirm this fixes the problem for you?
> >>>
> >>> hw/virtio/vhost.c | 5 ++++-
> >>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >>> index 8dd2f59..02c5604 100644
> >>> --- a/hw/virtio/vhost.c
> >>> +++ b/hw/virtio/vhost.c
> >>> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev,
> >>> VirtIODevice *vdev)
> >>> }
> >>>
> >>> if (hdev->log_enabled) {
> >>> + uint64_t log_base;
> >>> +
> >>> hdev->log_size = vhost_get_log_size(hdev);
> >>> hdev->log = hdev->log_size ?
> >>> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
> >>> - r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
> >>> hdev->log);
> >>> + log_base = (uint64_t)(unsigned long)log_base;
> >> ^^^^^^^^
> >>
> >> s/log_base/hdev->log ?
> >
> > Also s/unsigned long/uintptr_t/ please. The subsequent cast to uint64_t
> > is not necessary.
>
> I think this is our remaining for-2.3 bug; would somebody like
> to produce and test a patch with all the fixes mentioned in
> this thread?
>
> thanks
> -- PMM
I just posted it but it's not tested yet.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-17 14:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-15 9:37 [Qemu-devel] [PATCH] vhost: fix log base address Michael S. Tsirkin
2015-04-15 9:56 ` zhanghailiang
2015-04-15 10:08 ` Wen Congyang
2015-04-16 21:26 ` Paolo Bonzini
2015-04-17 4:02 ` Wen Congyang
2015-04-17 14:40 ` Peter Maydell
2015-04-17 14:56 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).