diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 795cc68837..aa371cceca 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -2099,6 +2099,37 @@ static int add_object_vtable_internal( return 0; fail: + /* Remove members we already inserted into the global tables before failing (identified by + * parent), so they don't dangle once the slot and node are freed below. */ + if (s && s->node_vtable.interface) + for (v = bus_vtable_next(vtable, vtable); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(vtable, v)) { + BusVTableMember key, *x; + Set *set; + + if (v->type == _SD_BUS_VTABLE_METHOD) { + set = bus->vtable_methods; + key.member = v->x.method.member; + } else if (IN_SET(v->type, _SD_BUS_VTABLE_PROPERTY, _SD_BUS_VTABLE_WRITABLE_PROPERTY)) { + set = bus->vtable_properties; + key.member = v->x.property.member; + } else + continue; + + /* The entry that failed validation may carry a NULL member, which was never + * inserted and would crash the hashmap lookup below. Skip it. */ + if (isempty(key.member)) + continue; + + key.path = n->path; + key.interface = s->node_vtable.interface; + + x = set_get(set, &key); + if (x && x->parent == &s->node_vtable) { + assert_se(set_remove(set, &key) == x); + free(x); + } + } + sd_bus_slot_unref(s); bus_node_gc(bus, n); diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c index ac33086a6f..5d8e772cb4 100644 --- a/src/libsystemd/sd-bus/test-bus-objects.c +++ b/src/libsystemd/sd-bus/test-bus-objects.c @@ -7,6 +7,7 @@ #include "bus-internal.h" #include "bus-message.h" #include "log.h" +#include "set.h" #include "strv.h" #include "tests.h" @@ -566,6 +567,25 @@ static int client(void *p) { return 0; } +static const sd_bus_vtable partial_vtable[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("Foo", NULL, NULL, NULL, 0), + SD_BUS_METHOD("has space", NULL, NULL, NULL, 0), /* invalid member name, registration fails here */ + SD_BUS_VTABLE_END +}; + +static void test_vtable_partial_failure(void) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + ASSERT_OK(sd_bus_new(&bus)); + + /* Registration fails at the invalid member, after "Foo" was already inserted into the global vtable + * member tables. Check that the fail path removes it again. */ + ASSERT_ERROR(sd_bus_add_object_vtable(bus, NULL, "/x", "org.test.iface", partial_vtable, NULL), EINVAL); + + ASSERT_TRUE(set_isempty(bus->vtable_methods)); +} + int main(int argc, char *argv[]) { _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(sd_future_unrefp) sd_future *f_server = NULL, *f_client = NULL; @@ -573,6 +593,8 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); + test_vtable_partial_failure(); + c.automatic_integer_property = 4711; ASSERT_NOT_NULL(c.automatic_string_property = strdup("dudeldu"));