[libvirt] [PATCH libvirt] qemu: Fix segmentation fault when attaching a non iSCSI host device

Marc Hartmayer posted 1 patch 6 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180117122608.8668-1-mhartmay@linux.vnet.ibm.com
src/qemu/qemu_hotplug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[libvirt] [PATCH libvirt] qemu: Fix segmentation fault when attaching a non iSCSI host device
Posted by Marc Hartmayer 6 years, 3 months ago
Add a check if it's a iSCSI hostdev and if it's not then don't use the
union member 'iscsi'. The segmentation fault occured when accessing
secinfo->type, but this can vary from case to case.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
---
 src/qemu/qemu_hotplug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6dc16a1054af..83d0e1c71a8e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2343,8 +2343,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
     bool secobjAdded = false;
     virJSONValuePtr secobjProps = NULL;
     virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
-    virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
-    qemuDomainStorageSourcePrivatePtr srcPriv;
+    qemuDomainStorageSourcePrivatePtr srcPriv = NULL;
     qemuDomainSecretInfoPtr secinfo = NULL;
 
     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
@@ -2386,7 +2385,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
     if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0)
         goto cleanup;
 
-    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
+    if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
+        srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(scsisrc->u.iscsi.src);
     if (srcPriv)
         secinfo = srcPriv->secinfo;
     if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
-- 
2.13.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] qemu: Fix segmentation fault when attaching a non iSCSI host device
Posted by John Ferlan 6 years, 3 months ago

On 01/17/2018 07:26 AM, Marc Hartmayer wrote:
> Add a check if it's a iSCSI hostdev and if it's not then don't use the
> union member 'iscsi'. The segmentation fault occured when accessing
> secinfo->type, but this can vary from case to case.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_hotplug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Thanks - thought I got this with '6050affb7', but right it must be
protocol iSCSI <sigh>.

Reviewed-by: John Ferlan <jferlan@redhat.com>

(and safe for freeze - I'll push shortly),

John

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6dc16a1054af..83d0e1c71a8e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2343,8 +2343,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>      bool secobjAdded = false;
>      virJSONValuePtr secobjProps = NULL;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
> -    virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
> -    qemuDomainStorageSourcePrivatePtr srcPriv;
> +    qemuDomainStorageSourcePrivatePtr srcPriv = NULL;
>      qemuDomainSecretInfoPtr secinfo = NULL;
>  
>      if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
> @@ -2386,7 +2385,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>      if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0)
>          goto cleanup;
>  
> -    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
> +    if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
> +        srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(scsisrc->u.iscsi.src);
>      if (srcPriv)
>          secinfo = srcPriv->secinfo;

Probably could combine into one if statement...


>      if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] qemu: Fix segmentation fault when attaching a non iSCSI host device
Posted by Marc Hartmayer 6 years, 3 months ago
On Wed, Jan 17, 2018 at 02:39 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
> On 01/17/2018 07:26 AM, Marc Hartmayer wrote:
>> Add a check if it's a iSCSI hostdev and if it's not then don't use the
>> union member 'iscsi'. The segmentation fault occured when accessing
>> secinfo->type, but this can vary from case to case.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
>> ---
>>  src/qemu/qemu_hotplug.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>
> Thanks - thought I got this with '6050affb7', but right it must be
> protocol iSCSI <sigh>.
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
>
> (and safe for freeze - I'll push shortly),

Thanks.

>
> John
>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 6dc16a1054af..83d0e1c71a8e 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -2343,8 +2343,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>>      bool secobjAdded = false;
>>      virJSONValuePtr secobjProps = NULL;
>>      virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
>> -    virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
>> -    qemuDomainStorageSourcePrivatePtr srcPriv;
>> +    qemuDomainStorageSourcePrivatePtr srcPriv = NULL;
>>      qemuDomainSecretInfoPtr secinfo = NULL;
>>
>>      if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
>> @@ -2386,7 +2385,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>>      if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0)
>>          goto cleanup;
>>
>> -    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
>> +    if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
>> +        srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(scsisrc->u.iscsi.src);
>>      if (srcPriv)
>>          secinfo = srcPriv->secinfo;
>
> Probably could combine into one if statement...

Yep.

>
>
>>      if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
>>
>
--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list