sd-bus: drop half-registered vtable members on failure
add_object_vtable_internal() inserts BusVTableMember entries into the
global vtable_methods/vtable_properties sets as it walks the vtable, but
only sets node_vtable.node (which gates the disconnect-time cleanup)
after the loop. A failure mid-loop thus left the already-inserted
members behind, dangling once the slot's interface string and the node's
path were freed, so a later registration reading those keys hit a
use-after-free. Remove this slot's members on the fail path.
The test asserts the failed registration leaves no member behind.
Follow-up for 19befb2d5f
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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"));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user