From 9d7a4c6690ef9962a3b20034f65008f1ea15c1d6 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 22 Jan 2015 11:21:37 +0100
Subject: [PATCH 1/4] coverity: Improve model for GLib memory allocation
In current versions of GLib, g_new() may expand into g_malloc_n().
When it does, Coverity can't see the memory allocation, because we
don't model g_malloc_n(). Similarly for g_new0(), g_renew(),
g_try_new(), g_try_new0(), g_try_renew().
Model g_malloc_n(), g_malloc0_n(), g_realloc_n(). Model
g_try_malloc_n(), g_try_malloc0_n(), g_try_realloc_n() by adding
indeterminate out of memory conditions on top.
To avoid undue duplication, replace the existing models for g_malloc()
& friends by trivial wrappers around g_malloc_n() & friends.
In a local scan, this flags four additional RESOURCE_LEAKs and one
NULL_RETURNS.
The NULL_RETURNS is a false positive: Coverity can now see that
g_try_malloc(l1_sz * sizeof(uint64_t)) in
qcow2_check_metadata_overlap() may return NULL, but is too stupid to
recognize that a loop executing l1_sz times won't be entered then.
Three out of the four RESOURCE_LEAKs appear genuine. The false
positive is in ppce500_prep_device_tree(): the pointer dies, but a
pointer to a struct member escapes, and we get the pointer back for
freeing with container_of(). Too funky for Coverity.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/coverity-model.c | 139 +++++++++++++++++++++++++++++----------
1 file changed, 104 insertions(+), 35 deletions(-)
diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 4c99a85cfc..8d0839ef08 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -90,7 +90,8 @@ static int get_keysym(const name2keysym_t *table,
}
}
-/* glib memory allocation functions.
+/*
+ * GLib memory allocation functions.
*
* Note that we ignore the fact that g_malloc of 0 bytes returns NULL,
* and g_realloc of 0 bytes frees the pointer.
@@ -107,60 +108,128 @@ static int get_keysym(const name2keysym_t *table,
* we'll get a buffer overflow reported anyway.
*/
-void *malloc(size_t);
-void *calloc(size_t, size_t);
-void *realloc(void *, size_t);
-void free(void *);
+/*
+ * Allocation primitives, cannot return NULL
+ * See also Coverity's library/generic/libc/all/all.c
+ */
-void *
-g_malloc(size_t n_bytes)
+void *g_malloc_n(size_t nmemb, size_t size)
{
- void *mem;
- __coverity_negative_sink__(n_bytes);
- mem = malloc(n_bytes == 0 ? 1 : n_bytes);
- if (!mem) __coverity_panic__();
- return mem;
+ size_t sz;
+ void *ptr;
+
+ __coverity_negative_sink__(nmemb);
+ __coverity_negative_sink__(size);
+ sz = nmemb * size;
+ ptr = __coverity_alloc__(size);
+ __coverity_mark_as_uninitialized_buffer__(ptr);
+ __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ return ptr;
}
-void *
-g_malloc0(size_t n_bytes)
+void *g_malloc0_n(size_t nmemb, size_t size)
{
- void *mem;
- __coverity_negative_sink__(n_bytes);
- mem = calloc(1, n_bytes == 0 ? 1 : n_bytes);
- if (!mem) __coverity_panic__();
- return mem;
+ size_t sz;
+ void *ptr;
+
+ __coverity_negative_sink__(nmemb);
+ __coverity_negative_sink__(size);
+ sz = nmemb * size;
+ ptr = __coverity_alloc__(size);
+ __coverity_writeall0__(ptr);
+ __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ return ptr;
}
-void g_free(void *mem)
+void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
{
- free(mem);
+ size_t sz;
+
+ __coverity_negative_sink__(nmemb);
+ __coverity_negative_sink__(size);
+ sz = nmemb * size;
+ __coverity_escape__(ptr);
+ ptr = __coverity_alloc__(size);
+ /*
+ * Memory beyond the old size isn't actually initialized. Can't
+ * model that. See Coverity's realloc() model
+ */
+ __coverity_writeall__(ptr);
+ __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ return ptr;
}
-void *g_realloc(void * mem, size_t n_bytes)
+void g_free(void *ptr)
{
- __coverity_negative_sink__(n_bytes);
- mem = realloc(mem, n_bytes == 0 ? 1 : n_bytes);
- if (!mem) __coverity_panic__();
- return mem;
+ __coverity_free__(ptr);
+ __coverity_mark_as_afm_freed__(ptr, AFM_free);
}
-void *g_try_malloc(size_t n_bytes)
+/*
+ * Derive the g_try_FOO_n() from the g_FOO_n() by adding indeterminate
+ * out of memory conditions
+ */
+
+void *g_try_malloc_n(size_t nmemb, size_t size)
{
- __coverity_negative_sink__(n_bytes);
- return malloc(n_bytes == 0 ? 1 : n_bytes);
+ int nomem;
+
+ if (nomem) {
+ return NULL;
+ }
+ return g_malloc_n(nmemb, size);
}
-void *g_try_malloc0(size_t n_bytes)
+void *g_try_malloc0_n(size_t nmemb, size_t size)
{
- __coverity_negative_sink__(n_bytes);
- return calloc(1, n_bytes == 0 ? 1 : n_bytes);
+ int nomem;
+
+ if (nomem) {
+ return NULL;
+ }
+ return g_malloc0_n(nmemb, size);
}
-void *g_try_realloc(void *mem, size_t n_bytes)
+void *g_try_realloc_n(void *ptr, size_t nmemb, size_t size)
{
- __coverity_negative_sink__(n_bytes);
- return realloc(mem, n_bytes == 0 ? 1 : n_bytes);
+ int nomem;
+
+ if (nomem) {
+ return NULL;
+ }
+ return g_realloc_n(ptr, nmemb, size);
+}
+
+/* Trivially derive the g_FOO() from the g_FOO_n() */
+
+void *g_malloc(size_t size)
+{
+ return g_malloc_n(1, size);
+}
+
+void *g_malloc0(size_t size)
+{
+ return g_malloc0_n(1, size);
+}
+
+void *g_realloc(void *ptr, size_t size)
+{
+ return g_realloc_n(ptr, 1, size);
+}
+
+void *g_try_malloc(size_t size)
+{
+ return g_try_malloc_n(1, size);
+}
+
+void *g_try_malloc0(size_t size)
+{
+ return g_try_malloc0_n(1, size);
+}
+
+void *g_try_realloc(void *ptr, size_t size)
+{
+ return g_try_realloc_n(ptr, 1, size);
}
/* Other glib functions */
From e4b77daa5724a9dd41aaa44d2dea4b8e92351081 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 26 Jan 2015 15:05:11 +0100
Subject: [PATCH 2/4] coverity: Model GLib string allocation partially
Without a model, Coverity can't know that the result of g_strdup()
needs to be fed to g_free().
One way to get such a model is to scan GLib, build a derived model
file with cov-collect-models, and use that when scanning QEMU.
Unfortunately, the Coverity Scan service we use doesn't support that.
Thus, we're stuck with the other way: write a user model. Doing that
for all of GLib is hardly practical. I'm doing it for the "String
Utility Functions" we actually use that return dynamically allocated
strings.
In a local scan, this flags 20 additional RESOURCE_LEAKs. The ones I
checked look genuine.
It also loses a NULL_RETURNS about ppce500_init() using
qemu_find_file() without error checking. I don't understand why.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/coverity-model.c | 89 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 8d0839ef08..230bc30cf9 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -40,6 +40,8 @@ typedef unsigned long long uint64_t;
typedef long long int64_t;
typedef _Bool bool;
+typedef struct va_list_str *va_list;
+
/* exec.c */
typedef struct AddressSpace AddressSpace;
@@ -232,6 +234,93 @@ void *g_try_realloc(void *ptr, size_t size)
return g_try_realloc_n(ptr, 1, size);
}
+/*
+ * GLib string allocation functions
+ */
+
+char *g_strdup(const char *s)
+{
+ char *dup;
+ size_t i;
+
+ if (!s) {
+ return NULL;
+ }
+
+ __coverity_string_null_sink__(s);
+ __coverity_string_size_sink__(s);
+ dup = __coverity_alloc_nosize__();
+ __coverity_mark_as_afm_allocated__(dup, AFM_free);
+ for (i = 0; (dup[i] = s[i]); i++) ;
+ return dup;
+}
+
+char *g_strndup(const char *s, size_t n)
+{
+ char *dup;
+ size_t i;
+
+ __coverity_negative_sink__(n);
+
+ if (!s) {
+ return NULL;
+ }
+
+ dup = g_malloc(n + 1);
+ for (i = 0; i < n && (dup[i] = s[i]); i++) ;
+ dup[i] = 0;
+ return dup;
+}
+
+char *g_strdup_printf(const char *format, ...)
+{
+ char ch, *s;
+ size_t len;
+
+ __coverity_string_null_sink__(format);
+ __coverity_string_size_sink__(format);
+
+ ch = *format;
+
+ s = __coverity_alloc_nosize__();
+ __coverity_writeall__(s);
+ __coverity_mark_as_afm_allocated__(s, AFM_free);
+ return s;
+}
+
+char *g_strdup_vprintf(const char *format, va_list ap)
+{
+ char ch, *s;
+ size_t len;
+
+ __coverity_string_null_sink__(format);
+ __coverity_string_size_sink__(format);
+
+ ch = *format;
+ ch = *(char *)ap;
+
+ s = __coverity_alloc_nosize__();
+ __coverity_writeall__(s);
+ __coverity_mark_as_afm_allocated__(s, AFM_free);
+
+ return len;
+}
+
+char *g_strconcat(const char *s, ...)
+{
+ char *s;
+
+ /*
+ * Can't model: last argument must be null, the others
+ * null-terminated strings
+ */
+
+ s = __coverity_alloc_nosize__();
+ __coverity_writeall__(s);
+ __coverity_mark_as_afm_allocated__(s, AFM_free);
+ return s;
+}
+
/* Other glib functions */
typedef struct _GIOChannel GIOChannel;
From 7ad4c7200111d20eb97eed4f46b6026e3f0b0eef Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 26 Jan 2015 21:37:15 +0100
Subject: [PATCH 3/4] coverity: Model g_free() isn't necessarily free()
Memory allocated with GLib needs to be freed with GLib. Freeing it
with free() instead of g_free() is a common error. Harmless when
g_free() is a trivial wrapper around free(), which is commonly the
case. But model the difference anyway.
In a local scan, this flags four ALLOC_FREE_MISMATCH. Requires
--enable ALLOC_FREE_MISMATCH, because the checker is still preview.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/coverity-model.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 230bc30cf9..58356afa66 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -125,7 +125,7 @@ void *g_malloc_n(size_t nmemb, size_t size)
sz = nmemb * size;
ptr = __coverity_alloc__(size);
__coverity_mark_as_uninitialized_buffer__(ptr);
- __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ __coverity_mark_as_afm_allocated__(ptr, "g_free");
return ptr;
}
@@ -139,7 +139,7 @@ void *g_malloc0_n(size_t nmemb, size_t size)
sz = nmemb * size;
ptr = __coverity_alloc__(size);
__coverity_writeall0__(ptr);
- __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ __coverity_mark_as_afm_allocated__(ptr, "g_free");
return ptr;
}
@@ -157,14 +157,14 @@ void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
* model that. See Coverity's realloc() model
*/
__coverity_writeall__(ptr);
- __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+ __coverity_mark_as_afm_allocated__(ptr, "g_free");
return ptr;
}
void g_free(void *ptr)
{
__coverity_free__(ptr);
- __coverity_mark_as_afm_freed__(ptr, AFM_free);
+ __coverity_mark_as_afm_freed__(ptr, "g_free");
}
/*
@@ -250,7 +250,7 @@ char *g_strdup(const char *s)
__coverity_string_null_sink__(s);
__coverity_string_size_sink__(s);
dup = __coverity_alloc_nosize__();
- __coverity_mark_as_afm_allocated__(dup, AFM_free);
+ __coverity_mark_as_afm_allocated__(dup, "g_free");
for (i = 0; (dup[i] = s[i]); i++) ;
return dup;
}
@@ -284,7 +284,7 @@ char *g_strdup_printf(const char *format, ...)
s = __coverity_alloc_nosize__();
__coverity_writeall__(s);
- __coverity_mark_as_afm_allocated__(s, AFM_free);
+ __coverity_mark_as_afm_allocated__(s, "g_free");
return s;
}
@@ -301,7 +301,7 @@ char *g_strdup_vprintf(const char *format, va_list ap)
s = __coverity_alloc_nosize__();
__coverity_writeall__(s);
- __coverity_mark_as_afm_allocated__(s, AFM_free);
+ __coverity_mark_as_afm_allocated__(s, "g_free");
return len;
}
@@ -317,7 +317,7 @@ char *g_strconcat(const char *s, ...)
s = __coverity_alloc_nosize__();
__coverity_writeall__(s);
- __coverity_mark_as_afm_allocated__(s, AFM_free);
+ __coverity_mark_as_afm_allocated__(s, "g_free");
return s;
}
From 8c413e7902ef0c19ced516f575db989ccc3785f8 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 28 Jan 2015 11:29:57 +0100
Subject: [PATCH 4/4] MAINTAINERS: Add myself as Coverity model maintainer
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index fd335a47bf..b68cb7e133 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -783,6 +783,11 @@ M: Samuel Thibault <samuel.thibault@ens-lyon.org>
S: Maintained
F: backends/baum.c
+Coverity model
+M: Markus Armbruster <armbru@redhat.com>
+S: Supported
+F: scripts/coverity-model.c
+
CPU
M: Andreas Färber <afaerber@suse.de>
S: Supported