All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2
@ 2021-08-02 16:15 Paolo Bonzini
  2021-08-02 16:15 ` [PULL 1/8] vl: introduce machine_merge_property Paolo Bonzini
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 7742fe64e5c2c2c9f9787d107b693eaac602eaae:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20210729-pull-request' into staging (2021-07-29 18:49:39 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to e17bdaab2b36db54f0214a14f394fa773cee58df:

  coverity-model: write models fully for non-array allocation functions (2021-07-30 12:04:01 +0200)

----------------------------------------------------------------
Fix for smp-opts in configuration file.
Update Coverity model to what's currently uploaded.

----------------------------------------------------------------
Paolo Bonzini (8):
      vl: introduce machine_merge_property
      vl: stop recording -smp in QemuOpts
      coverity-model: update address_space_read/write models
      coverity-model: make g_free a synonym of free
      coverity-model: remove model for more allocation functions
      coverity-model: clean up the models for array allocation functions
      coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL
      coverity-model: write models fully for non-array allocation functions

 scripts/coverity-scan/model.c | 235 ++++++++++++++++++++----------------------
 softmmu/vl.c                  |  47 ++++++---
 2 files changed, 143 insertions(+), 139 deletions(-)
-- 
2.31.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PULL 1/8] vl: introduce machine_merge_property
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 2/8] vl: stop recording -smp in QemuOpts Paolo Bonzini
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

It will be used to parse smp-opts config groups from configuration
files.  The point to note is that it does not steal a reference
from the caller.  This is better because this function will be called
from qemu_config_foreach's callback; qemu_config_foreach does not cede
its reference to the qdict to the callback, and wants to free it.  To
balance that extra reference, machine_parse_property_opt now needs
a qobject_unref.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 4dee472c79..93aef8e747 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1534,23 +1534,36 @@ static void machine_help_func(const QDict *qdict)
     }
 }
 
+static void
+machine_merge_property(const char *propname, QDict *prop, Error **errp)
+{
+    QDict *opts;
+
+    opts = qdict_new();
+    /* Preserve the caller's reference to prop.  */
+    qobject_ref(prop);
+    qdict_put(opts, propname, prop);
+    keyval_merge(machine_opts_dict, opts, errp);
+    qobject_unref(opts);
+}
+
 static void
 machine_parse_property_opt(QemuOptsList *opts_list, const char *propname,
                            const char *arg, Error **errp)
 {
-    QDict *opts, *prop;
+    QDict *prop = NULL;
     bool help = false;
-    ERRP_GUARD();
 
     prop = keyval_parse(arg, opts_list->implied_opt_name, &help, errp);
     if (help) {
         qemu_opts_print_help(opts_list, true);
         exit(0);
     }
-    opts = qdict_new();
-    qdict_put(opts, propname, prop);
-    keyval_merge(machine_opts_dict, opts, errp);
-    qobject_unref(opts);
+    if (!prop) {
+        return;
+    }
+    machine_merge_property(propname, prop, errp);
+    qobject_unref(prop);
 }
 
 static const char *pid_file;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 2/8] vl: stop recording -smp in QemuOpts
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
  2021-08-02 16:15 ` [PULL 1/8] vl: introduce machine_merge_property Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 3/8] coverity-model: update address_space_read/write models Paolo Bonzini
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

-readconfig is still recording SMP options in QemuOpts instead of
using machine_opts_dict.  This means that SMP options from -readconfig
are ignored.

Just stop using QemuOpts for -smp, making it return false for
is_qemuopts_group.  Configuration files will merge the values in
machine_opts_dict using the new function machine_merge_property.

At the same time, fix -mem-prealloc which looked at QemuOpts to find the
number of guest CPUs, which it used as the default number of preallocation
threads.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 93aef8e747..5ca11e7469 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -31,6 +31,7 @@
 #include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu-version.h"
 #include "qemu/cutils.h"
