6:56 p.m.
On 09/19/2017 02:19 PM, ashish mittal wrote:
Hi,
Not sure how to reply to the v8 series email thread, or how to add
myself to the libvirt email list! Updating the v7 thread instead.
There's some magic you can do with the right clients... I'll remember to
include you on the next series.
I have already updated with the results of TLS testing by statically
adding TLS enabled VxHS devices to domain XML, booting up the guest, and
making sure devices are properly readable/writable.
I did the same test using hot plug. libvirtd crashes when adding TLS
device -
[root@audi Sources] 2017-09-18 19:54:27# virsh attach-device myfc24
hotplug_disk_2.xml
error: Disconnected from qemu:///system due to end of file
error: Failed to attach device from hotplug_disk_2.xml
error: End of file while reading data: Input/output error
gdb reports -
Thread 3 "libvirtd" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f97e6b99700 (LWP 20096)]
0x00007f97c9c42a3d in qemuDomainGetTLSObjects (qemuCaps=0x7f97c00fcca0,
secinfo=secinfo@entry=0x0, tlsCertdir=0x7f97c00ff000
"/etc/pki/libvirt-vxhs",
tlsListen=tlsListen@entry=false, tlsVerify=tlsVerify@entry=true,
srcAlias=0x7f97b8001e00 "virtio-disk0", tlsProps=0x7f97e6b988f0,
tlsAlias=0x7f97e6b98998, secProps=0x0,
secAlias=0x0) at qemu/qemu_hotplug.c:1736
1736 }
Reason for crash seems to be a latent issue with
qemuDomainGetTLSObjects(). Both qemuDomainGetTLSObjects() and
qemuBuildTLSx509BackendProps() suggest that char **secAlias may be NULL,
but qemuDomainGetTLSObjects() dereferences it without checking for NULL.
int
qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
qemuDomainSecretInfoPtr secinfo,
const char *tlsCertdir,
bool tlsListen,
bool tlsVerify,
const char *srcAlias,
virJSONValuePtr *tlsProps,
char **tlsAlias,
virJSONValuePtr *secProps,
char **secAlias)
{
/* Add a secret object in order to access the TLS environment.
* The secinfo will only be created for serial TCP device. */
if (secinfo) {
if (qemuBuildSecretInfoProps(secinfo, secProps) < 0)
return -1;
if (!(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false)))
return -1;
}
if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
*secAlias, qemuCaps, tlsProps) < 0)
<=== *secAlias is dereferencing char **secAlias, which can be NULL
return -1;
...
}
Good thing we test before pushing!
The following diff takes care of the above issue.
[root@audi libvirt] 2017-09-18 23:19:31# git diff
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4c1074d..1448fe7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1715,6 +1715,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
virJSONValuePtr *secProps,
char **secAlias)
{
+ char *secretAlias = NULL;
+
/* Add a secret object in order to access the TLS environment.
* The secinfo will only be created for serial TCP device. */
if (secinfo) {
@@ -1723,10 +1725,11 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
if (!(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false)))
return -1;
+ secretAlias = *secAlias;
}
if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
- *secAlias, qemuCaps, tlsProps) < 0)
+ secretAlias, qemuCaps, tlsProps) < 0)
return -1;
if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias)))
But it's not really right... it's OK to have NULL secalias - we just
have to handle it better. There's no need to create a local for it. I
think if you do something that turns "*secalias," into "secalias ?
*secalias : NULL," that'd probably be better. it it works, could you
just post a separate patch to fix that issue?
I was able to attach and detach two TLS enabled VxHS disks to a
running
guest multiple times with the above fix in place. Did a attach/detach of
these disks back-to-back without any problems. Was able to format these
disks and copy files to both these TLS hot plugged disks from within the
guest without any problems. Will send detailed test logs in a separate
email.
In the v8 review, Peter would like some adjustments which I'm in the
process of making. I'm going to push the first part of the series, but
repost the TLS code as a v9.
Tks -
John
[...]