Bug 1772842

Summary: Libvirtd is passing a NULL for the ids parameter by mistake
Product: Red Hat Enterprise Linux Advanced Virtualization Reporter: Dan Zheng <dzheng>
Component: libvirtAssignee: Erik Skultety <eskultet>
Status: CLOSED ERRATA QA Contact: Dan Zheng <dzheng>
Severity: urgent Docs Contact:
Priority: urgent    
Version: 8.2CC: berrange, chhu, dyuan, eskultet, jdenemar, jen, jsuchane, lcheng, lmen, weizhan, xuzhang
Target Milestone: betaKeywords: Automation, Regression, TestBlocker
Target Release: 8.0Flags: pm-rhel: mirror+
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-5.9.0-4.el8 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-05-05 09:50:55 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Dan Zheng 2019-11-15 10:00:24 UTC 
Description of problem:
Libvirtd is passing a NULL for the ids parameter by mistake, but client did pass non-Null for ids which causes virConnectListDomains to report an error.

Version-Release number of selected component (if applicable):
libvirt-5.9.0-2.module+el8.2.0+4683+7e10e783.x86_64

How reproducible:
100%

Steps to Reproduce:
1.
use Sys::Virt;

my $conn = Sys::Virt->new(address => 'qemu:///session');
print $conn->list_domains, "\n";


Client side the Perl bindings are correct:

# LIBVIRT_LOG_FILTERS=1:libvirt LIBVIRT_LOG_OUTPUTS=1:stderr  perl demo.pl
2019-11-15 09:20:51.953+0000: 2515731: debug : virConnectOpenInternal:1035 : driver 6 remote returned SUCCESS
2019-11-15 09:20:51.953+0000: 2515731: debug : virConnectNumOfDomains:90 : conn=0x5629b4f9fe70
2019-11-15 09:20:51.953+0000: 2515731: debug : virConnectListDomains:57 : conn=0x5629b4f9fe70, ids=0x5629b50d9570, maxids=0

^^^^ids is non-NULL

2019-11-15 09:20:51.953+0000: 2515731: error : virNetClientProgramDispatchError:172 : ids in virConnectListDomains must not be NULL
libvirt error code: 8, message: ids in virConnectListDomains must not be NULL
2019-11-15 09:20:51.953+0000: 2515731: debug : virConnectClose:1229 : conn=0x5629b4f9fe70


If we use logging in libvitd

2019-11-15 09:20:51.953+0000: 2515617: debug : virConnectOpenInternal:1035 : driver 14 QEMU returned SUCCESS
2019-11-15 09:20:51.953+0000: 2515615: debug : virConnectNumOfDomains:90 : conn=0x7f236c003630
2019-11-15 09:20:51.953+0000: 2515616: debug : virConnectListDomains:57 : conn=0x7f236c003630, ids=(nil), maxids=0
2019-11-15 09:20:51.953+0000: 2515616: error : virConnectListDomains:62 : ids in virConnectListDomains must not be NULL


So the remote driver is passing a NULL for the ids parameter.

This is because  g_malloc() will return a NULL pointer when
asked for 0 bytes, where as malloc() which libvirt called
via VIR_ALLOC will return a 1 byte allocation, even if asked
for 0 bytes.  Mstly we should have been ok to cope with
either behaviour, except that we have this non-NULL param
check.

Libvirt shouldn't require ids to be non-NULL when maxids is
zero IMHO. This pattern probably spreads across many other
places.



Actual results:
See above

Expected results:
No error

Additional info:
Comment 1 Jeff Nelson 2019-11-15 19:29:28 UTC 
This is a serious issue that affects OSCI testing. The basic ability to connect to libvirt fails.

Raising priority and severity and setting ITR and blocker?.
Comment 4 Daniel Berrangé 2019-11-19 11:03:44 UTC 
PoC posted by Erik at

https://www.redhat.com/archives/libvir-list/2019-November/msg00791.html
Comment 5 Erik Skultety 2019-11-21 10:29:50 UTC 
v2 posted:
https://www.redhat.com/archives/libvir-list/2019-November/msg01043.html
Comment 6 Jiri Denemark 2019-11-21 17:55:11 UTC 
The fix was pushed upstream as

commit d6064e2759a24e0802f363e3a810dc5a7d7ebb15
Refs: v5.9.0-361-gd6064e2759
Author:     Erik Skultety <eskultet>
AuthorDate: Mon Nov 18 12:04:16 2019 +0100
Commit:     Erik Skultety <eskultet>
CommitDate: Thu Nov 21 18:16:35 2019 +0100

    libvirt-<module>: Check caller-provided buffers to be NULL with size > 0

    Pre-Glib era which used malloc allowed the size of the client-side
    buffers to be declared as 0, because malloc documents that it can either
    return 0 or a unique pointer on 0 size allocations.
    With glib this doesn't work anymore, because glib documents that for
    such allocation requests NULL is always returned which results in an
    error in our public API checks server-side.
    This patch complements the fix in the RPC layer by explicitly erroring
    out on the following combination of args used by our legacy APIs (their
    moder equivalents don't suffer from this):

    function(caller-allocated-array, size, ...) {
       	if (!caller-allocated-array && size > 0)
            return error;
    }

    treating everything else as a valid input and potentially let that fail
    on the server-side rather than client-side.

    https://bugzilla.redhat.com/show_bug.cgi?id=1772842

    Signed-off-by: Erik Skultety <eskultet>
    Reviewed-by: Daniel P. Berrangé <berrange>
Comment 9 Dan Zheng 2019-11-22 01:54:39 UTC 
Test packages:

perl-Sys-Virt-5.8.0-1.module+el8.2.0+4793+b09dd2fb.x86_64
libvirt-5.9.0-4.module+el8.2.0+4836+a8e32ad7.x86_64
qemu-kvm-4.2.0-1.module+el8.2.0+4793+b09dd2fb.x86_64
kernel-4.18.0-151.el8.x86_64

perl-Sys-Virt acceptance automation job can run and get results. This bug originally blocked all the tests of perl-Sys-Virt job. 
So I verify this bug now.
Comment 12 errata-xmlrpc 2020-05-05 09:50:55 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:2017