@@ -2166,7 +2167,8 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
 static bool is_qemuopts_group(const char *group)
 {
     if (g_str_equal(group, "object") ||
-        g_str_equal(group, "machine")) {
+        g_str_equal(group, "machine") ||
+        g_str_equal(group, "smp-opts")) {
         return false;
     }
     return true;
@@ -2186,6 +2188,8 @@ static void qemu_record_config_group(const char *group, QDict *dict,
          */
         assert(!from_json);
         keyval_merge(machine_opts_dict, dict, errp);
+    } else if (g_str_equal(group, "smp-opts")) {
+        machine_merge_property("smp", dict, &error_fatal);
     } else {
         abort();
     }
@@ -2452,13 +2456,15 @@ static void qemu_validate_options(const QDict *machine_opts)
 static void qemu_process_sugar_options(void)
 {
     if (mem_prealloc) {
-        char *val;
-
-        val = g_strdup_printf("%d",
-                 (uint32_t) qemu_opt_get_number(qemu_find_opts_singleton("smp-opts"), "cpus", 1));
-        object_register_sugar_prop("memory-backend", "prealloc-threads", val,
-                                   false);
-        g_free(val);
+        QObject *smp = qdict_get(machine_opts_dict, "smp");
+        if (smp && qobject_type(smp) == QTYPE_QDICT) {
+            QObject *cpus = qdict_get(qobject_to(QDict, smp), "cpus");
+            if (cpus && qobject_type(cpus) == QTYPE_QSTRING) {
+                const char *val = qstring_get_str(qobject_to(QString, cpus));
+                object_register_sugar_prop("memory-backend", "prealloc-threads",
+                                           val, false);
+            }
+        }
         object_register_sugar_prop("memory-backend", "prealloc", "on", false);
     }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 3/8] coverity-model: update address_space_read/write models
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
  2021-08-02 16:15 ` [PULL 1/8] vl: introduce machine_merge_property Paolo Bonzini
  2021-08-02 16:15 ` [PULL 2/8] vl: stop recording -smp in QemuOpts Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 4/8] coverity-model: make g_free a synonym of free Paolo Bonzini
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

Use void * for consistency with the actual function; provide a model
for MemoryRegionCache functions and for address_space_rw.  These
let Coverity understand the bounds of the data that various functions
read and write even at very high levels of inlining (e.g. pci_dma_read).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 48 ++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 2c0346ff25..e1bdf0ad84 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -45,9 +45,10 @@ typedef struct va_list_str *va_list;
 /* exec.c */
 
 typedef struct AddressSpace AddressSpace;
+typedef struct MemoryRegionCache MemoryRegionCache;
 typedef uint64_t hwaddr;
 typedef uint32_t MemTxResult;
-typedef uint64_t MemTxAttrs;
+typedef struct MemTxAttrs {} MemTxAttrs;
 
 static void __bufwrite(uint8_t *buf, ssize_t len)
 {
@@ -67,9 +68,40 @@ static void __bufread(uint8_t *buf, ssize_t len)
     int last = buf[len-1];
 }
 
+MemTxResult address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
+                                      MemTxAttrs attrs,
+                                      void *buf, int len)
+{
+    MemTxResult result;
+    // TODO: investigate impact of treating reads as producing
+    // tainted data, with __coverity_tainted_data_argument__(buf).
+    __bufwrite(buf, len);
+    return result;
+}
+
+MemTxResult address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
+                                MemTxAttrs attrs,
+                                const void *buf, int len)
+{
+    MemTxResult result;
+    __bufread(buf, len);
+    return result;
+}
+
+MemTxResult address_space_rw_cached(MemoryRegionCache *cache, hwaddr addr,
+                                    MemTxAttrs attrs,
+                                    void *buf, int len, bool is_write)
+{
+    if (is_write) {
+        return address_space_write_cached(cache, addr, attrs, buf, len);
+    } else {
+        return address_space_read_cached(cache, addr, attrs, buf, len);
+    }
+}
+
 MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
                                MemTxAttrs attrs,
