Commit Graph

267 Commits

Author SHA1 Message Date
Simon McVittie
6c22a5ee2b Merge branch '2925-gdbus-threading-test' into 'main'
gdbusconnection: Explicitly destroy an idle source on cleanup

See merge request GNOME/glib!3296
2023-03-02 12:35:21 +00:00
Philip Withnall
7b101588e9 gdbusconnection: Make GDBusMethodInvocation transfer a bit clearer
Add a missing steal call in `schedule_method_call()`. This introduces no
functional changes, but documents the ownership transfer more clearly.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2924
2023-02-23 12:16:05 +00:00
Philip Withnall
5a69cc22dc gdbusconnection: Explicitly destroy an idle source on cleanup
Otherwise it’s possible for it to hang around in the `GMainContext`
after the “send message” operation has finished. In the best case, this
will cause the `GTask` and `GDBusMessage` to not be freed when the
calling code expects. In the worst case, it could cause use-after-free
problems if it derefs allocations which have since been freed.

I have not seen either of these problems in practice, but it would be
best for the code to eliminate the risk of them altogether by explicitly
destroying the source when the operation is finished.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2925
2023-02-23 12:14:29 +00:00
Marco Trevisan (Treviño)
e733a3db10 gdbusconnection: Avoid copying connection registered set values twice 2022-12-16 18:45:36 +01:00
Sonny Piers
29da935c28 Fix doc typo in g_dbus_connection_signal_subscribe 2022-11-28 11:33:09 +00:00
Matthias Clasen
794ee60306 gdbusconnection: Set a name on all sources
We already set names on most sources, this
one was just forgotten. This lets us set
a static name, and prevents g_task_attach_source
from setting a non-static one.
2022-11-01 11:48:33 +00:00
Simon McVittie
32b226d1b1 gdbus: Allow cross-namespace connections to Linux session and system buses
The dominant implementations of the well-known session and system
message buses are the reference implementation from the dbus project
(dbus-daemon) and the sd-bus-based reimplementation dbus-broker, both
of which have correct implementations for EXTERNAL authentication with
an unspecified authorization identity.

This makes it reasonably safe to assume that the well-known message
buses can cope with the unspecified authorization identity, even if we
cannot make the same assumption for custom servers such as the ones
used in ibus and gvfs (which might have been started with an older
GLib version before upgrading GLib in-place).

Signed-off-by: Simon McVittie <smcv@collabora.com>
2022-07-24 14:07:02 +01:00
Simon McVittie
e0a0749268 gdbusauthmechanismexternal: Optionally send empty authorization identity
When using a GDBus client in a non-trivial user namespace, the result of
geteuid() can differ from the uid in the namespace where the server is
running. This would result in connection attempts being rejected, because
the identity that the client claims to have does not match the identity
that the server derives from its credentials.

RFC 4422 allows us to send an empty authorization identity, which means we
want to authenticate as whatever identity the server can derive from our
out-of-band credentials. In particular, this resolves the authentication
failure when crossing between different Linux user namespaces.

Because D-Bus does not have a way to represent an empty initial response
as distinct from the absence of an initial response, we cannot use the
initial-response optimization (RFC 4422 §4.3.a) in this case, and must
fall back to waiting for the server to send a challenge.

Unfortunately, GDBus versions older than glib!2826 did not implement
the server side of this protocol correctly, and would respond to the
missing initial response in a way that breaks the SASL state machine
(expecting a response without sending a challenge), causing client and
server to deadlock with each waiting for the other to respond. Until
fixed versions of GDBus are widespread, we can't rely on having a server
that can cope with this, so gate it behind a flag, which can be set for
connections that are known to cross non-trivial namespace boundaries.

Originally inspired by
<1ed4723d38>,
and based on earlier work by Giuseppe Scrivano (in which the
cross-namespace behaviour was unconditional, rather than gated by a
flag).

