qemu_tpm: Don't crash if qemuTPMPcrBankBitmapToStr(NULL)

[PATCH] qemu_tpm: Don't crash if qemuTPMPcrBankBitmapToStr(NULL)

Posted by Michal Privoznik 1 year, 8 months ago

Historically, the tpm->data.emulator.activePcrBanks member was an
unsigned int but since it was used as a bitmap it was converted
to virBitmap type instead. Now, the virBitmap is allocated inside
of virDomainTPMDefParseXML() but only if <activePcrBanks/> was
found with at last one child element. Otherwise it stays NULL.

Fast forward to starting a domain with TPM 2.0 and no
<activePcrBanks/> configured. Eventually,
qemuTPMEmulatorBuildCommand() is called, which subsequently calls
qemuTPMEmulatorReconfigure() and finally
qemuTPMPcrBankBitmapToStr() passing the NULL value. Before
rewrite to virBitmap this function would return NULL for empty
activePcrBanks but now, well, now it crashes.

Fixes: 52c7c31c8038aa31d502f59a40e4fb4ba9f61113
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_tpm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index c08b0851da..584c787b70 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -449,6 +449,9 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
     ssize_t bank = -1;
 
+    if (!activePcrBanks)
+        return NULL;
+
     while ((bank = virBitmapNextSetBit(activePcrBanks, bank)) > -1)
         virBufferAsprintf(&buf, "%s,", virDomainTPMPcrBankTypeToString(bank));
 
-- 
2.35.1

Re: [PATCH] qemu_tpm: Don't crash if qemuTPMPcrBankBitmapToStr(NULL)

Posted by Ján Tomko 1 year, 8 months ago

On a Thursday in 2022, Michal Privoznik wrote:
>Historically, the tpm->data.emulator.activePcrBanks member was an
>unsigned int but since it was used as a bitmap it was converted
>to virBitmap type instead. Now, the virBitmap is allocated inside
>of virDomainTPMDefParseXML() but only if <activePcrBanks/> was
>found with at last one child element. Otherwise it stays NULL.
>
>Fast forward to starting a domain with TPM 2.0 and no
><activePcrBanks/> configured. Eventually,
>qemuTPMEmulatorBuildCommand() is called, which subsequently calls
>qemuTPMEmulatorReconfigure() and finally
>qemuTPMPcrBankBitmapToStr() passing the NULL value. Before
>rewrite to virBitmap this function would return NULL for empty
>activePcrBanks but now, well, now it crashes.
>
>Fixes: 52c7c31c8038aa31d502f59a40e4fb4ba9f61113
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_tpm.c | 3 +++
> 1 file changed, 3 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano