mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
From: Clemens Gruber <clemens.gruber@pqgruber.com>
To: ptxdist@pengutronix.de
Cc: Clemens Gruber <clemens.gruber@pqgruber.com>
Subject: [ptxdist] [PATCH 04/22] dbus: add upstream patches for OOM conditions
Date: Tue, 16 Jan 2018 16:50:22 +0100	[thread overview]
Message-ID: <20180116155040.10061-4-clemens.gruber@pqgruber.com> (raw)
In-Reply-To: <20180116155040.10061-1-clemens.gruber@pqgruber.com>

Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
---
 ...r_new_for_socket-Invalidate-watches-durin.patch |  28 +++++
 ...r_new_for_socket-Properly-disconnect-duri.patch | 124 +++++++++++++++++++++
 ...n_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch |  37 ++++++
 ...r_new_for_socket-Iterate-over-arrays-as-i.patch |  54 +++++++++
 patches/dbus-1.12.2/series                         |   7 ++
 5 files changed, 250 insertions(+)
 create mode 100644 patches/dbus-1.12.2/0001-_dbus_server_new_for_socket-Invalidate-watches-durin.patch
 create mode 100644 patches/dbus-1.12.2/0002-_dbus_server_new_for_socket-Properly-disconnect-duri.patch
 create mode 100644 patches/dbus-1.12.2/0003-_dbus_listen_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch
 create mode 100644 patches/dbus-1.12.2/0004-_dbus_server_new_for_socket-Iterate-over-arrays-as-i.patch
 create mode 100644 patches/dbus-1.12.2/series

