8:36 a.m.
There are a few other issues that Coverity found, but I believe
all can be ignored ... except one:
We don't set umask anywhere inside nbdkit. Coverity complains that
this is a problem where we create temporary files, since the result of
mkstemp depends implicitly on the umask value. I think we might
consider setting umask anyway (eg. to 022) just to make plugin
behaviour more predictable. What do you think?
Rich.
8:36 a.m.
New subject: [PATCH nbdkit 1/2] plugins: nbd: Free h (handle) along error paths.
Found by Coverity.
---
plugins/nbd/nbd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index b9e72bc..2b5569b 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -465,6 +465,7 @@ nbd_open (int readonly)
h->fd = socket (AF_UNIX, SOCK_STREAM, 0);
if (h->fd < 0) {
nbdkit_error ("socket: %m");
+ free (h);
return NULL;
}
/* We already validated length during nbd_config_complete */
@@ -559,6 +560,7 @@ nbd_open (int readonly)
err:
close (h->fd);
+ free (h);
return NULL;
}
--
2.16.2
4:04 p.m.
New subject: [PATCH nbdkit 1/2] plugins: nbd: Free h (handle) along error paths.
On 06/14/2018 08:36 AM, Richard W.M. Jones wrote:
Found by Coverity.
---
plugins/nbd/nbd.c | 2 ++
1 file changed, 2 insertions(+)
ACK.
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index b9e72bc..2b5569b 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -465,6 +465,7 @@ nbd_open (int readonly)
h->fd = socket (AF_UNIX, SOCK_STREAM, 0);
if (h->fd < 0) {
nbdkit_error ("socket: %m");
+ free (h);
return NULL;
}
/* We already validated length during nbd_config_complete */
@@ -559,6 +560,7 @@ nbd_open (int readonly)
err:
close (h->fd);
+ free (h);
return NULL;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
8:36 a.m.
New subject: [PATCH nbdkit 2/2] crypto: Fix error path when sending to gnutls socket.
Found by Coverity.
---
src/crypto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/crypto.c b/src/crypto.c
index 17a667b..23c5c8f 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -294,7 +294,7 @@ crypto_send (struct connection *conn, const void *vbuf, size_t len)
while (len > 0) {
r = gnutls_record_send (*session, buf, len);
- if (r == -1) {
+ if (r < 0) {
if (r == GNUTLS_E_INTERRUPTED || r == GNUTLS_E_AGAIN)
continue;
return -1;
--
2.16.2
4:06 p.m.
On 06/14/2018 08:36 AM, Richard W.M. Jones wrote:
There are a few other issues that Coverity found, but I believe
all can be ignored ... except one:
We don't set umask anywhere inside nbdkit. Coverity complains that
this is a problem where we create temporary files, since the result of
mkstemp depends implicitly on the umask value. I think we might
consider setting umask anyway (eg. to 022) just to make plugin
behaviour more predictable. What do you think?
Setting umask() is not threadsafe - it must be done up front before any
threads can be created (and is therefore unsafe to do in a library that
might be linked into a larger multithreaded program). But setting a
sane umask up front seems reasonable to me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
2282
days inactive
2286
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Richard W.M. Jones