qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06
@ 2018-07-06 17:14 Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size Paolo Bonzini
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 2a018f6e98782a4931b936a3087404ed81685bac:

  Merge remote-tracking branch 'remotes/berrange/tags/qcrypto-next-pull-request' into staging (2018-07-03 23:06:18 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to e20122ff0faf07cb701d35e39e106d1783c07725:

  checkpatch: handle token pasting better (2018-07-06 18:39:19 +0200)

----------------------------------------------------------------
Bug fixes.

----------------------------------------------------------------
Greg Kurz (1):
      i386: fix '-cpu ?' output for host cpu type

Julia Suvorova (1):
      qtest: Use cpu address space instead of system memory

Michal Privoznik (1):
      pr-helper: Rework socket path handling

Paolo Bonzini (4):
      pr-helper: avoid error on PR IN command with zero request size
      pr-manager-helper: fix memory leak on event
      ioapic: remove useless lower bounds check
      checkpatch: handle token pasting better

xinhua.Cao (1):
      qemu-char: check errno together with ret < 0

 chardev/char-socket.c    |  7 +++-
 hw/intc/ioapic.c         |  2 +-
 qtest.c                  | 39 ++++++++++++-------
 scripts/checkpatch.pl    |  9 ++---
 scsi/pr-manager-helper.c |  1 +
 scsi/qemu-pr-helper.c    | 99 +++++++++++++++++++-----------------------------
 target/i386/cpu.c        | 14 +++----
 7 files changed, 84 insertions(+), 87 deletions(-)
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 2/8] pr-helper: Rework socket path handling Paolo Bonzini
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel

After reading a PR IN command with zero request size in prh_read_request,
the resp->result field will be uninitialized and the resp.sz field will
be also uninitialized when returning to prh_co_entry.

If resp->result == GOOD (from a previous successful reply or just luck),
then the assert in prh_write_response might not be triggered and
uninitialized response will be sent.

The fix is to remove the whole handling of sz == 0 in prh_co_entry.
Those errors apply only to PR OUT commands and it's perfectly okay to
catch them later in do_pr_out and multipath_pr_out; the check for
too-short parameters in fact doesn't apply in the easy SG_IO case, as
it can be left to the target firmware even.

The result is that prh_read_request does not fail requests anymore and
prh_co_entry becomes simpler.

Reported-by: Dima Stepanov <dimastep@yandex-team.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scsi/qemu-pr-helper.c | 63 ++++++++++++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 0218d65..c89a446 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -455,6 +455,14 @@ static int multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *sense,
     char transportids[PR_HELPER_DATA_SIZE];
     int r;
 
