Instead of using timestamp 0 as a magic number (in this case interpreted
as 1970-01-01T00:00:00-08:00), calculate a timestamp from a recent
year/month/day in winter, in this case 2024-01-01T00:00:00-08:00.
Similarly, instead of using a timestamp 15 million seconds later
(1970-06-23T15:40:00-07:00), calculate a timestamp from a recent
year/month/day in summer, in this case 2024-07-01T00:00:00-07:00.
Signed-off-by: Simon McVittie <smcv@debian.org>
In newer tzdata, it is an alias for America/Los_Angeles, which has a
slightly different meaning: DST did not exist there before 1883. As a
result, we can no longer hard-code the knowledge that interval 0 is
standard time and interval 1 is summer time, and instead we need to look
up the correct intervals from known timestamps.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3502
Bug-Debian: https://bugs.debian.org/1084190
[smcv: expand commit message, fix whitespace]
Signed-off-by: Simon McVittie <smcv@debian.org>
This was a bug that existed during development of this branch; make sure
it doesn't come back.
This test fails with a use-after-free and crash if we comment out the
part of name_watcher_unref_watched_name() that removes the name watcher
from `map_method_serial_to_name_watcher`.
It would also fail with an assertion failure if we asserted in
name_watcher_unref_watched_name() that get_name_owner_serial == 0
(i.e. that GetNameOwner is not in-flight at destruction).
Signed-off-by: Simon McVittie <smcv@collabora.com>
The vulnerability reported as GNOME/glib#3268 can be characterized
as: these signals from an attacker should not be delivered to either
the GDBusConnection or the GDBusProxy, but in fact they are (in at
least some scenarios).
Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
The expected result is that because TEST_CONN_SERVICE owns
ALREADY_OWNED_NAME but not (yet) OWNED_LATER_NAME, the signal will be
delivered to the subscriber for the former but not the latter.
Before #3268 was fixed, it was incorrectly delivered to both.
Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268 (partially)
Signed-off-by: Simon McVittie <smcv@collabora.com>
On GNOME/glib#3268 there was some concern about whether this would
allow an attacker to send signals and have them be matched to a
GDBusProxy in this situation, but it seems that was a false alarm.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This somewhat duplicates test_connection_signals(), but is easier to
extend to cover different scenarios.
Each scenario is tested three times: once with lower-level
GDBusConnection APIs, once with the higher-level GDBusProxy (which
cannot implement all of the subscription scenarios, so some message
counts are lower), and once with both (to check that delivery of the
same message to multiple destinations is handled appropriately).
[Backported to glib-2-74, resolving conflicts in gio/tests/meson.build]
Signed-off-by: Simon McVittie <smcv@collabora.com>
In a D-Bus-Specification-compliant message bus, the owner of a well-known
name is a unique name. However, ibus has its own small implementation
of a message bus (src/ibusbus.c) in which org.freedesktop.IBus is
special-cased to also have itself as its owner (like org.freedesktop.DBus
on a standard message bus), and connects to that bus with the
G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION flag. The ability to do
this regressed when CVE-2024-34397 was fixed.
Relax the checks to allow the owner of a well-known name to be any valid
D-Bus name, even if it is not syntactically a unique name.
Fixes: 683b14b9 "gdbus: Track name owners for signal subscriptions"
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3353
Bug-Debian: https://bugs.debian.org/1070730
Bug-Debian: https://bugs.debian.org/1070736
Bug-Debian: https://bugs.debian.org/1070743
Bug-Debian: https://bugs.debian.org/1070745
Signed-off-by: Simon McVittie <smcv@debian.org>
Otherwise a malicious connection on a shared bus, especially the system
bus, could trick GDBus clients into processing signals sent by the
malicious connection as though they had come from the real owner of a
well-known service name.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
We will use this in a subsequent commit to prevent signals from an
impostor from being delivered to a subscriber.
To avoid message reordering leading to misleading situations, this does
not use the existing mechanism for watching bus name ownership, which
delivers the ownership changes to other main-contexts. Instead, it all
happens on the single thread used by the GDBusWorker, so the order in
which messages are received is the order in which they are processed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This will become confusing when we start tracking the owner of a
well-known-name sender, and it's redundant anyway. Instead, track the
1 bit of data that we actually need: whether it's a well-known name.
Strictly speaking this too is redundant, because it's syntactically
derivable from the sender, but only via extra string operations.
A subsequent commit will add a data structure to keep track of the
owner of a well-known-name sender, at which point this boolean will
be replaced by the presence or absence of that data structure.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional change, just removing some nesting. The check for whether
signal_data->subscribers is empty changes from a conditional that tests
whether it is into an early-return if it isn't.
A subsequent commit will add additional conditions that make us consider
a SignalData to be still in use and therefore not eligible to be removed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional changes, except that the implicit ownership-transfer
for the rule field becomes explicit (the local variable is set to NULL
afterwards).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Subsequent changes will need to access these data structures from
on_worker_message_received(). No functional change here, only moving
code around.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Using these is a bit more clearly correct than repeating them everywhere.
To avoid excessive diffstat in a branch for a bug fix, I'm not
immediately replacing all existing occurrences of the same literals with
these names.
The names of these constants are chosen to be consistent with libdbus,
despite using somewhat outdated terminology (D-Bus now uses the term
"well-known bus name" for what used to be called a service name,
reserving the word "service" to mean specifically the programs that
have .service files and participate in service activation).
Signed-off-by: Simon McVittie <smcv@collabora.com>
As with all D-Bus signal subscriptions, it’s possible for a signal
callback to be invoked in one thread (T1) while another thread (T2) is
unsubscribing from that signal. In this case, T1 is the main thread, and
T2 is the D-Bus connection worker thread which is unsubscribing all
signals as it’s in the process of closing.
Due to this possibility, all `user_data` for signal callbacks needs to
be referenced outside the lifecycle of the code which
subscribes/unsubscribes the signal. In other words, it’s not safe to
subscribe to a signal, store the subscription ID in a struct,
unsubscribe from the signal when freeing the struct, and dereference the
struct in the signal callback. The data passed to the signal callback
has to have its own strong reference.
Instead, it’s safe to subscribe to a signal and add a strong reference
to the struct, store the subscription ID in that struct, and unsubscribe
from the signal when the last external reference to your struct is
dropped. That unsubscription should break the refcount cycle between the
signal connection and the struct, and allow the struct to be completely
freed. Only with that approach is it safe to dereference the struct in
the signal callback, if there’s any possibility that the signal might be
unsubscribed from a separate thread.
The tests need specific additional main loop cycles to completely emit
the NameLost signal callback. Ideally they need refactoring, but this
will do (1000 test cycles passed).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #978
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
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
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
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
It already implicitly held a strong ref on its `GTask` values, but
didn’t have a free function set so that they would be automatically
unreffed on removal from the map.
This meant that the functions handling removals from the map,
`on_worker_closed()` (via `cancel_method_on_close()`) and
`send_message_with_reply_cleanup()` had to call unref once more than
they would otherwise.
In `send_message_with_reply_cleanup()`, this behaviour depended on
whether it was called with `remove == TRUE`. If not, it was `(transfer
none)` not `(transfer full)`. This led to bugs in its callers.
For example, this led to a direct leak in `cancel_method_on_close()`, as
it needed to remove tasks from `map_method_serial_to_task`, but called
`send_message_with_reply_cleanup(remove = FALSE)` and erroneously didn’t
call unref an additional time.
Try and simplify it all by setting a `GDestroyNotify` on
`map_method_serial_to_task`’s values, and making the refcount handling
of `send_message_with_reply_cleanup()` not be conditional on its
arguments.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
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
Wherever we use gssize to allow passing -1, we need to ensure we don't
overflow the value by assigning a gsize to it without checking if the
size exceeds the maximum gssize. The safest way to do this is to just
use normal gsize everywhere instead and use gssize only for the
parameter.
Our computers don't have enough RAM to write tests for this. I tried
forcing string->len to high values for test purposes, but this isn't
valid and will just cause out of bounds reads/writes due to
string->allocated_len being unexpectedly small, so I don't think we can
test this easily.
In file included from glib/glibconfig.h:9,
from glib/gtypes.h:32,
from glib/gstring.h:32,
from glib/gstring.c:37:
glib/gstring.c: In function ‘g_string_insert_len’:
glib/gstring.c:441:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
g_return_val_if_fail (pos <= string->len, string);
^~
glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’
#define G_LIKELY(expr) (expr)
^~~~
glib/gstring.c:441:5: note: in expansion of macro ‘g_return_val_if_fail’
g_return_val_if_fail (pos <= string->len, string);
^~~~~~~~~~~~~~~~~~~~
glib/gstring.c:458:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (pos < string->len)
^
glib/gstring.c:462:18: error: comparison of integer expressions of different signedness: ‘gsize’ {aka ‘long unsigned int’} and ‘gssize’ {aka ‘long int’} [-Werror=sign-compare]
if (offset < pos)
^
In file included from glib/glibconfig.h:9,
from glib/gtypes.h:32,
from glib/gstring.h:32,
from glib/gstring.c:37:
glib/gmacros.h:351:26: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘long unsigned int’ [-Werror=sign-compare]
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
^
glib/gstring.c:464:22: note: in expansion of macro ‘MIN’
precount = MIN (len, pos - offset);
^~~
glib/gmacros.h:351:35: error: operand of ?: changes signedness from ‘gssize’ {aka ‘long int’} to ‘long unsigned int’ due to unsignedness of other operand [-Werror=sign-compare]
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
^~~
glib/gstring.c:464:22: note: in expansion of macro ‘MIN’
precount = MIN (len, pos - offset);
^~~
glib/gstring.c:469:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (len > precount)
^
glib/gstring.c:481:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (pos < string->len)
^
In file included from glib/glibconfig.h:9,
from glib/gtypes.h:32,
from glib/gstring.h:32,
from glib/gstring.c:37:
glib/gstring.c: In function ‘g_string_insert_c’:
glib/gstring.c:782:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
g_return_val_if_fail (pos <= string->len, string);
^~
glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’
#define G_LIKELY(expr) (expr)
^~~~
glib/gstring.c:782:5: note: in expansion of macro ‘g_return_val_if_fail’
g_return_val_if_fail (pos <= string->len, string);
^~~~~~~~~~~~~~~~~~~~
glib/gstring.c:785:11: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (pos < string->len)
^
In file included from glib/glibconfig.h:9,
from glib/gtypes.h:32,
from glib/gstring.h:32,
from glib/gstring.c:37:
glib/gstring.c: In function ‘g_string_insert_unichar’:
glib/gstring.c:857:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
g_return_val_if_fail (pos <= string->len, string);
^~
glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’
#define G_LIKELY(expr) (expr)
^~~~
glib/gstring.c:857:5: note: in expansion of macro ‘g_return_val_if_fail’
g_return_val_if_fail (pos <= string->len, string);
^~~~~~~~~~~~~~~~~~~~
glib/gstring.c:860:11: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (pos < string->len)
^
In file included from glib/glibconfig.h:9,
from glib/gtypes.h:32,
from glib/gstring.h:32,
from glib/gstring.c:37:
glib/gstring.c: In function ‘g_string_erase’:
glib/gstring.c:969:29: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
g_return_val_if_fail (pos <= string->len, string);
^~
glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’
#define G_LIKELY(expr) (expr)
^~~~
glib/gstring.c:969:3: note: in expansion of macro ‘g_return_val_if_fail’
g_return_val_if_fail (pos <= string->len, string);
^~~~~~~~~~~~~~~~~~~~
glib/gstring.c:975:39: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
g_return_val_if_fail (pos + len <= string->len, string);
^~
glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’
#define G_LIKELY(expr) (expr)
^~~~
glib/gstring.c:975:7: note: in expansion of macro ‘g_return_val_if_fail’
g_return_val_if_fail (pos + len <= string->len, string);
^~~~~~~~~~~~~~~~~~~~
glib/gstring.c:977:21: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (pos + len < string->len)
^
`SOCKS4_CONN_MSG_LEN` failed to account for the length of the final nul
byte in the connect message, which is an addition in SOCKSv4a vs
SOCKSv4.
This means that the buffer for building and transmitting the connect
message could be overflowed if the username and hostname are both
`SOCKS4_MAX_LEN` (255) bytes long.
Proxy configurations are normally statically configured, so the username
is very unlikely to be near its maximum length, and hence this overflow
is unlikely to be triggered in practice.
(Commit message by Philip Withnall, diagnosis and fix by Michael
Catanzaro.)
Fixes: #3461
Currently, the trash functionality is disabled for system internal mounts.
That might be a problem in some cases. The `x-gvfs-notrash` mount option
allows disabling the trash functionality for certain mounts. Let's add
support for the `x-gvfs-trash` mount option to allow the opposite.
See: https://issues.redhat.com/browse/RHEL-46828
Add support for x-gvfs-notrash mount option, which allows to disable
trash functionality for certain mounts. This might be especially useful
e.g. to prevent trash folder creation on enterprise shares, which are
also accessed from Windows...
https://bugzilla.redhat.com/show_bug.cgi?id=1096200
There is already g_unix_mount_at function which allows to find certain
unix mount for given mount path. It would be useful to have similar
function for mount points, which will allow to replace custom codes in
gvfs. Let's add g_unix_mount_point_at.
GVfsUDisks2VolumeMonitor handles x-gvfs-hide/x-gvfs-show mount options
used to overwrite our heuristics whether the mount should be shown, or
hidden. Unfortunately, it works currently only for mounts with
corresponding fstab entries, because the options are read over
g_unix_mount_point_get_options. Let's introduce g_unix_mount_get_options
to allow reading of the options for all sort of mounts (e.g. created
over pam_mount, or manually mounted).
(Minor fixes to the documentation by Philip Withnall
<withnall@endlessm.com>.)
https://bugzilla.gnome.org/show_bug.cgi?id=668132
Without gatomic.h, build fails on:
In file included from garcbox.c:24:0:
garcbox.c: In function ‘g_atomic_rc_box_acquire’:
grefcount.h:101:13: error: implicit declaration of function ‘g_atomic_int_get’; did you mean ‘__atomic_store’? [-Werror=implicit-function-declaration]
(void) (g_atomic_int_get (rc) == G_MAXINT ? 0 : g_atomic_int_inc ((rc))); \
^
garcbox.c:292:3: note: in expansion of macro ‘g_atomic_ref_count_inc’
g_atomic_ref_count_inc (&real_box->ref_count);
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
If we're using GCC we can use __extension__ to inline the grefcount and
gatomicrefcount API, and avoid the function call.
These macros are only enabled if G_DISABLE_CHECKS is defined, as they
remove critical warnings when the reference counters achieve saturation.
We have a common pattern for reference counting in GLib, but we always
implement it with ad hoc code. This is a good chance at trying to
standardise the implementation and make it public, so that other code
using GLib can take advantage of shared behaviour and semantics.
Instead of simply taking an integer variable, we should create type
aliases, to immediately distinguish the reference counting semantics of
the code; we can handle mixing atomic reference counting with a
non-atomic type (and vice versa) by using differently signed values for
the atomic and non-atomic cases.
The gatomicrefcount type is modelled on the Linux kernel refcount_t
type; the grefcount type is added to let single-threaded code bases to
avoid paying the price of atomic memory barriers on reference counting
operations.
If a copy operation is started with `G_FILE_COPY_TARGET_DEFAULT_PERMS`,
don’t create the destination file as private. Instead, create it with
the process’ current umask (i.e. ‘default permissions’).
This is a partial re-work of commit d8f8f4d637, with
input from Ondrej Holy.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #174
These generate basic .c and .h files containing the GDBusInterfaceInfo
for a D-Bus introspection XML file, but no other code (no skeletons,
proxies, GObjects, etc.).
This is useful for projects who want to describe their D-Bus interfaces
using introspection XML, but who wish to implement the interfaces
manually (for various reasons, typically because the skeletons generated
by gdbus-codegen are too simplistic and limiting). Previously, these
projects would have had to write the GDBusInterfaceInfo manually, which
is painstaking and error-prone.
The new --interface-info-[body|header] options are very similar to
--[body|header], but mutually exclusive with them.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=795304
Recursive annotations do seem to be supported, so we should support them
properly in the type system representation. This currently introduces no
behavioural changes, but will be used in upcoming commits.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=795304
`NM_STATE_CONNECTED_SITE` is documented to mean that a default route is
available, but that the internet connectivity check failed. A default
route being available is compatible with the documentation for
GNetworkMonitor:network-available, which should be true if the system
has a default route for at least one of IPv4 and IPv6.
https://developer.gnome.org/NetworkManager/stable/nm-dbus-types.html
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1788
The network-available property can be asserted by querying the NMState
describing the current overval network state, instead of the
NMConnectivityState. The advantage of the NMState is that is reflects
immediately the network state modification, while the connectivity
state is tested at a fixed frequency.