Co-authored-by: Giuseppe Scrivano <giuseppe@scrivano.org>
Signed-off-by: Simon McVittie <smcv@collabora.com>
2022-07-24 13:46:26 +01:00
Philip Withnall
5942cd7984 gio: Add SPDX license headers automatically
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.

This commit was entirely generated using the command:
```
git ls-files gio/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #1415
2022-05-18 09:18:52 +01:00
Philip Withnall
6c31ef6f18 Merge branch 'cleanup-warnings-split-8' into 'main'
Cleanup warnings split 8

See merge request GNOME/glib!2497
2022-04-01 15:13:32 +00:00
Philip Withnall
e4d77f7e89 gdbusconnection: Use g_strv_contains() rather than a home-grown version
The public `g_strv_contains()` API didn’t exist at the time this code
was originally written. Now, happily, it does.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2022-03-17 16:24:08 +00:00
Gabor Karsay
7e64004db0 docs: mark macros, flags, enums with percent sign 2022-03-04 16:21:55 +00:00
Loic Le Page
c2b60e0323 Fix redefinition of local variable in gio/gdbusconnection.c 2022-02-18 11:02:05 +01:00
Hu Jialun
7f044ba9c0 Amend g_bus_get* documentation regarding private connection 2022-01-28 13:11:34 +00:00
Philip Withnall
117b748e44 gdbusconnection: Fix race between subtree method call and unregistration
Fix another variant of the previous commit, this time specific to the
idle callback of a method call on a subtree object, racing with
unregistration of that subtree.

In this case, the `process_subtree_vtable_message_in_idle_cb()` idle
callback already has a pointer to the right `ExportedSubtree` struct,
but again doesn’t have a strong reference to it.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2400
2021-10-28 14:53:48 +01:00
Philip Withnall
50fbf05d61 gdbusconnection: Fix race between method calls and object unregistration
If `g_dbus_connection_unregister_object()` (or `unregister_subtree()`)
was called from one thread, while an idle callback for a method call (or
a property get or set) was being invoked in another, it was possible for
the two to race after the idle callback had checked that the
object/subtree was registered, but before it had finished dereferencing
all the data related to that object/subtree.

Unregistering the object/subtree would immediately free the data,
leading the idle callback to cause a use-after-free error.

Fix that by giving the idle callback a strong reference to the data from
inside the locked section where it checks whether the object/subtree is
still registered.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2400
2021-10-28 14:53:48 +01:00
Philip Withnall
c8c2ed4af5 gdbusconnection: Make ExportedInterface/ExportedSubtree refcounted
This is needed for an upcoming change which decouples their lifecycle
from their presence in the `map_id_to_ei` and `map_id_to_es` hash
tables.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2400
2021-10-28 14:53:48 +01:00
Philip Withnall
a497fdf302 gdbusconnection: Add some ownership annotations
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2021-10-28 14:53:48 +01:00
Philip Withnall
310f2c1632 gdbusconnection: Move ExportedSubtree definition
Move it further up the file, but make no changes to it. This will help
with a subsequent commit.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2400
2021-10-28 14:53:48 +01:00
Philip Withnall
8e963e0e31 Port internal uses to use g_source_set_static_name()
This should reduce allocations.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2021-07-26 11:01:07 +01:00
Philip Withnall
055f7c6bc5 gdbusconnection: Fix a typo in a code comment
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2021-06-10 12:37:23 +01:00
Philip Withnall
c794261a40 docs: Expand documentation about D-Bus GUIDs
Clarify that the terms ‘GUID’ and ‘UUID’ are used interchangeably in the
context of D-Bus, and that neither of them are an RFC 4122 UUID.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2021-06-10 12:36:25 +01:00
Philip Withnall
9ed66b21dd Merge branch 'confusing_errors' into 'main'
gdbusconnection: removed a confusing message

Closes #793

See merge request GNOME/glib!2118
2021-06-09 12:49:04 +00:00
nitinosiris
5e2986ea2c gdbusconnection: removed a confusing message
The message `No such interface %s on object at path %s`
displayed when requested object does not exist, which was kind of
confusing.

Closes #793
2021-06-08 21:07:21 +05:30
Philip Withnall
1a43d950b4 docs: Update various external links to use HEAD instead of master
Update several links to allow the remote to use its configured default
branch name, rather than specifying `master` as the default branch name.
This will help avoid breakage if any of these projects rename their
default branch in the future.

Fix a few of the links where they were hitting redirects or had moved.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2348
2021-06-07 14:03:48 +01:00
Thomas Haller
b0157af9a6 gdbus: document completion after idle action for g_dbus_connection_signal_unsubscribe()
Since commit ab285899a6 ('gdbusconnection: Document main context
iteration for unsubscript'), we document when the user is guaranteed
that all resources are gone after g_dbus_connection_signal_unsubscribe().
This is not merely an implementation detail, it's something that the
user needs to be able to rely on. It is good that this is documented.

However, libnm does something different ([1]). It registers to several D-Bus
signals without providing a GDestroyNotify. After unsubscription, it schedules
another idle action with lower priority and uses that to know when
cleanup is complete. I think this is a useful alternative and should
also be guaranteed and documented to work.

Also note that this isn't just some implementation detail that currently
happens to work. GDBusConnection tightly integrates with GMainContext and it
works by scheduling idle sources with G_PRIORITY_DEFAULT priority. It needs to
schedule all events with this same priority, otherwise the ordering is not
preserved. At this point, with GDBusConnection working this way, this is no longer
something that can reasonably be any different. It's how GDBusConnection fundamentally
works, and a user must be able to rely on that. As such, this new promise isn't
something that we would want to break in the future.

Thus document it.

[1] a55c10c6cb/src/libnm-client-impl/nm-client.c (L7918)
2021-05-10 13:16:05 +02:00
Thomas Haller
090eeabe9c gdbus: simplify error handling in g_dbus_connection_send_message_unlocked()
"goto out" is a fine pattern, especially when we are not using
cleanup attribute macros.

But in this case it was unnecessary.
2021-03-31 20:56:45 +02:00
Thomas Haller
60d4092a2c gdbus: refactor duplicate code for freeing FilterData 2021-03-31 20:56:45 +02:00
Avinash Sonawane
5ce6ba287f docs: Replace git.gnome.org with gitlab.gnome.org urls 2021-03-24 16:18:53 +05:30
Philip Withnall
1a6aa9a493 gdbus: Add flags to require authentication as the same user
This eliminates a common use case for the
`GDBusAuthObserver::authorize-authenticated-peer` signal, which is often
implemented incorrectly by people.

Suggested by Simon McVittie.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #1804
2021-02-11 16:12:40 +00:00
Simon McVittie
ba25c8a770 gdbus: Reject attempts to set future connection or server flags
The GDBusConnectionFlags and GDBusServerFlags can affect how we carry
out authentication and authorization, either making it more or less
restrictive, so it's desirable to "fail closed" if a program is compiled
against a new version of GLib but run against an old version.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2021-02-08 13:35:16 +00:00
Philip Withnall
73b293fd30 gio: Use g_memdup2() instead of g_memdup() in obvious places
Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()`), so that they use
`g_memdup2()` instead.