+    if (sz < PR_OUT_FIXED_PARAM_SIZE) {
+        /* Illegal request, Parameter list length error.  This isn't fatal;
+         * we have read the data, send an error without closing the socket.
+         */
+        scsi_build_sense(sense, SENSE_CODE(INVALID_PARAM_LEN));
+        return CHECK_CONDITION;
+    }
+
     switch (rq_servact) {
     case MPATH_PROUT_REG_SA:
     case MPATH_PROUT_RES_SA:
@@ -574,6 +582,12 @@ static int do_pr_out(int fd, const uint8_t *cdb, uint8_t *sense,
                      const uint8_t *param, int sz)
 {
     int resp_sz;
+
+    if ((fcntl(fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
+        scsi_build_sense(sense, SENSE_CODE(INVALID_OPCODE));
+        return CHECK_CONDITION;
+    }
+
 #ifdef CONFIG_MPATH
     if (is_mpath(fd)) {
         return multipath_pr_out(fd, cdb, sense, param, sz);
@@ -690,21 +704,6 @@ static int coroutine_fn prh_read_request(PRHelperClient *client,
                                  errp) < 0) {
             goto out_close;
         }
-        if ((fcntl(client->fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
-            scsi_build_sense(resp->sense, SENSE_CODE(INVALID_OPCODE));
-            sz = 0;
-        } else if (sz < PR_OUT_FIXED_PARAM_SIZE) {
-            /* Illegal request, Parameter list length error.  This isn't fatal;
-             * we have read the data, send an error without closing the socket.
-             */
-            scsi_build_sense(resp->sense, SENSE_CODE(INVALID_PARAM_LEN));
-            sz = 0;
-        }
-        if (sz == 0) {
-            resp->result = CHECK_CONDITION;
-            close(client->fd);
-            client->fd = -1;
-        }
     }
 
     req->fd = client->fd;
@@ -785,25 +784,23 @@ static void coroutine_fn prh_co_entry(void *opaque)
             break;
         }
 
-        if (sz > 0) {
-            num_active_sockets++;
-            if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
-                r = do_pr_out(req.fd, req.cdb, resp.sense,
-                              client->data, sz);
-                resp.sz = 0;
-            } else {
-                resp.sz = sizeof(client->data);
-                r = do_pr_in(req.fd, req.cdb, resp.sense,
-                             client->data, &resp.sz);
-                resp.sz = MIN(resp.sz, sz);
-            }
-            num_active_sockets--;
-            close(req.fd);
-            if (r == -1) {
-                break;
-            }
-            resp.result = r;
+        num_active_sockets++;
+        if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
+            r = do_pr_out(req.fd, req.cdb, resp.sense,
+                          client->data, sz);
+            resp.sz = 0;
+        } else {
+            resp.sz = sizeof(client->data);
+            r = do_pr_in(req.fd, req.cdb, resp.sense,
+                         client->data, &resp.sz);
+            resp.sz = MIN(resp.sz, sz);
+        }
+        num_active_sockets--;
+        close(req.fd);
+        if (r == -1) {
+            break;
         }
+        resp.result = r;
 
         if (prh_write_response(client, &req, &resp, &local_err) < 0) {
             break;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 2/8] pr-helper: Rework socket path handling
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 3/8] qtest: Use cpu address space instead of system memory Paolo Bonzini
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michal Privoznik

From: Michal Privoznik <mprivozn@redhat.com>

When reviewing Paolo's pr-helper patches I've noticed couple of
problems:

1) socket_path needs to be calculated at two different places
(one for printing out help, the other if socket activation is NOT
used),

2) even though the default socket_path is allocated in
compute_default_paths() it is the only default path the function
handles. For instance, pidfile is allocated outside of this
function. And yet again, at different places than 1)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <c791ba035f26ea957e8f3602e3009b621769b1ba.1530611283.git.mprivozn@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scsi/qemu-pr-helper.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index c89a446..1528a71 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -76,14 +76,12 @@ static int gid = -1;
 
 static void compute_default_paths(void)
 {
-    if (!socket_path) {
-        socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
-    }
+    socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
+    pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
 }
 
 static void usage(const char *name)
 {
-    compute_default_paths();
     (printf) (
 "Usage: %s [OPTIONS] FILE\n"
 "Persistent Reservation helper program for QEMU\n"
@@ -841,19 +839,6 @@ static gboolean accept_client(QIOChannel *ioc, GIOCondition cond, gpointer opaqu
     return TRUE;
 }
 
-
-/*
- * Check socket parameters compatibility when socket activation is used.
- */
-static const char *socket_activation_validate_opts(void)
-{
-    if (socket_path != NULL) {
-        return "Unix socket can't be set when using socket activation";
-    }
-
-    return NULL;
-}
-
 static void termsig_handler(int signum)
 {
     atomic_cmpxchg(&state, RUNNING, TERMINATE);
@@ -927,6 +912,7 @@ int main(int argc, char **argv)
     char *trace_file = NULL;
     bool daemonize = false;
     bool pidfile_specified = false;
+    bool socket_path_specified = false;
     unsigned socket_activation;
 
     struct sigaction sa_sigterm;
@@ -943,12 +929,14 @@ int main(int argc, char **argv)
     qemu_add_opts(&qemu_trace_opts);
     qemu_init_exec_dir(argv[0]);
 
-    pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
+    compute_default_paths();
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'k':
-            socket_path = optarg;
+            g_free(socket_path);
+            socket_path = g_strdup(optarg);
+            socket_path_specified = true;
             if (socket_path[0] != '/') {
                 error_report("socket path must be absolute");
                 exit(EXIT_FAILURE);
@@ -1039,10 +1027,9 @@ int main(int argc, char **argv)
     socket_activation = check_socket_activation();
     if (socket_activation == 0) {
         SocketAddress saddr;
-        compute_default_paths();
         saddr = (SocketAddress){
             .type = SOCKET_ADDRESS_TYPE_UNIX,
-            .u.q_unix.path = g_strdup(socket_path)
+            .u.q_unix.path = socket_path,
         };
         server_ioc = qio_channel_socket_new();
         if (qio_channel_socket_listen_sync(server_ioc, &saddr, &local_err) < 0) {
@@ -1050,12 +1037,10 @@ int main(int argc, char **argv)
             error_report_err(local_err);
             return 1;
         }
-        g_free(saddr.u.q_unix.path);
     } else {
         /* Using socket activation - check user didn't use -p etc. */
-        const char *err_msg = socket_activation_validate_opts();
-        if (err_msg != NULL) {
-            error_report("%s", err_msg);
+        if (socket_path_specified) {
+            error_report("Unix socket can't be set when using socket activation");
             exit(EXIT_FAILURE);
         }
 
@@ -1072,7 +1057,6 @@ int main(int argc, char **argv)
                          error_get_pretty(local_err));
             exit(EXIT_FAILURE);
         }
-        socket_path = NULL;
     }
 
     if (qemu_init_main_loop(&local_err)) {
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 3/8] qtest: Use cpu address space instead of system memory
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 2/8] pr-helper: Rework socket path handling Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 4/8] i386: fix '-cpu ?' output for host cpu type Paolo Bonzini
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Julia Suvorova

From: Julia Suvorova <jusual@mail.ru>

Some devices (like nvic in armv7m) are not accessable through
address_space_memory, therefore can not be tested with qtest.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Message-Id: <20180702065237.27899-1-jusual@mail.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qtest.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/qtest.c b/qtest.c
index cbbfb71..69b9e99 100644
--- a/qtest.c
+++ b/qtest.c
@@ -387,19 +387,23 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][5] == 'b') {
             uint8_t data = value;
-            cpu_physical_memory_write(addr, &data, 1);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             &data, 1, true);
         } else if (words[0][5] == 'w') {
             uint16_t data = value;
             tswap16s(&data);
-            cpu_physical_memory_write(addr, &data, 2);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &data, 2, true);
         } else if (words[0][5] == 'l') {
             uint32_t data = value;
             tswap32s(&data);
-            cpu_physical_memory_write(addr, &data, 4);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &data, 4, true);
         } else if (words[0][5] == 'q') {
             uint64_t data = value;
             tswap64s(&data);
-            cpu_physical_memory_write(addr, &data, 8);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &data, 8, true);
         }
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
@@ -417,18 +421,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][4] == 'b') {
             uint8_t data;
-            cpu_physical_memory_read(addr, &data, 1);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             &data, 1, false);
             value = data;
         } else if (words[0][4] == 'w') {
             uint16_t data;
-            cpu_physical_memory_read(addr, &data, 2);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &data, 2, false);
             value = tswap16(data);
         } else if (words[0][4] == 'l') {
             uint32_t data;
-            cpu_physical_memory_read(addr, &data, 4);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &data, 4, false);
             value = tswap32(data);
         } else if (words[0][4] == 'q') {
-            cpu_physical_memory_read(addr, &value, 8);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             (uint8_t *) &value, 8, false);
             tswap64s(&value);
         }
         qtest_send_prefix(chr);
