[libvirt] [PATCH] Fix crash when multiple event callbacks were registered

CVE-2013-2230 Don't overwrite the callback ID returned by virDomainEventStateRegisterID in ret by 0. Introduced by abf75aea. --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 571d1f8..b0180c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10043,9 +10043,7 @@ qemuConnectDomainEventRegisterAny(virConnectPtr conn, driver->domainEventState, dom, eventID, callback, opaque, freecb, &ret) < 0) - goto cleanup; - - ret = 0; + ret = -1; cleanup: return ret; -- 1.8.1.5

On Wed, Jul 10, 2013 at 12:59:48PM +0200, Ján Tomko wrote:
CVE-2013-2230
This should be in the subject line so it is more visible.
Don't overwrite the callback ID returned by virDomainEventStateRegisterID in ret by 0.
Introduced by abf75aea. --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 571d1f8..b0180c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10043,9 +10043,7 @@ qemuConnectDomainEventRegisterAny(virConnectPtr conn, driver->domainEventState, dom, eventID, callback, opaque, freecb, &ret) < 0) - goto cleanup; - - ret = 0; + ret = -1;
cleanup: return ret;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/10/2013 01:02 PM, Daniel P. Berrange wrote:
On Wed, Jul 10, 2013 at 12:59:48PM +0200, Ján Tomko wrote:
CVE-2013-2230
This should be in the subject line so it is more visible.
Don't overwrite the callback ID returned by virDomainEventStateRegisterID in ret by 0.
Introduced by abf75aea. --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) ACK
Daniel
Thanks, pushed to master and v1.1.0-maint. Jan

On 07/10/2013 05:02 AM, Daniel P. Berrange wrote:
On Wed, Jul 10, 2013 at 12:59:48PM +0200, Ján Tomko wrote:
CVE-2013-2230
This should be in the subject line so it is more visible.
Oh well, it was pushed without the subject line change. But I noticed that DV had added a signed tag to our previous CVE (2013-2218, just before 1.1.0), and that is also easily visible if you use 'tig', so I've just finished creating lots of other signed tags for CVE fixes over the last three years: CVE-2011-1146 CVE-2012-3411 CVE-2013-0170 CVE-2013-2230 CVE-2011-1486 CVE-2012-3445 CVE-2013-1962 CVE-2011-2178 CVE-2012-4423 CVE-2013-2218 Since signed tags can be added after the fact, they are a nice way to consistently mark bug fixes, regardless of whether the commit itself was aware of a CVE number (for example, some of those tags are on commits that were made public long before a CVE was assigned, because no one realized the exploit until after the patch was pushed). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Jul 10, 2013 at 08:11:14AM -0600, Eric Blake wrote:
On 07/10/2013 05:02 AM, Daniel P. Berrange wrote:
On Wed, Jul 10, 2013 at 12:59:48PM +0200, Ján Tomko wrote:
CVE-2013-2230
This should be in the subject line so it is more visible.
Oh well, it was pushed without the subject line change. But I noticed that DV had added a signed tag to our previous CVE (2013-2218, just before 1.1.0), and that is also easily visible if you use 'tig', so I've just finished creating lots of other signed tags for CVE fixes over the last three years:
CVE-2011-1146 CVE-2012-3411 CVE-2013-0170 CVE-2013-2230 CVE-2011-1486 CVE-2012-3445 CVE-2013-1962 CVE-2011-2178 CVE-2012-4423 CVE-2013-2218
Since signed tags can be added after the fact, they are a nice way to consistently mark bug fixes, regardless of whether the commit itself was aware of a CVE number (for example, some of those tags are on commits that were made public long before a CVE was assigned, because no one realized the exploit until after the patch was pushed).
+1 I think at this point it is the best way. The rationale too, is that sometimes we may commit a fix, and the CVE about it gets assigned later. With tags we can always add that extra information. So let's try to be consistent and always use git tags in the future. Those tags should be PGP signed (as you did :) Thanks for doing the history work :-) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

On 07/10/2013 12:59 PM, Ján Tomko wrote:
CVE-2013-2230
Don't overwrite the callback ID returned by virDomainEventStateRegisterID in ret by 0.
Introduced by abf75aea. --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 571d1f8..b0180c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10043,9 +10043,7 @@ qemuConnectDomainEventRegisterAny(virConnectPtr conn, driver->domainEventState, dom, eventID, callback, opaque, freecb, &ret) < 0) - goto cleanup; - - ret = 0; + ret = -1;
cleanup: return ret;
ACK, Martin
participants (5)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake
-
Ján Tomko
-
Martin Kletzander