In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319
2021-02-04 14:13:21 +00:00
Emmanuel Fleury
68e69a4128 Fix missing initializer warning in gio/gdbusconnection.c:g_dbus_connection_register_object_with_closures()
gio/gdbusconnection.c: In function ‘g_dbus_connection_register_object_with_closures’:
gio/gdbusconnection.c:5527:5: error: missing initializer for field ‘padding’ of ‘GDBusInterfaceVTable’ {aka ‘struct _GDBusInterfaceVTable’}
 5527 |     };
      |     ^
2021-02-01 11:14:21 +01:00
Philip Withnall
eb8d1c4826 gdbusconnection: Improve documentation formatting slightly
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
2020-12-15 09:14:20 +00:00
Philip Withnall
06b5e3e54c gdbusconnection: Add missing (transfer) and (nullable) return annotations
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2227
2020-12-15 09:13:22 +00:00
Philip Withnall
83e48d8ac1 docs: Document not to use volatile qualifiers
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #600
2020-11-20 14:41:07 +00:00
Philip Withnall
3361240439 gdbusconnection: Drop unnecessary volatile qualifiers from variables
This should introduce no API changes; there are public functions
exported by `GDBusConnection` which still have some (incorrectly)
`volatile` arguments, but dropping those qualifiers would be an API
break.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #600
2020-11-20 14:40:19 +00:00
Simon McVittie
fc1f4969bf gdbus: Document the intended semantics of handles and fds
In the D-Bus wire protocol, the handle type (G_VARIANT_TYPE_HANDLE, h)
is intended to be an index/pointer into the implementation's closest
equivalent of GUnixFDList: its numeric value has no semantic meaning
(in the same way that the numeric values of pointers have no semantic
meaning), but a handle with value n acts as a reference to the nth fd
in the fd list.