@@ -448,7 +456,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(len);
 
         data = g_malloc(len);
-        cpu_physical_memory_read(addr, data, len);
+        address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                         data, len, false);
 
         enc = g_malloc(2 * len + 1);
         for (i = 0; i < len; i++) {
@@ -473,7 +482,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(ret == 0);
 
         data = g_malloc(len);
-        cpu_physical_memory_read(addr, data, len);
+        address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                         data, len, false);
         b64_data = g_base64_encode(data, len);
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %s\n", b64_data);
@@ -507,7 +517,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                 data[i] = 0;
             }
         }
-        cpu_physical_memory_write(addr, data, len);
+        address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                         data, len, true);
         g_free(data);
 
         qtest_send_prefix(chr);
@@ -529,7 +540,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         if (len) {
             data = g_malloc(len);
             memset(data, pattern, len);
-            cpu_physical_memory_write(addr, data, len);
+            address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                             data, len, true);
             g_free(data);
         }
 
@@ -562,7 +574,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             out_len = MIN(out_len, len);
         }
 
-        cpu_physical_memory_write(addr, data, out_len);
+        address_space_rw(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+                         data, len, true);
 
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 4/8] i386: fix '-cpu ?' output for host cpu type
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 3/8] qtest: Use cpu address space instead of system memory Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 5/8] qemu-char: check errno together with ret < 0 Paolo Bonzini
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

