|
 |
64ccc2 |
From 598eecf5c1c948535ca626833bc5cea59060913f Mon Sep 17 00:00:00 2001
|
|
|
15abaf |
From: Lennart Poettering <lennart@poettering.net>
|
|
|
15abaf |
Date: Wed, 20 Apr 2022 22:30:22 +0200
|
|
|
15abaf |
Subject: [PATCH] sd-bus: switch to a manual overflow check in
|
|
|
15abaf |
sd_bus_track_add_name()
|
|
|
15abaf |
|
|
|
15abaf |
This is generally used in a directly client controllable way, hence we
|
|
|
15abaf |
should handle ref count overflow gracefully, instead of hitting an
|
|
|
15abaf |
assert().
|
|
|
15abaf |
|
|
|
15abaf |
As discussed:
|
|
|
15abaf |
|
|
|
15abaf |
https://github.com/systemd/systemd/pull/23099#discussion_r854341850
|
|
|
15abaf |
(cherry picked from commit 7f40cb7c86b0fff3a82096a9499570bad9c19fd2)
|
|
|
15abaf |
|
|
|
15abaf |
[msekleta: We've never switched to using track_item_ref/unref introduced
|
|
|
15abaf |
in c2d7dd35d2 hence we still had potential undefined behavior related to
|
|
|
15abaf |
overflow check and this commit fixes that.]
|
|
|
15abaf |
|
|
|
64ccc2 |
Related: #2047373
|
|
|
15abaf |
---
|
|
|
15abaf |
src/libsystemd/sd-bus/bus-track.c | 10 +++++++---
|
|
|
15abaf |
1 file changed, 7 insertions(+), 3 deletions(-)
|
|
|
15abaf |
|
|
|
15abaf |
diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c
|
|
|
15abaf |
index 8893f190a1..b818e93bec 100644
|
|
|
15abaf |
--- a/src/libsystemd/sd-bus/bus-track.c
|
|
|
15abaf |
+++ b/src/libsystemd/sd-bus/bus-track.c
|
|
|
15abaf |
@@ -208,12 +208,16 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) {
|
|
|
15abaf |
i = hashmap_get(track->names, name);
|
|
|
15abaf |
if (i) {
|
|
|
15abaf |
if (track->recursive) {
|
|
|
15abaf |
- unsigned k = i->n_ref + 1;
|
|
|
15abaf |
+ assert(i->n_ref > 0);
|
|
|
15abaf |
|
|
|
15abaf |
- if (k < i->n_ref) /* Check for overflow */
|
|
|
15abaf |
+ /* Manual oveflow check (instead of a DEFINE_TRIVIAL_REF_FUNC() helper or so), so
|
|
|
15abaf |
+ * that we can return a proper error, given this is almost always called in a
|
|
|
15abaf |
+ * directly client controllable way, and thus better should never hit an assertion
|
|
|
15abaf |
+ * here. */
|
|
|
15abaf |
+ if (i->n_ref >= UINT_MAX)
|
|
|
15abaf |
return -EOVERFLOW;
|
|
|
15abaf |
|
|
|
15abaf |
- i->n_ref = k;
|
|
|
15abaf |
+ i->n_ref++;
|
|
|
15abaf |
}
|
|
|
15abaf |
|
|
|
15abaf |
bus_track_remove_from_queue(track);
|