GDBus provides a fairly direct mapping from the wire protocol to the
C API, which makes it technically possible to attach and use fds
without ever referring to them in the message body, and some
GLib-centric D-Bus APIs rely on this.

However, the other major implementations of D-Bus (libdbus and sd-bus)
transparently replace file descriptors with handles when building
messages, and transparently replace handles with file descriptors when
parsing messages. This means they cannot implement D-Bus APIs that do
not follow the conventional meaning of handles as indexes/pointers into
an equivalent of GUnixFDList.

For interoperability, we should encourage D-Bus API designers to follow
the convention, even though code written against GDBus doesn't strictly
need to do so.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2020-10-28 11:52:22 +00:00
Philip Withnall
5a74c2f445 gdbusconnection: Clarify nullability of SignalInstance.sender
Following on from !1425.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-04-03 12:16:49 +01:00
Philip Withnall
ab285899a6 gdbusconnection: Document main context iteration for unsubscriptions
Add a note to the documentation of
`g_dbus_connection_signal_unsubscribe()`, `g_bus_unwatch_name()` and
`g_bus_unown_name()` warning about the need to continue iterating the
caller’s thread-default `GMainContext` until the
unsubscribe/unwatch/unown operation is complete.

See the previous few commits and #1515 for an idea of the insidious bugs
that can be caused by not iterating the `GMainContext` until
everything’s synchronised.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-02-24 09:03:02 +00:00
Philip Withnall
1a51681e6d gdbusconnection: Simplify some control flow
This removes an unhelpful `goto`. It introduces no functional changes.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-02-24 07:46:22 +00:00
Philip Withnall
bcee182a49 gdbusconnection: Drop an unnecessary GMainContext reference
`CallDestroyNotifyData` never uses that `GMainContext`, and holding a
ref to it could cause reference count cycles if the `GMainContext` is no
longer being iterated.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #1515
2020-02-24 07:46:22 +00:00
Philip Withnall
37b1acdf8c gdbusconnection: Document threading and refcounting for signals
This is essentially a mini writeup of #978.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 15:13:52 +00:00
Philip Withnall
4ec2175d21 gdbusconnection: Tidy up unsubscription code
This just removes a now-redundant intermediate array. This means that
the `SignalSubscriber` instances are now potentially freed a little
sooner, inside the locked segment, but they are already careful to only
call their `user_data_free_func` in the right thread. So that should not
deadlock.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 15:13:52 +00:00
Philip Withnall
130455bbb2 gdbusconnection: Fix race when emitting D-Bus signal callbacks
Instead of storing a copy of the `callback` and `user_data` from a
`SignalSubscriber` in a `SignalInstance` struct (which is the closure
for signal callback data as it’s sent from the D-Bus worker thread to
the thread which originally subscribed to a signal), store a strong
reference to the `SignalSubscriber` struct itself.