diff --git a/patches/dbus-1.12.2/0001-_dbus_server_new_for_socket-Invalidate-watches-durin.patch b/patches/dbus-1.12.2/0001-_dbus_server_new_for_socket-Invalidate-watches-durin.patch
new file mode 100644
index 000000000..0a024be7f
--- /dev/null
+++ b/patches/dbus-1.12.2/0001-_dbus_server_new_for_socket-Invalidate-watches-durin.patch
@@ -0,0 +1,28 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Tue, 21 Nov 2017 14:36:02 +0000
+Subject: [PATCH] _dbus_server_new_for_socket: Invalidate watches during error
+ unwinding
+
+We assert that every watch is invalidated before it is freed, but
+in some OOM code paths this didn't happen.
+
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Reviewed-by: Philip Withnall <withnall@endlessm.com>
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104
+(cherry picked from commit 1ce34beef85a7a0b3c25890837e3a72f8bdac1f0)
+---
+ dbus/dbus-server-socket.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c
+index d716f500234d..61e143ba9e43 100644
+--- a/dbus/dbus-server-socket.c
++++ b/dbus/dbus-server-socket.c
+@@ -358,6 +358,7 @@ _dbus_server_new_for_socket (DBusSocket       *fds,
+     {
+       if (socket_server->watch[i] != NULL)
+         {
++          _dbus_watch_invalidate (socket_server->watch[i]);
+           _dbus_watch_unref (socket_server->watch[i]);
+           socket_server->watch[i] = NULL;
+         }
diff --git a/patches/dbus-1.12.2/0002-_dbus_server_new_for_socket-Properly-disconnect-duri.patch b/patches/dbus-1.12.2/0002-_dbus_server_new_for_socket-Properly-disconnect-duri.patch
new file mode 100644
index 000000000..e01e12786
--- /dev/null
+++ b/patches/dbus-1.12.2/0002-_dbus_server_new_for_socket-Properly-disconnect-duri.patch
@@ -0,0 +1,124 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Tue, 21 Nov 2017 14:38:13 +0000
+Subject: [PATCH] _dbus_server_new_for_socket: Properly disconnect during error
+ unwinding
+
+_dbus_server_finalize_base() asserts that the socket has been
+disconnected, but in some OOM code paths we would call it without
+officially disconnecting. Do so.
+
+This means we need to be a bit more careful about what is
+socket_disconnect()'s responsibility to clean up, what is
+_dbus_server_new_for_socket()'s responsibility, and what is the caller's
+responsibility.
+
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Reviewed-by: Philip Withnall <withnall@endlessm.com>
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104
+(cherry picked from commit 0c03b505a9718c6d497caffb7d6083371679a852)
+---
+ dbus/dbus-server-protected.h |  1 +
+ dbus/dbus-server-socket.c    | 27 ++++++++++++++++++++++-----
+ dbus/dbus-server.c           | 24 +++++++++++++++---------
+ 3 files changed, 38 insertions(+), 14 deletions(-)
+
+diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h
+index f613bf34e0d8..5664b2b4e150 100644
+--- a/dbus/dbus-server-protected.h
++++ b/dbus/dbus-server-protected.h
+@@ -96,6 +96,7 @@ dbus_bool_t _dbus_server_init_base      (DBusServer             *server,
+                                          const DBusString       *address,
+                                          DBusError              *error);
+ void        _dbus_server_finalize_base  (DBusServer             *server);
++void        _dbus_server_disconnect_unlocked (DBusServer        *server);
+ dbus_bool_t _dbus_server_add_watch      (DBusServer             *server,
+                                          DBusWatch              *watch);
+ void        _dbus_server_remove_watch   (DBusServer             *server,
+diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c
+index 61e143ba9e43..42d71450b051 100644
+--- a/dbus/dbus-server-socket.c
++++ b/dbus/dbus-server-socket.c
+@@ -243,8 +243,11 @@ socket_disconnect (DBusServer *server)
+           socket_server->watch[i] = NULL;
+         }
+ 
+-      _dbus_close_socket (socket_server->fds[i], NULL);
+-      _dbus_socket_invalidate (&socket_server->fds[i]);
++      if (_dbus_socket_is_valid (socket_server->fds[i]))
++        {
++          _dbus_close_socket (socket_server->fds[i], NULL);
++          _dbus_socket_invalidate (&socket_server->fds[i]);
++        }
+     }
+ 
+   if (socket_server->socket_name != NULL)
+@@ -338,10 +341,24 @@ _dbus_server_new_for_socket (DBusSocket       *fds,
+                                    socket_server->watch[i]))
+         {
+           int j;
+-          for (j = 0 ; j < i ; j++)
+-            _dbus_server_remove_watch (server,
+-                                       socket_server->watch[j]);
+ 
++          /* The caller is still responsible for closing the fds until
++           * we return successfully, so don't let socket_disconnect()
++           * close them */
++          for (j = 0; j < n_fds; j++)
++            _dbus_socket_invalidate (&socket_server->fds[i]);
++
++          /* socket_disconnect() will try to remove all the watches;
++           * make sure it doesn't see the ones that weren't even added
++           * yet */
++          for (j = i; j < n_fds; j++)
++            {
++              _dbus_watch_invalidate (socket_server->watch[i]);
++              _dbus_watch_unref (socket_server->watch[i]);
++              socket_server->watch[i] = NULL;
++            }
++
++          _dbus_server_disconnect_unlocked (server);
+           SERVER_UNLOCK (server);
+           _dbus_server_finalize_base (&socket_server->base);
+           goto failed_2;
+diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c
+index ea9aff2dbe45..d91df8325cd7 100644
+--- a/dbus/dbus-server.c
++++ b/dbus/dbus-server.c
+@@ -764,6 +764,20 @@ dbus_server_unref (DBusServer *server)
+     }
+ }
+ 
++void
++_dbus_server_disconnect_unlocked (DBusServer *server)
++{
++  _dbus_assert (server->vtable->disconnect != NULL);
++
++  if (!server->disconnected)
++    {
++      /* this has to be first so recursive calls to disconnect don't happen */
++      server->disconnected = TRUE;
++
++      (* server->vtable->disconnect) (server);
++    }
++}
++
+ /**
+  * Releases the server's address and stops listening for
+  * new clients. If called more than once, only the first
+@@ -780,15 +794,7 @@ dbus_server_disconnect (DBusServer *server)
+   dbus_server_ref (server);
+   SERVER_LOCK (server);
+ 
+-  _dbus_assert (server->vtable->disconnect != NULL);
+-
+-  if (!server->disconnected)
+-    {
+-      /* this has to be first so recursive calls to disconnect don't happen */
+-      server->disconnected = TRUE;
+-      
+-      (* server->vtable->disconnect) (server);
+-    }
++  _dbus_server_disconnect_unlocked (server);
+ 
+   SERVER_UNLOCK (server);
+   dbus_server_unref (server);
diff --git a/patches/dbus-1.12.2/0003-_dbus_listen_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch b/patches/dbus-1.12.2/0003-_dbus_listen_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch
new file mode 100644
index 000000000..d3c637d56
--- /dev/null
+++ b/patches/dbus-1.12.2/0003-_dbus_listen_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch
@@ -0,0 +1,37 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Tue, 21 Nov 2017 14:43:01 +0000
+Subject: [PATCH] _dbus_listen_tcp_socket: Don't rely on dbus_realloc setting
+ errno
+
+dbus_realloc() doesn't guarantee to set errno (if it did, the
+only reasonable thing it could set it to would be ENOMEM). In
+particular, faking OOM conditions doesn't set it. This can cause an
+assertion failure when OOM tests assert that the only error that can
+validly occur is DBUS_ERROR_NO_MEMORY.
+
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Reviewed-by: Philip Withnall <withnall@endlessm.com>
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104
+(cherry picked from commit 9ded6907e66b89c3c74620a4485e8f752f092a60)
+---
+ dbus/dbus-sysdeps-unix.c | 6 ++----
+ 1 file changed, 2 insertions(+), 4 deletions(-)
+
+diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
+index bebf2073b680..b0ba89919dab 100644
+--- a/dbus/dbus-sysdeps-unix.c
++++ b/dbus/dbus-sysdeps-unix.c
+@@ -1576,11 +1576,9 @@ _dbus_listen_tcp_socket (const char     *host,
+       newlisten_fd = dbus_realloc(listen_fd, sizeof(DBusSocket)*(nlisten_fd+1));
+       if (!newlisten_fd)
+         {
+-          saved_errno = errno;
+           _dbus_close (fd, NULL);
+-          dbus_set_error (error, _dbus_error_from_errno (saved_errno),
+-                          "Failed to allocate file handle array: %s",
+-                          _dbus_strerror (saved_errno));
++          dbus_set_error (error, DBUS_ERROR_NO_MEMORY,
++                          "Failed to allocate file handle array");
+           goto failed;
+         }
+       listen_fd = newlisten_fd;
diff --git a/patches/dbus-1.12.2/0004-_dbus_server_new_for_socket-Iterate-over-arrays-as-i.patch b/patches/dbus-1.12.2/0004-_dbus_server_new_for_socket-Iterate-over-arrays-as-i.patch
new file mode 100644
index 000000000..eb0270aa0
--- /dev/null
+++ b/patches/dbus-1.12.2/0004-_dbus_server_new_for_socket-Iterate-over-arrays-as-i.patch
@@ -0,0 +1,54 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Mon, 27 Nov 2017 16:23:16 +0000
+Subject: [PATCH] _dbus_server_new_for_socket: Iterate over arrays as intended
+
+Commit 0c03b505 was meant to clear all the fds indexed by j in
+[0, n_fds), which socket_disconnect() can't be allowed to close
+(because on failure the caller remains responsible for closing them);
+but instead it closed the one we failed to add to the main loop
+(fd i), repeatedly.
+
+Similarly, it was meant to invalidate all the watches indexed by j
+in [i, n_fds) (the one we failed to add to the main loop and the ones
+we didn't try to add to the main loop yet), which socket_disconnect()
+can't be allowed to see (because it would fail to remove them from
+the main loop and hit an assertion failure); but instead it invalidated
+fd i, repeatedly.
+
+These happen to be the same thing if you only have one fd, resulting
+in the test-case passing on an IPv4-only system, but failing on a
+system with both IPv4 and IPv6.
+
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Reviewed-by: Philip Withnall <withnall@endlessm.com>
+(cherry picked from commit c9aa00ce730f9741ab39ff704e46ec33dd4a11ea)
+---
+ dbus/dbus-server-socket.c | 8 ++++----
+ 1 file changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c
+index 42d71450b051..6a93091a2291 100644
+--- a/dbus/dbus-server-socket.c
++++ b/dbus/dbus-server-socket.c
+@@ -346,16 +346,16 @@ _dbus_server_new_for_socket (DBusSocket       *fds,
+            * we return successfully, so don't let socket_disconnect()
+            * close them */
+           for (j = 0; j < n_fds; j++)
+-            _dbus_socket_invalidate (&socket_server->fds[i]);
++            _dbus_socket_invalidate (&socket_server->fds[j]);
+ 
+           /* socket_disconnect() will try to remove all the watches;
+            * make sure it doesn't see the ones that weren't even added
+            * yet */
+           for (j = i; j < n_fds; j++)
+             {
+-              _dbus_watch_invalidate (socket_server->watch[i]);
+-              _dbus_watch_unref (socket_server->watch[i]);
+-              socket_server->watch[i] = NULL;
++              _dbus_watch_invalidate (socket_server->watch[j]);
++              _dbus_watch_unref (socket_server->watch[j]);
++              socket_server->watch[j] = NULL;
+             }
+ 
+           _dbus_server_disconnect_unlocked (server);
diff --git a/patches/dbus-1.12.2/series b/patches/dbus-1.12.2/series
new file mode 100644
index 000000000..ddb681448
--- /dev/null
+++ b/patches/dbus-1.12.2/series
@@ -0,0 +1,7 @@
+# generated by git-ptx-patches
+#tag:base --start-number 1
+0001-_dbus_server_new_for_socket-Invalidate-watches-durin.patch
+0002-_dbus_server_new_for_socket-Properly-disconnect-duri.patch
+0003-_dbus_listen_tcp_socket-Don-t-rely-on-dbus_realloc-s.patch
+0004-_dbus_server_new_for_socket-Iterate-over-arrays-as-i.patch
+# 25ab7f66637a3a950cd047d33df3f9e5  - git-ptx-patches magic
-- 
2.15.1


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

  parent reply	other threads:[~2018-01-16 15:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-16 15:50 [ptxdist] [PATCH 01/22] coreutils: version bump 8.27 -> 8.29 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 02/22] cryptsetup: version bump 1.7.5 -> 2.0.0 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 03/22] dbus: version bump 1.10.24 -> 1.12.2 Clemens Gruber
2018-01-24 14:40   ` Michael Olbrich
2018-01-16 15:50 ` Clemens Gruber [this message]
2018-01-16 15:50 ` [ptxdist] [PATCH 05/22] e2fsprogs: version bump 1.43.6 -> 1.43.8 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 06/22] expat: version bump 2.2.4 -> 2.2.5 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 07/22] file: version bump 5.30 -> 5.32 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 08/22] host-meson: version bump 0.43.0 -> 0.44.0 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 09/22] iproute2: version bump 4.13 -> 4.14.1 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 10/22] json-c: version bump 0.12.1 -> 0.13 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 11/22] kexec-tools: version bump 2.0.14 -> 2.0.16 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 12/22] libsodium: version bump 1.0.11 -> 1.0.16 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 13/22] lvm2: version bump 2.02.66 -> 2.02.177 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 14/22] mpg123: version bump 1.25.6 -> 1.25.8 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 15/22] nano: version bump 2.8.4 -> 2.9.2 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 16/22] ninja: version bump 1.7.2 -> 1.8.2 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 17/22] nginx: update pkg-config patches from buildroot Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 18/22] protobuf: version bump 3.3.2 -> 3.5.1 Clemens Gruber
2018-01-25 14:38   ` Michael Olbrich
2018-01-25 15:04     ` Clemens Gruber
2018-01-25 15:24       ` Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 19/22] strace: version bump 4.18 -> 4.20 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 20/22] trace-cmd: version bump 2.6.1 -> 2.6.2 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 21/22] u-boot-tools: version bump 2017.07 -> 2018.01 Clemens Gruber
2018-01-16 15:50 ` [ptxdist] [PATCH 22/22] util-linux-ng: version bump 2.30.2 -> 2.31.1 Clemens Gruber

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=20180116155040.10061-4-clemens.gruber@pqgruber.com \
    --to=clemens.gruber@pqgruber.com \
    --cc=ptxdist@pengutronix.de \
    /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