All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	amit.shah@redhat.com, dgilbert@redhat.com, kraxel@redhat.com,
	qemu-devel@nongnu.org, qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list
Date: Tue, 22 Jul 2014 17:48:17 +0200	[thread overview]
Message-ID: <53CE87C1.5060704@redhat.com> (raw)
In-Reply-To: <53CE8671.4070603@redhat.com>

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

  reply	other threads:[~2014-07-22 15:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-22 15:26 [Qemu-devel] [PATCH for-2.1 for-stable] vmstate_xhci_event: fix unterminated field list Laszlo Ersek
2014-07-22 15:35 ` Amit Shah
2014-07-22 15:42 ` Paolo Bonzini
2014-07-22 15:48   ` Laszlo Ersek [this message]
2014-07-22 16:14     ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53CE87C1.5060704@redhat.com \
    --to=lersek@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.