This keeps the `SignalSubscriber` alive until the emission is
complete, which ensures that the `user_data` is not freed prematurely.
It also slightly reduces the allocation size of `SignalInstance` (not
that it matters).

This is threadsafe because the fields in `SignalSubscriber` are all
immutable after construction.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Philip Withnall
bee27dd9f0 gdbusconnection: Tidy up destroy notification for signal subscriptions
Tie the destruction of the `user_data` to the destruction of the
`SignalSubscriber` struct. This is tidier, and ensures that the fields
in `SignalSubscriber` are all immutable after being set, so the
structure can safely be used across threads without locking.

It doesn’t matter which thread we call `call_destroy_notify()` in, since
it always defers calling `user_data_free_func` to the user-provided
`GMainContext`.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Philip Withnall
9b1c8d7dd5 gdbusconnection: Allocate SignalSubscriber structs individually
The `SignalSubscriber` structs contain the callback and `user_data` of each
subscriber to a signal, along with the `guint id` token held by that
subscriber to identify their subscription. There are one or more
`SignalSubscriber` structs for a given signal match rule, which is
represented as a `SignalData` struct.

Previously, the `SignalSubscriber` structs were stored in a `GArray` in
the `SignalData` struct, to reduce the number of allocations needed
when subscribing to a signal.

However, this means that a `SignalSubscriber` struct cannot have a
lifetime which exceeds the `SignalData` which contains it. In order to
fix the race in #978, one thread needs to be able to unsubscribe from a
signal (destroying the `SignalData` struct) while zero or more other
threads are in the process of calling the callbacks from a previous
emission of that signal (using the callback and `user_data` from zero or
more `SignalSubscriber` structs). Multiple threads could be calling
callbacks because callbacks are invoked in the `GMainContext` which
originally made a subscription, and GDBus supports subscribing to a
signal from multiple threads. In that case, the callbacks are dispatched
to multiple threads.

In order to allow the `SignalSubscriber` structs to outlive the
`SignalData` which contained their old match rule, store them in a
`GPtrArray` in the `SignalData` struct, and refcount them individually.

This commit in itself should make no functional changes to how GDBus
works, but will allow following commits to do so.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Bastien Nocera
145dc5a49e gdbus: Fix runtime warning with debug enabled
With debug enabled, g_dbus_connection_call_done() will throw a
g_warning() if the call failed (on purpose or not) while trying to the
serial of a non-existant reply.

(/builds/GNOME/glib/_build/gio/tests/gdbus-connection:26921): GLib-GIO-CRITICAL **: 10:10:16.311: g_dbus_message_get_reply_serial: assertion 'G_IS_DBUS_MESSAGE (message)' failed
2019-12-11 11:42:03 +01:00
Philip Withnall
55f9c6d2f4 gatomic: Add various casts to use of g_atomic_*()s to fix warnings
When compiling GLib with `-Wsign-conversion`, we get various warnings
about the atomic calls. A lot of these were fixed by
3ad375a629, but some remain. Fix them by
adding appropriate casts at the call sites.

Note that `g_atomic_int_{and,or,xor}()` actually all operate on `guint`s
rather than `gint`s (which is what the rest of the `g_atomic_int_*()`
functions operate on). I can’t find any written reasoning for this, but
assume that it’s because signedness is irrelevant when you’re using an
integer as a bit field. It’s unfortunate that they’re named a
`g_atomic_int_*()` rather than `g_atomic_uint_*()` functions.

Tested by compiling GLib as:
```
CFLAGS=-Wsign-conversion jhbuild make -ac |& grep atomic
```

I’m not going to add `-Wsign-conversion` to the set of default warnings
for building GLib, because it mostly produces false positives throughout
the rest of GLib.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #1565
2019-09-21 10:48:23 +02:00
Дилян Палаузов
512655aa12 minor typos in the documentation (a/an) 2019-08-24 19:14:05 +00:00