[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 1/2] vhost-user-blk-test: fix Coverity open(2) false positives
From: |
Stefan Hajnoczi |
Subject: |
[PATCH v2 1/2] vhost-user-blk-test: fix Coverity open(2) false positives |
Date: |
Tue, 1 Jun 2021 16:57:54 +0100 |
Coverity checks that the file descriptor return value of open(2) is
checked and used. Normally this is helpful in identifying real bugs but
vhost-user-blk-test opens /dev/null as stdin and stdout after fork.
In this case we don't need to look at the return value because these
open(2) calls cannot fail in any reasonable environment. We already know
their return values ahead of time since we closed stdin and stdout
previously. open(2) is guaranteed to pick the lowest available fd
number.
Silence Coverity by introducing code that checks what we already know to
be true.
** CID 1453270: Resource leaks (RESOURCE_LEAK)
/qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
________________________________________________________________________________________________________
*** CID 1453270: Resource leaks (RESOURCE_LEAK)
/qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
914 * Close standard file descriptors so tap-driver.pl pipe
detects when
915 * our parent terminates.
916 */
917 close(0);
918 close(1);
919 open("/dev/null", O_RDONLY);
>>> CID 1453270: Resource leaks (RESOURCE_LEAK)
>>> Ignoring handle opened by "open("/dev/null", 1)" leaks it.
920 open("/dev/null", O_WRONLY);
921
922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
NULL);
923 exit(1);
924 }
925 g_string_free(storage_daemon_command, true);
** CID 1453269: Error handling issues (NEGATIVE_RETURNS)
/qemu/tests/qtest/vhost-user-blk-test.c: 829 in create_listen_socket()
________________________________________________________________________________________________________
*** CID 1453269: Error handling issues (NEGATIVE_RETURNS)
/qemu/tests/qtest/vhost-user-blk-test.c: 829 in create_listen_socket()
823 char *path;
824
825 /* No race because our pid makes the path unique */
826 path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
827 tmp_fd = mkstemp(path);
828 g_assert_cmpint(tmp_fd, >=, 0);
>>> CID 1453269: Error handling issues (NEGATIVE_RETURNS)
>>> "tmp_fd" is passed to a parameter that cannot be negative.
829 close(tmp_fd);
830 unlink(path);
831
832 *fd = qtest_socket_server(path);
833 g_test_queue_destroy(destroy_file, path);
834 return path;
** CID 1453268: (CHECKED_RETURN)
/qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
/qemu/tests/qtest/vhost-user-blk-test.c: 919 in start_vhost_user_blk()
________________________________________________________________________________________________________
*** CID 1453268: (CHECKED_RETURN)
/qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
914 * Close standard file descriptors so tap-driver.pl pipe
detects when
915 * our parent terminates.
916 */
917 close(0);
918 close(1);
919 open("/dev/null", O_RDONLY);
>>> CID 1453268: (CHECKED_RETURN)
>>> Calling "open("/dev/null", 1)" without checking return value. This
>>> library function may fail and return an error code. [Note: The source code
>>> implementation of the function has been overridden by a builtin model.]
920 open("/dev/null", O_WRONLY);
921
922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
NULL);
923 exit(1);
924 }
925 g_string_free(storage_daemon_command, true);
/qemu/tests/qtest/vhost-user-blk-test.c: 919 in start_vhost_user_blk()
913 /*
914 * Close standard file descriptors so tap-driver.pl pipe
detects when
915 * our parent terminates.
916 */
917 close(0);
918 close(1);
>>> CID 1453268: (CHECKED_RETURN)
>>> Calling "open("/dev/null", 0)" without checking return value. This
>>> library function may fail and return an error code. [Note: The source code
>>> implementation of the function has been overridden by a builtin model.]
919 open("/dev/null", O_RDONLY);
920 open("/dev/null", O_WRONLY);
921
922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
NULL);
923 exit(1);
924 }
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/qtest/vhost-user-blk-test.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/vhost-user-blk-test.c
b/tests/qtest/vhost-user-blk-test.c
index 8796c74ca4..c5c4667759 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -910,14 +910,18 @@ static void start_vhost_user_blk(GString *cmd_line, int
vus_instances,
storage_daemon_command->str);
pid_t pid = fork();
if (pid == 0) {
+ int fd;
+
/*
* Close standard file descriptors so tap-driver.pl pipe detects when
* our parent terminates.
*/
close(0);
+ fd = open("/dev/null", O_RDONLY);
+ g_assert_cmpint(fd, ==, 0);
close(1);
- open("/dev/null", O_RDONLY);
- open("/dev/null", O_WRONLY);
+ fd = open("/dev/null", O_WRONLY);
+ g_assert_cmpint(fd, ==, 1);
execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
exit(1);
--
2.31.1