[libvirt PATCH 0/2] Coverity inspired fixes for qemuPasstStart
Jiri Denemark (2): qemu: Don't check pidfile in qemuPasstStart qemu: Change some gotos in qemuPasstStart to direct return src/qemu/qemu_passt.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) -- 2.39.0
The pidfile is guaranteed to be non-NULL (thanks to glib allocation functions) and it's dereferenced two lines above anyway. Reported by coverity: /src/qemu/qemu_passt.c: 278 in qemuPasstStart() 272 return 0; 273 274 error: 275 ignore_value(virPidFileReadPathIfLocked(pidfile, &pid)); 276 if (pid != -1) 277 virProcessKillPainfully(pid, true); >>> CID 404360: Null pointer dereferences (REVERSE_INULL) >>> Null-checking "pidfile" suggests that it may be null, but it >>> has already been dereferenced on all paths leading to the check. 278 if (pidfile) 279 unlink(pidfile); 280 281 return -1; Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_passt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index 3355f7b2fa..5493eee494 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -275,8 +275,7 @@ qemuPasstStart(virDomainObj *vm, ignore_value(virPidFileReadPathIfLocked(pidfile, &pid)); if (pid != -1) virProcessKillPainfully(pid, true); - if (pidfile) - unlink(pidfile); + unlink(pidfile); return -1; } -- 2.39.0
Jumping to the error label and reading the pidfile does not make sense until we reached qemuSecurityCommandRun which creates the pidfile. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_passt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index 5493eee494..a7ee841af7 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -209,14 +209,14 @@ qemuPasstStart(virDomainObj *vm, /* validation guarantees this will never happen */ virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid portForward proto value %u"), pf->proto); - goto error; + return -1; } if (VIR_SOCKET_ADDR_VALID(&pf->address)) { g_autofree char *addr = NULL; if (!(addr = virSocketAddrFormat(&pf->address))) - goto error; + return -1; virBufferAddStr(&buf, addr); @@ -258,7 +258,7 @@ qemuPasstStart(virDomainObj *vm, if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) - goto error; + return -1; if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0) goto error; -- 2.39.0
On Wed, Jan 11, 2023 at 10:44:57AM +0100, Jiri Denemark wrote:
Jiri Denemark (2): qemu: Don't check pidfile in qemuPasstStart qemu: Change some gotos in qemuPasstStart to direct return
src/qemu/qemu_passt.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
-- 2.39.0
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety -
Jiri Denemark