From: Greg Kurz <groug@kaod.org>

Since commit d6dcc5583e7, '-cpu ?' shows the description of the
X86_CPU_TYPE_NAME("max") for the host CPU model:

Enables all features supported by the accelerator in the current host

instead of the expected:

KVM processor with all supported host features

or

HVF processor with all supported host features

This is caused by the early use of kvm_enabled() and hvf_enabled() in
a class_init function. Since the accelerator isn't configured yet, both
helpers return false unconditionally.

A QEMU binary will only be compiled with one of these accelerators, not
both. The appropriate description can thus be decided at build time.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <153055056654.212317.4697363278304826913.stgit@bahia.lan>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b0b87c3..e0e2f2e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2836,13 +2836,13 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
     xcc->host_cpuid_required = true;
     xcc->ordering = 8;
 
-    if (kvm_enabled()) {
-        xcc->model_description =
-            "KVM processor with all supported host features ";
-    } else if (hvf_enabled()) {
-        xcc->model_description =
-            "HVF processor with all supported host features ";
-    }
+#if defined(CONFIG_KVM)
+    xcc->model_description =
+        "KVM processor with all supported host features ";
+#elif defined(CONFIG_HVF)
+    xcc->model_description =
+        "HVF processor with all supported host features ";
+#endif
 }
 
 static const TypeInfo host_x86_cpu_type_info = {
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 5/8] qemu-char: check errno together with ret < 0
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 4/8] i386: fix '-cpu ?' output for host cpu type Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 6/8] pr-manager-helper: fix memory leak on event Paolo Bonzini
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: xinhua.Cao

From: "xinhua.Cao" <caoxinhua@huawei.com>

In the tcp_chr_write function, we checked errno,
but errno was not reset before a read or write operation.
Therefore, this check of errno's actions is often
incorrect after EAGAIN has occurred.
we need check errno together with ret < 0.

Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
Message-Id: <20180704033642.15996-1-caoxinhua@huawei.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Fixes: 9fc53a10f81d3a9027b23fa810147d21be29e614
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char-socket.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 17519ec..efbad6e 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
                                         s->write_msgfds,
                                         s->write_msgfds_num);
 
-        /* free the written msgfds in any cases other than errno==EAGAIN */
-        if (EAGAIN != errno && s->write_msgfds_num) {
+        /* free the written msgfds in any cases
+         * other than ret < 0 && errno == EAGAIN
+         */
+        if (!(ret < 0 && EAGAIN == errno)
+            && s->write_msgfds_num) {
             g_free(s->write_msgfds);
             s->write_msgfds = 0;
             s->write_msgfds_num = 0;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 6/8] pr-manager-helper: fix memory leak on event
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 5/8] qemu-char: check errno together with ret < 0 Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 7/8] ioapic: remove useless lower bounds check Paolo Bonzini
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel

