Bug 671569

Summary:	race condition in libvirt could lead to crash on event handling
Product:	Red Hat Enterprise Linux 5	Reporter:	Eric Blake <eblake>
Component:	libvirt	Assignee:	Eric Blake <eblake>
Status:	CLOSED ERRATA	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	5.6	CC:	ajia, eblake, gren, kgrainge, veillard, virt-maint, xen-maint, ydu, yoyzhang
Target Milestone:	rc
Target Release:	5.7
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	libvirt-0.8.2-17	Doc Type:	Bug Fix
Doc Text:	libvirt has a race window where event handlers refer to a callback handler that is retrieved from an array. The array can be simultaneously reallocated by other threads, so if the race is lost, another thread can reallocate the array. Previously, libvirt would refer back to the original array, so the callback was called with stale data or libvirt would crash because it was accessing invalidated memory. libvirt has been updated so that it no longer refers to the old array and behaves as expected.	Story Points:	---
Clone Of:	671567	Environment:
Last Closed:	2011-07-21 10:31:15 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	671567
Bug Blocks:

Description Eric Blake 2011-01-21 21:26:51 UTC

+++ This bug was initially created as a clone of Bug #671567 +++

Description of problem:
Libvirt has a race window where event handlers make reference to a callback handler retrieved from an array that can be simultaneously reallocated by other threads.  If this race is lost, the callback will be called with stale data; or even worse, libvirt will crash when accessing invalidated memory.

Version-Release number of selected component (if applicable):
libvirt-0.8.7-3.el6; libvirt-0.8.2-15.el5_6.1

How reproducible:
Found via code inspection; it would be difficult to set up a test to actually expose the race without the use of debuggers and/or recompilation to inject arbitrary sleeps() to force the outcome of the race.  The fact that the bug has been upstream for more than 2 years states that either the race is uncommon, or that no one has been able to pin an actual failure/data corruption on this particular bug, but does not lessen the severity of the bug itself.

Steps to Reproduce:
1. thread 1 is in daemon/event.c:virEventDispatchHandles, and determines that a callback must be called prior to releasing the lock
2. thread 2 wakes up and registers a new handle, which causes the eventLoop.handles array to be reallocated
3. thread 1 resumes, and grabs the argument to the callback from stale memory
  
Actual results:
reading stale memory has unspecified results

Expected results:
thread 1 should no longer refer to the array after releasing lock

Additional info:
Fixed by the first hunk of this upstream patch:
https://www.redhat.com/archives/libvir-list/2011-January/msg00921.html

Comment 1 Eric Blake 2011-01-21 23:25:38 UTC

Patch posted for 5.7; it would also be appropriate to clone this to 5.6.z:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2011-January/msg01105.html

Comment 3 Cui Chun 2011-03-22 10:10:26 UTC

Verified it according to bug 671567 comment 4. 

Checked in source packages that libvirt-event-fix-event-handling-data-race.patch has been included in libvirt-0.8.2-17.el5.src.rpm.

So it is passed for libvirt-0.8.2-17.el5.

Comment 5 yanbing du 2011-05-03 09:13:47 UTC

Inspected the code of libvirt-event-fix-event-handling-data-race.patch which has been included in libvirt-0.8.2-18.el5.src.rpm. 
Move to VERIFIED.

Comment 6 Kate Grainger 2011-07-18 03:46:02 UTC

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
libvirt has a race window where event handlers refer to a callback handler that is retrieved from an array. The array can be simultaneously reallocated by other threads, so if the race is lost, another thread can reallocate the array. Previously, libvirt would refer back to the original array, so the callback was called with stale data or libvirt would crash because it was accessing invalidated memory. libvirt has been updated so that it no longer refers to the old array and behaves as expected.

Comment 7 errata-xmlrpc 2011-07-21 10:31:15 UTC

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2011-1019.html