Bug 1814923

For the usage corrupted issue, it can be reproduced by file, iscsi backend with luks usage.
Not reproduced by gluster with luks uuid.

Does the crash happen if you start a clean libvirtd and retry the steps just once? I've identified a double-free regarding the 'usage' string, but that manifests itself only after you destroy the VM and start it again.

(In reply to Peter Krempa from comment #2)
> Does the crash happen if you start a clean libvirtd and retry the steps just
> once? I've identified a double-free regarding the 'usage' string, but that
> manifests itself only after you destroy the VM and start it again.

The crash will not happen when following 1~5 with a fresh libvirtd:
➜  luks virsh start new           
Domain new started

➜  luks virsh blockcopy new sda --xml file-dest.xml --pivot --transient-job --verbose --wait --reuse-external
Block Copy: [100 %]
Successfully pivoted
➜  luks virsh destroy new                                                                                    
Domain new destroyed

➜  luks virsh dumpxml new |xmllint --xpath //disk -
-:48: parser error : Input is not proper UTF-8, indicate encoding !
Bytes: 0xC0 0x94 0xBB 0x7F
        <secret type='passphrase' usage=''/>

I have installed rpm of v6.1.0-190-gaeb909bf9b, which includes:
commit 38bc76bcc1e1bccb7f3265e15b60b0f6f8fe6dfa
Author: Peter Krempa <pkrempa>
Date:   Fri Mar 6 14:44:43 2020 +0100

    qemu: Don't take double pointer in qemuDomainSecretInfoFree
    
    Using a double pointer prevents the function from being used as the
    automatic cleanup function for the given type.
    
    Remove the double pointer use by replacing the calls with
    g_clear_pointer which ensures that the pointer is cleared.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>


But the error in comment3 could be reproduced again.

(In reply to Han Han from comment #3)
> (In reply to Peter Krempa from comment #2)
> > Does the crash happen if you start a clean libvirtd and retry the steps just
> > once? I've identified a double-free regarding the 'usage' string, but that
> > manifests itself only after you destroy the VM and start it again.
> 
> The crash will not happen when following 1~5 with a fresh libvirtd:
> ➜  luks virsh start new           
> Domain new started
> 
> ➜  luks virsh blockcopy new sda --xml file-dest.xml --pivot --transient-job
> --verbose --wait --reuse-external
> Block Copy: [100 %]
> Successfully pivoted
> ➜  luks virsh destroy new                                           


Thank you for confirming.
        
> 
> Domain new destroyed
> 
> ➜  luks virsh dumpxml new |xmllint --xpath //disk -
> -:48: parser error : Input is not proper UTF-8, indicate encoding !
> Bytes: 0xC0 0x94 0xBB 0x7F
>         <secret type='passphrase' usage=''/>

I've seen this symptom while debugging.

(In reply to Han Han from comment #4)
> I have installed rpm of v6.1.0-190-gaeb909bf9b, which includes:
> commit 38bc76bcc1e1bccb7f3265e15b60b0f6f8fe6dfa
> Author: Peter Krempa <pkrempa>
> Date:   Fri Mar 6 14:44:43 2020 +0100
> 
>     qemu: Don't take double pointer in qemuDomainSecretInfoFree
>     
>     Using a double pointer prevents the function from being used as the
>     automatic cleanup function for the given type.
>     
>     Remove the double pointer use by replacing the calls with
>     g_clear_pointer which ensures that the pointer is cleared.
>     
>     Signed-off-by: Peter Krempa <pkrempa>
>     Reviewed-by: Ján Tomko <jtomko>
> 
> 
> But the error in comment3 could be reproduced again.

That one was just a simplification of the code. The proper fix for the issue should be the patch I've posted for a wrong copy of the 'usage' string:

https://www.redhat.com/archives/libvir-list/2020-March/msg00693.html

Fixed upstream:

commit 299796328c34a30295d6cdc7ebce5d65843e921f
Author: Peter Krempa <pkrempa>
Date:   Thu Mar 19 15:38:06 2020 +0100

    virStorageEncryptionSecretCopy: Properly copy internals
    
    virStorageEncryptionSecretPtr may have a string inside it, thus we must
    copy the string too. Use virSecretLookupDefCopy to do that.
    
    Caused by non-obvious code introduced in 756b46ddd24 and later 47e88b33b
    which added a string that needed to be copied.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1814923
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>
    Reviewed-by: Michal Privoznik <mprivozn>

commit 02f909b8a6ad8c271693fed9ceab606f0dda2294
Author: Peter Krempa <pkrempa>
Date:   Thu Mar 19 15:27:40 2020 +0100

    virSecretLookupDefCopy: Remove return value
    
    The function always returns succes so there's no need for a return
    value.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>
    Reviewed-by: Michal Privoznik <mprivozn>

Vefified on libvirt-6.0.0-17.module+el8.2.0+6257+0d066c28.x86_64
qemu-kvm-4.2.0-17.module+el8.2.0+6141+0f540f16.x86_64:

1. Start a VM

2. Prepare luks secret and set the password

3. Blockcopy to a luks disk:
# cat /tmp/file-dest.xml 
<disk type="file" device="disk">
  <driver name="qemu" type="qcow2"/>
  <source file="/var/lib/libvirt/images/pc-luks">
    <encryption format="luks">
      <secret type="passphrase" usage="luks"/>
    </encryption>
  </source>
  <target dev="sda" bus="scsi"/>
</disk>

➜  ~ virsh blockcopy pc sda --xml /tmp/file-dest.xml --wait --verbose --transient-job
Block Copy: [100 %]

➜  ~ virsh blockjob pc sda --pivot  

➜  ~ virsh dumpxml pc | xmllint --xpath //disk -
<disk type="file" device="disk">
      <driver name="qemu" type="qcow2"/>
      <source file="/var/lib/libvirt/images/pc-luks" index="6">
        <encryption format="luks">
          <secret type="passphrase" usage="luks"/>
        </encryption>
      </source>
      <backingStore/>
      <target dev="sda" bus="scsi"/>
      <alias name="scsi0-0-0-0"/>
      <address type="drive" controller="0" bus="0" target="0" unit="0"/>
    </disk>

3. Destroy and start vm, check the vm xml again
➜  ~ virsh destroy pc                           
Domain pc destroyed

➜  ~ virsh start pc
Domain pc started

➜  ~ virsh dumpxml pc | xmllint --xpath //disk -
<disk type="file" device="disk">
      <driver name="qemu" type="qcow2"/>
      <source file="/var/lib/libvirt/images/pc-luks" index="2"/>
      <backingStore/>
      <target dev="sda" bus="scsi"/>
      <encryption format="luks">
        <secret type="passphrase" usage="luks"/>
      </encryption>
      <alias name="scsi0-0-0-0"/>
      <address type="drive" controller="0" bus="0" target="0" unit="0"/>
    </disk>

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