-                               uint8_t *buf, int len)
+                               void *buf, int len)
 {
     MemTxResult result;
     // TODO: investigate impact of treating reads as producing
@@ -80,13 +112,23 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
 
 MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
                                 MemTxAttrs attrs,
-                                const uint8_t *buf, int len)
+                                const void *buf, int len)
 {
     MemTxResult result;
     __bufread(buf, len);
     return result;
 }
 
+MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
+                             MemTxAttrs attrs,
+                             void *buf, int len, bool is_write)
+{
+    if (is_write) {
+        return address_space_write(as, addr, attrs, buf, len);
+    } else {
+        return address_space_read(as, addr, attrs, buf, len);
+    }
+}
 
 /* Tainting */
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 4/8] coverity-model: make g_free a synonym of free
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 3/8] coverity-model: update address_space_read/write models Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 5/8] coverity-model: remove model for more allocation functions Paolo Bonzini
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

Recently, Coverity has started complaining about using g_free() to free
memory areas allocated by GLib functions not included in model.c,
such as g_strfreev.  This unfortunately goes against the GLib
documentation, which suggests that g_malloc() should be matched
with g_free() and plain malloc() with free(); since GLib 2.46 however
g_malloc() is hardcoded to always use the system malloc implementation,
and g_free is just "free" plus a tracepoint.  Therefore, this
should not cause any problem in practice.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index e1bdf0ad84..8e64a84c5a 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -186,7 +186,7 @@ void *g_malloc_n(size_t nmemb, size_t size)
     sz = nmemb * size;
     ptr = __coverity_alloc__(sz);
     __coverity_mark_as_uninitialized_buffer__(ptr);
-    __coverity_mark_as_afm_allocated__(ptr, "g_free");
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
 }
 
@@ -200,7 +200,7 @@ void *g_malloc0_n(size_t nmemb, size_t size)
     sz = nmemb * size;
     ptr = __coverity_alloc__(sz);
     __coverity_writeall0__(ptr);
-    __coverity_mark_as_afm_allocated__(ptr, "g_free");
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
 }
 
@@ -218,14 +218,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, "g_free");
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
 }
 
 void g_free(void *ptr)
 {
     __coverity_free__(ptr);
-    __coverity_mark_as_afm_freed__(ptr, "g_free");
+    __coverity_mark_as_afm_freed__(ptr, AFM_free);
 }
 
 /*
@@ -328,7 +328,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, "g_free");
+    __coverity_mark_as_afm_allocated__(dup, AFM_free);
     for (i = 0; (dup[i] = s[i]); i++) ;
     return dup;
 }
@@ -362,7 +362,7 @@ char *g_strdup_printf(const char *format, ...)
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, "g_free");
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
     return s;
 }
 
@@ -375,11 +375,10 @@ char *g_strdup_vprintf(const char *format, va_list ap)
     __coverity_string_size_sink__(format);
 
     ch = *format;
-    ch = *(char *)ap;
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, "g_free");
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
 
     return len;
 }
@@ -395,7 +394,7 @@ char *g_strconcat(const char *s, ...)
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, "g_free");
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
     return s;
 }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 5/8] coverity-model: remove model for more allocation functions
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 4/8] coverity-model: make g_free a synonym of free Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 6/8] coverity-model: clean up the models for array " Paolo Bonzini
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

These models are not needed anymore now that Coverity does not check
anymore that the result is used with "g_free".  Coverity understands
GCC attributes and uses them to detect leaks.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 105 +---------------------------------
 1 file changed, 1 insertion(+), 104 deletions(-)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 8e64a84c5a..1a5f39d2ae 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -263,7 +263,7 @@ void *g_try_realloc_n(void *ptr, size_t nmemb, size_t size)
     return g_realloc_n(ptr, nmemb, size);
 }
 
-/* Trivially derive the g_FOO() from the g_FOO_n() */
+/* Derive the g_FOO() from the g_FOO_n() */
 
 void *g_malloc(size_t size)
 {
@@ -295,109 +295,6 @@ void *g_try_realloc(void *ptr, size_t size)
     return g_try_realloc_n(ptr, 1, size);
 }
 
