qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Berger <stefanb@linux.ibm.com>
To: Peter Maydell <peter.maydell@linaro.org>, marcandre.lureau@redhat.com
Cc: qemu-devel@nongnu.org, "Daniel P. Berrangé" <berrange@redhat.com>,
	"Stefan Berger" <stefanb@linux.vnet.ibm.com>,
	"Markus Armbruster" <armbru@redhat.com>
Subject: Re: [PULL v2 05/25] error: add global &error_warn destination
Date: Thu, 6 Apr 2023 10:13:31 -0400	[thread overview]
Message-ID: <8520898b-14e8-33a8-c34f-e98fecbedcb3@linux.ibm.com> (raw)
In-Reply-To: <CAFEAcA9_GP8HqtYgG4mice_ACd8eqFLF6qrMYRz_5oe_HSM=-g@mail.gmail.com>



On 4/6/23 09:17, Peter Maydell wrote:
> On Thu, 6 Apr 2023 at 14:16, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Mon, 13 Mar 2023 at 11:47, <marcandre.lureau@redhat.com> wrote:
>>>
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> This can help debugging issues or develop, when error handling is
>>> introduced.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>> Message-Id: <20230221124802.4103554-6-marcandre.lureau@redhat.com>
>>
>> Hi; Coverity points out that this introduces a use-after-free
>> (CID 1507493):
> 
> ...and also CID 1508179 (same issue, just one warning about the
> callsite in error_setv() and one about the callsite in
> error_propagate()).
> 
> thanks
> -- PMM
> 

I'll be out starting tomorrow. I don't see Marc-André online.

Would this be acceptable?
It ensures that if error_handle() returns, err has been freed.
In the other two cases a copy is being made of the Error that can then be used after the error_handle() call.


diff --git a/util/error.c b/util/error.c
index 5537245da6..7a2296e969 100644
--- a/util/error.c
+++ b/util/error.c
@@ -46,6 +46,8 @@ static void error_handle(Error **errp, Error *err)
      }
      if (errp == &error_warn) {
          warn_report_err(err);
+    } else {
+        error_free(err);
      }
  }

@@ -55,7 +57,7 @@ static void error_setv(Error **errp,
                         ErrorClass err_class, const char *fmt, va_list ap,
                         const char *suffix)
  {
-    Error *err;
+    Error *err, *err_bak;
      int saved_errno = errno;

      if (errp == NULL) {
@@ -75,8 +77,10 @@ static void error_setv(Error **errp,
      err->line = line;
      err->func = func;

+    err_bak = error_copy(err);
      error_handle(errp, err);
-    *errp = err;
+
+    *errp = err_bak;

      errno = saved_errno;
  }
@@ -285,14 +289,14 @@ void error_free_or_abort(Error **errp)

  void error_propagate(Error **dst_errp, Error *local_err)
  {
+    Error *local_err_bak;
      if (!local_err) {
          return;
      }
+    local_err_bak = error_copy(local_err);
      error_handle(dst_errp, local_err);
      if (dst_errp && !*dst_errp) {
-        *dst_errp = local_err;
-    } else {
-        error_free(local_err);
+        *dst_errp = local_err_bak;
      }
  }



  reply	other threads:[~2023-04-06 14:14 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-13 11:46 [PULL v2 00/25] Win socket patches marcandre.lureau
2023-03-13 11:46 ` [PULL v2 01/25] util: drop qemu_fork() marcandre.lureau
2023-03-13 11:46 ` [PULL v2 02/25] tests: use closesocket() marcandre.lureau
2023-03-13 11:46 ` [PULL v2 03/25] io: " marcandre.lureau
2023-03-13 11:46 ` [PULL v2 04/25] tests: add test-error-report marcandre.lureau
2023-03-13 11:46 ` [PULL v2 05/25] error: add global &error_warn destination marcandre.lureau
2023-04-06 13:16   ` Peter Maydell
2023-04-06 13:17     ` Peter Maydell
2023-04-06 14:13       ` Stefan Berger [this message]
2023-04-06 14:36         ` Peter Maydell
2023-04-06 15:00           ` Stefan Berger
2023-04-06 15:04             ` Peter Maydell
2023-04-06 15:30             ` Marc-André Lureau
2023-04-06 15:17         ` Markus Armbruster
2023-03-13 11:46 ` [PULL v2 06/25] win32/socket: introduce qemu_socket_select() helper marcandre.lureau
2023-03-13 11:46 ` [PULL v2 07/25] win32/socket: introduce qemu_socket_unselect() helper marcandre.lureau
2023-03-13 11:46 ` [PULL v2 08/25] aio: make aio_set_fd_poll() static to aio-posix.c marcandre.lureau
2023-03-13 11:46 ` [PULL v2 09/25] aio/win32: aio_set_fd_handler() only supports SOCKET marcandre.lureau
2023-03-13 11:46 ` [PULL v2 10/25] main-loop: remove qemu_fd_register(), win32/slirp/socket specific marcandre.lureau
2023-03-13 11:46 ` [PULL v2 11/25] slirp: unregister the win32 SOCKET marcandre.lureau
2023-03-13 11:46 ` [PULL v2 12/25] slirp: open-code qemu_socket_(un)select() marcandre.lureau
2023-03-13 11:46 ` [PULL v2 13/25] win32: avoid mixing SOCKET and file descriptor space marcandre.lureau
2023-03-13 11:46 ` [PULL v2 14/25] os-posix: remove useless ioctlsocket() define marcandre.lureau
2023-03-13 11:46 ` [PULL v2 15/25] win32: replace closesocket() with close() wrapper marcandre.lureau
2023-03-13 11:46 ` [PULL v2 16/25] tests: fix path separator, use g_build_filename() marcandre.lureau
2023-03-13 11:46 ` [PULL v2 17/25] char: do not double-close fd when failing to add client marcandre.lureau
2023-03-13 11:46 ` [PULL v2 18/25] tests/docker: fix a win32 error due to portability marcandre.lureau
2023-03-13 11:46 ` [PULL v2 19/25] osdep: implement qemu_socketpair() for win32 marcandre.lureau
2023-03-13 11:46 ` [PULL v2 20/25] qmp: 'add_client' actually expects sockets marcandre.lureau
2023-03-13 11:46 ` [PULL v2 21/25] monitor: release the lock before calling close() marcandre.lureau
2023-03-13 11:46 ` [PULL v2 22/25] qmp: add 'get-win32-socket' marcandre.lureau
2023-03-13 11:46 ` [PULL v2 23/25] libqtest: make qtest_qmp_add_client work on win32 marcandre.lureau
2023-03-13 11:46 ` [PULL v2 24/25] qtest: enable vnc-display test " marcandre.lureau
2023-03-13 11:46 ` [PULL v2 25/25] monitor: restrict command getfd to POSIX hosts marcandre.lureau
2023-03-13 17:09 ` [PULL v2 00/25] Win socket patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8520898b-14e8-33a8-c34f-e98fecbedcb3@linux.ibm.com \
    --to=stefanb@linux.ibm.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanb@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).