Reported by Coverity.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scsi/pr-manager-helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
index 519a296..3027dde 100644
--- a/scsi/pr-manager-helper.c
+++ b/scsi/pr-manager-helper.c
@@ -46,6 +46,7 @@ static void pr_manager_send_status_changed_event(PRManagerHelper *pr_mgr)
     if (id) {
         qapi_event_send_pr_manager_status_changed(id, !!pr_mgr->ioc,
                                                   &error_abort);
+        g_free(id);
     }
 }
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 7/8] ioapic: remove useless lower bounds check
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 6/8] pr-manager-helper: fix memory leak on event Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 17:14 ` [Qemu-devel] [PULL 8/8] checkpatch: handle token pasting better Paolo Bonzini
  2018-07-06 18:05 ` [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel

The vector cannot be negative.  Coverity now reports this because it sees an
array access before the check, in ioapic_stat_update_irq.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/ioapic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index b393780..b6896ac 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -152,7 +152,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
     if (vector == 0) {
         vector = 2;
     }
-    if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
+    if (vector < IOAPIC_NUM_PINS) {
         uint32_t mask = 1 << vector;
         uint64_t entry = s->ioredtbl[vector];
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 8/8] checkpatch: handle token pasting better
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 7/8] ioapic: remove useless lower bounds check Paolo Bonzini
@ 2018-07-06 17:14 ` Paolo Bonzini
  2018-07-06 18:05 ` [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-07-06 17:14 UTC (permalink / raw)
  To: qemu-devel

The mechanism to find possible type tokens can sometimes be confused and go into an
infinite loop.  This happens for example in QEMU for a line that looks like

         uint## BITS ##_t S = _S, T = _T;                            \
         uint## BITS ##_t as, at, xs, xt, xd;                        \

Because the token pasting operator does not have a space before _t, it does not
match $notPermitted.  However, (?x) is turned on in the regular expression for
modifiers, and thus ##_t matches the empty string.  As a result, annotate_values
goes in an infinite loop.

The solution is simply to remove token pasting operators from the string before
looking for modifiers.  In the example above, the string uintBITS_t will be
evaluated as a candidate modifier.  This is not optimal, but it works as long
as people do not write things like a##s##m, and it fits nicely into sub
possible.

For a similar reason, \# should be rejected always, even if it is not
at end of line or followed by whitespace.

The same patch was sent to the Linux kernel mailing list.

Reported-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 223681b..42e1c50 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1132,11 +1132,10 @@ sub possible {
 			case|
 			else|
 			asm|__asm__|
-			do|
-			\#|
-			\#\#
+			do
 		)(?:\s|$)|
-		^(?:typedef|struct|enum)\b
+		^(?:typedef|struct|enum)\b|
+		^\#
 	    )}x;
 	warn "CHECK<$possible> ($line)\n" if ($dbg_possible > 2);
 	if ($possible !~ $notPermitted) {
@@ -1146,7 +1145,7 @@ sub possible {
 		if ($possible =~ /^\s*$/) {
 
 		} elsif ($possible =~ /\s/) {
-			$possible =~ s/\s*$Type\s*//g;
+			$possible =~ s/\s*(?:$Type|\#\#)\s*//g;
 			for my $modifier (split(' ', $possible)) {
 				if ($modifier !~ $notPermitted) {
 					warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06
  2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2018-07-06 17:14 ` [Qemu-devel] [PULL 8/8] checkpatch: handle token pasting better Paolo Bonzini
@ 2018-07-06 18:05 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2018-07-06 18:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On 6 July 2018 at 18:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 2a018f6e98782a4931b936a3087404ed81685bac:
>
>   Merge remote-tracking branch 'remotes/berrange/tags/qcrypto-next-pull-request' into staging (2018-07-03 23:06:18 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to e20122ff0faf07cb701d35e39e106d1783c07725:
>
>   checkpatch: handle token pasting better (2018-07-06 18:39:19 +0200)
>
> ----------------------------------------------------------------
> Bug fixes.
>
> ----------------------------------------------------------------
> Greg Kurz (1):
>       i386: fix '-cpu ?' output for host cpu type
>
> Julia Suvorova (1):
>       qtest: Use cpu address space instead of system memory
>
> Michal Privoznik (1):
>       pr-helper: Rework socket path handling
>
> Paolo Bonzini (4):
>       pr-helper: avoid error on PR IN command with zero request size
>       pr-manager-helper: fix memory leak on event
>       ioapic: remove useless lower bounds check
>       checkpatch: handle token pasting better
>
> xinhua.Cao (1):
>       qemu-char: check errno together with ret < 0

Applied, thanks.

-- PMM

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

end of thread, other threads:[~2018-07-06 18:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-06 17:14 [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 2/8] pr-helper: Rework socket path handling Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 3/8] qtest: Use cpu address space instead of system memory Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 4/8] i386: fix '-cpu ?' output for host cpu type Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 5/8] qemu-char: check errno together with ret < 0 Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 6/8] pr-manager-helper: fix memory leak on event Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 7/8] ioapic: remove useless lower bounds check Paolo Bonzini
2018-07-06 17:14 ` [Qemu-devel] [PULL 8/8] checkpatch: handle token pasting better Paolo Bonzini
2018-07-06 18:05 ` [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).