Bug 1846450

Summary: libvirt-daemon-driver-vbox: NULL pointer deref when dealing with shared folders in domain specification
Product: [Fedora] Fedora Reporter: Jan Pokorný [poki] <fedora>
Component: libvirtAssignee: Ján Tomko <jtomko>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 33CC: agedosier, berrange, clalancette, crobinso, itamar, jforbes, jtomko, laine, libvirt-maint, veillard, virt-maint
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-08-18 21:26:10 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 Jan Pokorný [poki] 2020-06-11 15:33:07 UTC
While I can see VirtualBox support is a niche especially in Fedora,
there appears to be a bug with how Virtual Box driver of libvirt
deals with libvirt's own native data structures, observe:

(gdb) bt             
#0  0x00007f0d0ae227eb in vboxDumpSharedFolders (machine=<optimized out>, data=0x7f0d1c001d60, def=0x7f0d200140a0) at ../../src/vbox/vbox_common.c:36
30
#1  vboxDomainGetXMLDesc (dom=<optimized out>, flags=<optimized out>) at ../../src/vbox/vbox_common.c:4128
#2  0x00007f0d2f142146 in virDomainGetXMLDesc () at /lib64/libvirt.so.0
#3  0x0000560a4e76e1f8 in remoteDispatchDomainGetXMLDesc (server=0x560a50682df0, msg=0x560a50725330, ret=0x7f0d20000b80, args=0x7f0d20015550, rerr=0x
7f0d2c975960, client=<optimized out>) at ./remote/remote_daemon_dispatch_stubs.h:7077
#4  remoteDispatchDomainGetXMLDescHelper (server=0x560a50682df0, client=<optimized out>, msg=0x560a50725330, rerr=0x7f0d2c975960, args=0x7f0d20015550
, ret=0x7f0d20000b80) at ./remote/remote_daemon_dispatch_stubs.h:7054
#5  0x00007f0d2f065a70 in virNetServerProgramDispatch () at /lib64/libvirt.so.0
#6  0x00007f0d2f06ab58 in virNetServerHandleJob () at /lib64/libvirt.so.0
#7  0x00007f0d2ef7a772 in virThreadPoolWorker () at /lib64/libvirt.so.0
#8  0x00007f0d2ef79cf9 in virThreadHelper () at /lib64/libvirt.so.0
#9  0x00007f0d2ec00479 in start_thread (arg=0x7f0d2c976680) at pthread_create.c:462
#10 0x00007f0d2eb12b53 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

/usr/src/debug/libvirt-6.4.0-1.fc33.x86_64/src/vbox/vbox_common.c:

3628│         gVBoxAPI.UISharedFolder.GetHostPath(sharedFolder, &hostPathUtf16);
3629│         VBOX_UTF16_TO_UTF8(hostPathUtf16, &hostPath);
3630├>        def->fss[i]->src->path = g_strdup(hostPath);
3631│         VBOX_UTF8_FREE(hostPath);
3632│         VBOX_UTF16_FREE(hostPathUtf16);

(gdb) p def->fss[i]->src 
$1 = (virStorageSourcePtr) 0x0

(gdb) ptype def->fss[i]->src
type = struct _virStorageSource {
    virObject parent;
    unsigned int id;
    int type;
    char *path;
    int protocol;
    char *volume;
    char *snapshot;
    char *configFile;
    char *query;
    size_t nhosts;
    virStorageNetHostDefPtr hosts;
    size_t ncookies;
    virStorageNetCookieDefPtr *cookies;
    virStorageSourcePoolDefPtr srcpool;
    virStorageAuthDefPtr auth;
    virStorageEncryptionPtr encryption;
    virStoragePRDefPtr pr;
    virTristateBool sslverify;
    unsigned long long readahead;
    unsigned long long timeout;
    virStorageSourceNVMeDefPtr nvme;
    virStorageSourceInitiatorDef initiator;
    virObjectPtr privateData;
    int format;
    virBitmapPtr features;
    char *compat;
    _Bool nocow;
    _Bool sparse;
    virStorageSourceSlicePtr sliceStorage;
    virStoragePermsPtr perms;
    virStorageTimestampsPtr timestamps;
    unsigned long long capacity;
    unsigned long long allocation;
    unsigned long long physical;
    _Bool has_allocation;
    size_t nseclabels;
    virSecurityDeviceLabelDefPtr *seclabels;
    _Bool readonly;
    _Bool shared;
    virStorageSourcePtr backingStore;
    virStorageDriverDataPtr drv;
    char *relPath;
    char *backingStoreRaw;
    virStorageFileFormat backingStoreRawFormat;
    char *nodeformat;
    char *nodestorage;
    int haveTLS;
    _Bool tlsFromConfig;
    char *tlsAlias;
    char *tlsCertdir;
    _Bool detected;
    unsigned int debugLevel;
    _Bool debug;
    int iomode;
    int cachemode;
    int discard;
    int detect_zeroes;
    _Bool floppyimg;
    _Bool hostcdrom;
    char *ssh_user;
    _Bool ssh_host_key_check_disabled;
} *

Might be a fallout after some generalization from mere paths?

Nonetheless, it doesn't appear to involve VirtualBox API as such,
that's why I dare to raise it here :-)

$ rpm -qf /usr/sbin/libvirtd \
  /usr/lib64/libvirt/connection-driver/libvirt_driver_vbox.so
> libvirt-daemon-6.4.0-1.fc33.x86_64
> libvirt-daemon-driver-vbox-6.4.0-1.fc33.x86_64

Comment 1 Ján Tomko 2020-06-18 09:46:34 UTC
Yes, it looks like we forgot to allocate def->fss[i] properly back in 2016:
commit da665fbd4858890fbb3bbf5da2a7b6ca37bb3220
    filesystem: adds possibility to use storage pool as fs source

Comment 2 Ján Tomko 2020-06-18 12:15:53 UTC
Proposed upstream patch:
https://www.redhat.com/archives/libvir-list/2020-June/msg00779.html

Comment 3 Ben Cotton 2020-08-11 13:36:35 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 33 development cycle.
Changing version to 33.

Comment 4 Cole Robinson 2020-08-18 21:26:10 UTC
This was released in libvirt 6.5.0 which is in f33+