[libvirt] [PATCH v2] Fix libvirtd crash possibility
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally. The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index d13b621..95a6d76 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -1,7 +1,7 @@ /* * virnetserverprogram.c: generic network RPC server program * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -109,12 +109,19 @@ int virNetServerProgramMatches(virNetServerProgramPtr prog, static virNetServerProgramProcPtr virNetServerProgramGetProc(virNetServerProgramPtr prog, int procedure) { + virNetServerProgramProcPtr proc; + if (procedure < 0) return NULL; if (procedure >= prog->nprocs) return NULL; - return &prog->procs[procedure]; + proc = &prog->procs[procedure]; + + if (!proc->func) + return NULL; + + return proc; } unsigned int -- 1.7.12
On 09/12/2012 04:44 PM, Martin Kletzander wrote:
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone
s/continuousline/a continuous line/
tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/12/2012 05:00 PM, Eric Blake wrote:
On 09/12/2012 04:44 PM, Martin Kletzander wrote:
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone
s/continuousline/a continuous line/
tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
ACK.
If you haven't pushed yet, you may want to amend the commit message to mention that this appears to be a regression introduced in commit df0b57a9, in our RPC refactoring for 0.9.3. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/13/2012 05:04 PM, Eric Blake wrote:
On 09/12/2012 05:00 PM, Eric Blake wrote:
On 09/12/2012 04:44 PM, Martin Kletzander wrote:
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone
s/continuousline/a continuous line/
tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
ACK.
If you haven't pushed yet, you may want to amend the commit message to mention that this appears to be a regression introduced in commit df0b57a9, in our RPC refactoring for 0.9.3.
Oh, I pushed it without mentioning this. I missed this the mail at first, sorry. Martin
On 09/12/2012 04:44 PM, Martin Kletzander wrote:
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Given that this fixes CVE-2012-4423, I have gone and backported it to v0.9.6-maint and v0.9.11-maint. https://bugzilla.redhat.com/show_bug.cgi?id=857135 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/19/2012 07:49 PM, Eric Blake wrote:
On 09/12/2012 04:44 PM, Martin Kletzander wrote:
When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Given that this fixes CVE-2012-4423, I have gone and backported it to v0.9.6-maint and v0.9.11-maint. https://bugzilla.redhat.com/show_bug.cgi?id=857135
Oh, I thought that has to wait for Cole. Thanks very much for that. Martin
participants (2)
-
Eric Blake -
Martin Kletzander