[Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Laszlo Ersek]

"vmstate_xhci_event" was introduced in commit 37352df3 ("xhci: add live
migration support"), and first released in v1.6.0. The field list in this
VMSD is not terminated with the VMSTATE_END_OF_LIST() macro.

During normal use (ie. migration), the issue is practically invisible,
because the "vmstate_xhci_event" object (with the unterminated field list)
is only ever referenced -- via "vmstate_xhci_intr" -- if xhci_er_full()
returns true, for the "ev_buffer" test. Since that field_exists() check
(apparently) almost always returns false, we almost never traverse
"vmstate_xhci_event" during migration, which hides the bug.

However, Amit's vmstate checker forces recursion into this VMSD as well,
and the lack of VMSTATE_END_OF_LIST() breaks the field list terminator
check (field->name != NULL) in dump_vmstate_vmsd(). The result is
undefined behavior, which in my case translates to infinite recursion
(because the loop happens to overflow into "vmstate_xhci_intr", which then
links back to "vmstate_xhci_event").

Add the missing terminator.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/usb/hcd-xhci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7f2af89..58c4b11 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3737,6 +3737,7 @@ static const VMStateDescription vmstate_xhci_event = {
         VMSTATE_UINT32(flags,  XHCIEvent),
         VMSTATE_UINT8(slotid,  XHCIEvent),
         VMSTATE_UINT8(epid,    XHCIEvent),
+        VMSTATE_END_OF_LIST()
     }
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Amit Shah]

On (Tue) 22 Jul 2014 [17:26:41], Laszlo Ersek wrote:
> "vmstate_xhci_event" was introduced in commit 37352df3 ("xhci: add live
> migration support"), and first released in v1.6.0. The field list in this
> VMSD is not terminated with the VMSTATE_END_OF_LIST() macro.
> 
> During normal use (ie. migration), the issue is practically invisible,
> because the "vmstate_xhci_event" object (with the unterminated field list)
> is only ever referenced -- via "vmstate_xhci_intr" -- if xhci_er_full()
> returns true, for the "ev_buffer" test. Since that field_exists() check
> (apparently) almost always returns false, we almost never traverse
> "vmstate_xhci_event" during migration, which hides the bug.
> 
> However, Amit's vmstate checker forces recursion into this VMSD as well,
> and the lack of VMSTATE_END_OF_LIST() breaks the field list terminator
> check (field->name != NULL) in dump_vmstate_vmsd(). The result is
> undefined behavior, which in my case translates to infinite recursion
> (because the loop happens to overflow into "vmstate_xhci_intr", which then
> links back to "vmstate_xhci_event").
> 
> Add the missing terminator.
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Thanks for spotting this!

Reviewed-by: Amit Shah <amit.shah@redhat.com>


		Amit

Re: [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Paolo Bonzini]

Il 22/07/2014 17:26, Laszlo Ersek ha scritto:
> "vmstate_xhci_event" was introduced in commit 37352df3 ("xhci: add live
> migration support"), and first released in v1.6.0. The field list in this
> VMSD is not terminated with the VMSTATE_END_OF_LIST() macro.
> 
> During normal use (ie. migration), the issue is practically invisible,
> because the "vmstate_xhci_event" object (with the unterminated field list)
> is only ever referenced -- via "vmstate_xhci_intr" -- if xhci_er_full()
> returns true, for the "ev_buffer" test. Since that field_exists() check
> (apparently) almost always returns false, we almost never traverse
> "vmstate_xhci_event" during migration, which hides the bug.
> 
> However, Amit's vmstate checker forces recursion into this VMSD as well,
> and the lack of VMSTATE_END_OF_LIST() breaks the field list terminator
> check (field->name != NULL) in dump_vmstate_vmsd(). The result is
> undefined behavior, which in my case translates to infinite recursion
> (because the loop happens to overflow into "vmstate_xhci_intr", which then
> links back to "vmstate_xhci_event").
> 
> Add the missing terminator.
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 7f2af89..58c4b11 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3737,6 +3737,7 @@ static const VMStateDescription vmstate_xhci_event = {
>          VMSTATE_UINT32(flags,  XHCIEvent),
>          VMSTATE_UINT8(slotid,  XHCIEvent),
>          VMSTATE_UINT8(epid,    XHCIEvent),
> +        VMSTATE_END_OF_LIST()
>      }
>  };
>  
> 

Cc: qemu-stable@nongnu.org
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Laszlo Ersek]

On 07/22/14 17:42, Paolo Bonzini wrote:
> Il 22/07/2014 17:26, Laszlo Ersek ha scritto:
>> "vmstate_xhci_event" was introduced in commit 37352df3 ("xhci: add live
>> migration support"), and first released in v1.6.0. The field list in this
>> VMSD is not terminated with the VMSTATE_END_OF_LIST() macro.
>>
>> During normal use (ie. migration), the issue is practically invisible,
>> because the "vmstate_xhci_event" object (with the unterminated field list)
>> is only ever referenced -- via "vmstate_xhci_intr" -- if xhci_er_full()
>> returns true, for the "ev_buffer" test. Since that field_exists() check
>> (apparently) almost always returns false, we almost never traverse
>> "vmstate_xhci_event" during migration, which hides the bug.
>>
>> However, Amit's vmstate checker forces recursion into this VMSD as well,
>> and the lack of VMSTATE_END_OF_LIST() breaks the field list terminator
>> check (field->name != NULL) in dump_vmstate_vmsd(). The result is
>> undefined behavior, which in my case translates to infinite recursion
>> (because the loop happens to overflow into "vmstate_xhci_intr", which then
>> links back to "vmstate_xhci_event").
>>
>> Add the missing terminator.
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  hw/usb/hcd-xhci.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>> index 7f2af89..58c4b11 100644
>> --- a/hw/usb/hcd-xhci.c
>> +++ b/hw/usb/hcd-xhci.c
>> @@ -3737,6 +3737,7 @@ static const VMStateDescription vmstate_xhci_event = {
>>          VMSTATE_UINT32(flags,  XHCIEvent),
>>          VMSTATE_UINT8(slotid,  XHCIEvent),
>>          VMSTATE_UINT8(epid,    XHCIEvent),
>> +        VMSTATE_END_OF_LIST()
>>      }
>>  };
>>  
>>
> 
> Cc: qemu-stable@nongnu.org

As far as I can see, this address was present in my original To: list.
:) (Admittedly, not with CC.)

> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thank you!

Laszlo

Re: [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list

[Peter Maydell]

On 22 July 2014 16:48, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/22/14 17:42, Paolo Bonzini wrote:
>> Cc: qemu-stable@nongnu.org
>
> As far as I can see, this address was present in my original To: list.
> :) (Admittedly, not with CC.)

Including the line in the commit message means it can get
pulled back out of the git log automatically for application
to the stable branch.

I'll commit this to master shortly.

thanks
-- PMM