[libvirt] [PATCH 0/4] Cleanups on vTPM encrytion patches
This series of patches cleans up the Coverity issues reported by John. Stefan Stefan Berger (4): tpm: Fix memory leak and use existing variable instead tests: Call virCommandFree() in cleanup section utils: Remove ATTRIBUTE_NONNULL(1) from virCommandSetSendBuffer conf: Remove ATTRIBUTE_NONNULL(2) from virDomainCheckDeviceChanges() src/conf/domain_conf.h | 3 +-- src/qemu/qemu_tpm.c | 4 ++-- src/util/vircommand.h | 2 +- tests/commandtest.c | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) -- 2.20.1
Use the existing variables rather then calling virTPMSwtpmXYZ(). Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 7efd635831..a8a4d734c0 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -508,7 +508,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("%s does not support passing a passphrase using a file " - "descriptor"), virTPMGetSwtpmSetup()); + "descriptor"), swtpm_setup); goto cleanup; } if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0) @@ -648,7 +648,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("%s does not support passing passphrase via file descriptor"), - virTPMGetSwtpm()); + swtpm); goto error; } -- 2.20.1
Fix a potential memory leak by calling virCommandFree() in the cleanup section. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- tests/commandtest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index 991c0572b0..dfd15a2079 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1219,8 +1219,6 @@ static int test27(const void *unused ATTRIBUTE_UNUSED) goto cleanup; } - virCommandFree(cmd); - if (!outactual || !erractual) goto cleanup; @@ -1236,6 +1234,7 @@ static int test27(const void *unused ATTRIBUTE_UNUSED) ret = 0; cleanup: + virCommandFree(cmd); VIR_FORCE_CLOSE(pipe1[0]); VIR_FORCE_CLOSE(pipe2[0]); VIR_FORCE_CLOSE(pipe1[1]); -- 2.20.1
On Fri, Jul 26, 2019 at 09:42:44AM -0400, Stefan Berger wrote:
Fix a potential memory leak by calling virCommandFree() in the cleanup section.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- tests/commandtest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c index 991c0572b0..dfd15a2079 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1219,8 +1219,6 @@ static int test27(const void *unused ATTRIBUTE_UNUSED) goto cleanup; }
- virCommandFree(cmd); - if (!outactual || !erractual) goto cleanup;
@@ -1236,6 +1234,7 @@ static int test27(const void *unused ATTRIBUTE_UNUSED) ret = 0;
cleanup: + virCommandFree(cmd); VIR_FORCE_CLOSE(pipe1[0]); VIR_FORCE_CLOSE(pipe2[0]); VIR_FORCE_CLOSE(pipe1[1]);
We have the VIR_AUTO* macros you can use to get the compiler to free it automatically when it goes out of scope. Just declare the variable as: VIR_AUTOPTR(virCommand) cmd = NULL; Jano
Remove the ATTRIBUTE_NONNULL(1) from virCommandSetSendBuffer() prototype since we are checking for '!cmd'. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircommand.h b/src/util/vircommand.h index c2abc7b2c3..74574e3fb1 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -149,7 +149,7 @@ void virCommandSetWorkingDirectory(virCommandPtr cmd, int virCommandSetSendBuffer(virCommandPtr cmd, int fd, unsigned char *buffer, size_t buflen) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(3); void virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf) ATTRIBUTE_NONNULL(2); -- 2.20.1
On 7/26/19 9:42 AM, Stefan Berger wrote:
Remove the ATTRIBUTE_NONNULL(1) from virCommandSetSendBuffer() prototype since we are checking for '!cmd'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Coverity will complain that virCommandSetSendBuffer has: size_t i = virCommandGetNumSendBuffers(cmd); if (!cmd || cmd->has_error) return -1; Since @cmd is dereffed in virCommandGetNumSendBuffers Just move the initialization to after the return -1; and things will be good. John
diff --git a/src/util/vircommand.h b/src/util/vircommand.h index c2abc7b2c3..74574e3fb1 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -149,7 +149,7 @@ void virCommandSetWorkingDirectory(virCommandPtr cmd, int virCommandSetSendBuffer(virCommandPtr cmd, int fd, unsigned char *buffer, size_t buflen) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(3);
void virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf) ATTRIBUTE_NONNULL(2);
Since we are checking the 2nd parameter in the function for NULL, we need to remove ATTRIBUTE_NONNULL(2) from the prototype. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 285fa6c496..dc480bc7c5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3638,5 +3638,4 @@ bool virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics); int -virDomainCheckDeviceChanges(virDomainDefPtr def, virDomainDefPtr newDef) - ATTRIBUTE_NONNULL(2); +virDomainCheckDeviceChanges(virDomainDefPtr def, virDomainDefPtr newDef); -- 2.20.1
On 7/26/19 9:42 AM, Stefan Berger wrote:
This series of patches cleans up the Coverity issues reported by John.
Stefan
Stefan Berger (4): tpm: Fix memory leak and use existing variable instead tests: Call virCommandFree() in cleanup section utils: Remove ATTRIBUTE_NONNULL(1) from virCommandSetSendBuffer conf: Remove ATTRIBUTE_NONNULL(2) from virDomainCheckDeviceChanges()
src/conf/domain_conf.h | 3 +-- src/qemu/qemu_tpm.c | 4 ++-- src/util/vircommand.h | 2 +- tests/commandtest.c | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-)
With the cleanup in patch3 Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John
participants (3)
-
John Ferlan -
Ján Tomko -
Stefan Berger