-/* Other memory allocation functions */
-
-void *g_memdup(const void *ptr, unsigned size)
-{
-    unsigned char *dup;
-    unsigned i;
-
-    if (!ptr) {
-        return NULL;
-    }
-
-    dup = g_malloc(size);
-    for (i = 0; i < size; i++)
-        dup[i] = ((unsigned char *)ptr)[i];
-    return dup;
-}
-
-/*
- * 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;
-
-    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 pollfd GPollFD;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 6/8] coverity-model: clean up the models for array allocation functions
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 5/8] coverity-model: remove model for more allocation functions Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 7/8] coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL Paolo Bonzini
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

sz is only used in one place, so replace it with nmemb * size in
that one place.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 1a5f39d2ae..2d384bdd79 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -178,13 +178,11 @@ uint8_t replay_get_byte(void)
 
 void *g_malloc_n(size_t nmemb, size_t size)
 {
-    size_t sz;
     void *ptr;
 
     __coverity_negative_sink__(nmemb);
     __coverity_negative_sink__(size);
-    sz = nmemb * size;
-    ptr = __coverity_alloc__(sz);
+    ptr = __coverity_alloc__(nmemb * size);
     __coverity_mark_as_uninitialized_buffer__(ptr);
     __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
@@ -192,13 +190,11 @@ void *g_malloc_n(size_t nmemb, size_t size)
 
 void *g_malloc0_n(size_t nmemb, size_t size)
 {
-    size_t sz;
     void *ptr;
 
     __coverity_negative_sink__(nmemb);
     __coverity_negative_sink__(size);
-    sz = nmemb * size;
-    ptr = __coverity_alloc__(sz);
+    ptr = __coverity_alloc__(nmemb * size);
     __coverity_writeall0__(ptr);
     __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
@@ -206,13 +202,10 @@ void *g_malloc0_n(size_t nmemb, size_t size)
 
 void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
 {
-    size_t sz;
-
     __coverity_negative_sink__(nmemb);
     __coverity_negative_sink__(size);
-    sz = nmemb * size;
     __coverity_escape__(ptr);
-    ptr = __coverity_alloc__(sz);
+    ptr = __coverity_alloc__(nmemb * size);
     /*
      * Memory beyond the old size isn't actually initialized.  Can't
      * model that.  See Coverity's realloc() model
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 7/8] coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 6/8] coverity-model: clean up the models for array " Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 16:15 ` [PULL 8/8] coverity-model: write models fully for non-array allocation functions Paolo Bonzini
  2021-08-02 19:12 ` [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

g_malloc/g_malloc0/g_realloc only return NULL if the size is 0; we do not need
to cover that in the model, and so far have expected __coverity_alloc__
to model a non-NULL return value.  But that apparently does not work
anymore, so add some extra conditionals that invoke __coverity_panic__
for NULL pointers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 2d384bdd79..028f13e9e3 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -183,6 +183,9 @@ void *g_malloc_n(size_t nmemb, size_t size)
     __coverity_negative_sink__(nmemb);
     __coverity_negative_sink__(size);
     ptr = __coverity_alloc__(nmemb * size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
     __coverity_mark_as_uninitialized_buffer__(ptr);
     __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
@@ -195,6 +198,9 @@ void *g_malloc0_n(size_t nmemb, size_t size)
     __coverity_negative_sink__(nmemb);
     __coverity_negative_sink__(size);
     ptr = __coverity_alloc__(nmemb * size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
     __coverity_writeall0__(ptr);
     __coverity_mark_as_afm_allocated__(ptr, AFM_free);
     return ptr;
@@ -206,6 +212,9 @@ void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
     __coverity_negative_sink__(size);
     __coverity_escape__(ptr);
     ptr = __coverity_alloc__(nmemb * size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
     /*
      * Memory beyond the old size isn't actually initialized.  Can't
      * model that.  See Coverity's realloc() model
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 8/8] coverity-model: write models fully for non-array allocation functions
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 7/8] coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL Paolo Bonzini
@ 2021-08-02 16:15 ` Paolo Bonzini
  2021-08-02 19:12 ` [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-08-02 16:15 UTC (permalink / raw)
  To: qemu-devel

Coverity seems to have issues figuring out the properties of g_malloc0
and other non *_n functions.  While this was "fixed" by removing the
custom second argument to __coverity_mark_as_afm_allocated__, inline
the code from the array-based allocation functions to avoid future
issues.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-scan/model.c | 57 +++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 028f13e9e3..9d4fba53d9 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -269,32 +269,77 @@ void *g_try_realloc_n(void *ptr, size_t nmemb, size_t size)
 
 void *g_malloc(size_t size)
 {
-    return g_malloc_n(1, size);
+    void *ptr;
+
+    __coverity_negative_sink__(size);
+    ptr = __coverity_alloc__(size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
+    __coverity_mark_as_uninitialized_buffer__(ptr);
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    return ptr;
 }
 
 void *g_malloc0(size_t size)
 {
-    return g_malloc0_n(1, size);
+    void *ptr;
+
+    __coverity_negative_sink__(size);
+    ptr = __coverity_alloc__(size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
+    __coverity_writeall0__(ptr);
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    return ptr;
 }
 
 void *g_realloc(void *ptr, size_t size)
 {
-    return g_realloc_n(ptr, 1, size);
+    __coverity_negative_sink__(size);
+    __coverity_escape__(ptr);
+    ptr = __coverity_alloc__(size);
+    if (!ptr) {
+        __coverity_panic__();
+    }
+    /*
+     * 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_try_malloc(size_t size)
 {
-    return g_try_malloc_n(1, size);
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_malloc(size);
 }
 
 void *g_try_malloc0(size_t size)
 {
-    return g_try_malloc0_n(1, size);
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_malloc0(size);
 }
 
 void *g_try_realloc(void *ptr, size_t size)
 {
-    return g_try_realloc_n(ptr, 1, size);
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_realloc(ptr, size);
 }
 
 /* Other glib functions */
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2
  2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2021-08-02 16:15 ` [PULL 8/8] coverity-model: write models fully for non-array allocation functions Paolo Bonzini
@ 2021-08-02 19:12 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2021-08-02 19:12 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Mon, 2 Aug 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 7742fe64e5c2c2c9f9787d107b693eaac602eaae:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/usb-20210729-pull-request' into staging (2021-07-29 18:49:39 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to e17bdaab2b36db54f0214a14f394fa773cee58df:
>
>   coverity-model: write models fully for non-array allocation functions (2021-07-30 12:04:01 +0200)
>
> ----------------------------------------------------------------
> Fix for smp-opts in configuration file.
> Update Coverity model to what's currently uploaded.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-08-02 19:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 16:15 [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Paolo Bonzini
2021-08-02 16:15 ` [PULL 1/8] vl: introduce machine_merge_property Paolo Bonzini
2021-08-02 16:15 ` [PULL 2/8] vl: stop recording -smp in QemuOpts Paolo Bonzini
2021-08-02 16:15 ` [PULL 3/8] coverity-model: update address_space_read/write models Paolo Bonzini
2021-08-02 16:15 ` [PULL 4/8] coverity-model: make g_free a synonym of free Paolo Bonzini
2021-08-02 16:15 ` [PULL 5/8] coverity-model: remove model for more allocation functions Paolo Bonzini
2021-08-02 16:15 ` [PULL 6/8] coverity-model: clean up the models for array " Paolo Bonzini
2021-08-02 16:15 ` [PULL 7/8] coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL Paolo Bonzini
2021-08-02 16:15 ` [PULL 8/8] coverity-model: write models fully for non-array allocation functions Paolo Bonzini
2021-08-02 19:12 ` [PULL 0/8] vl.c, coverity patches for QEMU 6.1-rc2 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.