mirror of
https://gitlab.freedesktop.org/wayland/wayland.git
synced 2026-05-02 06:46:26 -04:00
client: Don't disconnect on receipt of object events destined for zombies
Server side objects give protocol designers exciting ways to break clients. For example, if a client deletes an object at the same time the server is sending an event containing a new object to that object, then we currently silently drop that event. If a following event in the buffer from an object that has not yet been deleted also contains a new object, the wl_map constraint that new objects must be 1 higher than the current highest object count is violated. This results in a disconnect. Instead, let's augment the zombie accounting code to keep the entire proxy around on deletion, for both client and server generated objects. This way we can create and immediately delete objects that are destined for zombie proxies - thus creating zombie descendants. We can go no further to clean this up in the client library - we can't call a destructor because the protocol might dictate that child objects will be automatically destroyed on the destruction of the parent. So we turn a situation that would lead to an erroneous disconnect into one that may or may not leak object ids depending on protocol definition. Fixes #74 for some definition of "fix" anyway. Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This commit is contained in:
Derek Foreman
parent
f736f11f99
commit
c05f4f86ea
5 changed files with 218 additions and 107 deletions
|
|
@ -49,4 +49,35 @@
|
|||
<arg name="passer" type="object" interface="fd_passer"/>
|
||||
</request>
|
||||
</interface>
|
||||
|
||||
<interface name="noop" version="1">
|
||||
<description summary="An interface that does nothing">
|
||||
A trivial interface.
|
||||
</description>
|
||||
|
||||
<request name="destroy" type="destructor"/>
|
||||
</interface>
|
||||
|
||||
<interface name="necromancer" version="1">
|
||||
<description summary="Helper for creating zombie objects">
|
||||
Simple interface for generating server side objects to use in
|
||||
zombie tests.
|
||||
</description>
|
||||
|
||||
<request name="destroy" type="destructor"/>
|
||||
|
||||
<request name="create_server_object">
|
||||
<description summary="create a new server side object">
|
||||
Create a new server side object.
|
||||
</description>
|
||||
</request>
|
||||
|
||||
<event name="server_object">
|
||||
<description summary="server side object">
|
||||
</description>
|
||||
<arg name="id" type="new_id" interface="noop" summary="server side object"/>
|
||||
</event>
|
||||
|
||||
|
||||
</interface>
|
||||
</protocol>
|
||||
|
|
|
|||
|
|
@ -200,12 +200,6 @@ close_fds(struct wl_ring_buffer *buffer, int max)
|
|||
buffer->tail += size;
|
||||
}
|
||||
|
||||
void
|
||||
wl_connection_close_fds_in(struct wl_connection *connection, int max)
|
||||
{
|
||||
close_fds(&connection->fds_in, max);
|
||||
}
|
||||
|
||||
int
|
||||
wl_connection_destroy(struct wl_connection *connection)
|
||||
{
|
||||
|
|
@ -809,7 +803,8 @@ wl_connection_demarshal(struct wl_connection *connection,
|
|||
goto err;
|
||||
}
|
||||
|
||||
if (wl_map_reserve_new(objects, id) < 0) {
|
||||
if (wl_map_reserve_new(objects, id) < 0 &&
|
||||
!wl_object_is_zombie(objects, id)) {
|
||||
wl_log("not a valid new object id (%u), "
|
||||
"message %s(%s)\n",
|
||||
id, message->name, message->signature);
|
||||
|
|
@ -879,10 +874,6 @@ wl_object_is_zombie(struct wl_map *map, uint32_t id)
|
|||
if (map->side == WL_MAP_SERVER_SIDE)
|
||||
return false;
|
||||
|
||||
/* Zombie objects can only have been created by the client. */
|
||||
if (id >= WL_SERVER_ID_START)
|
||||
return false;
|
||||
|
||||
flags = wl_map_lookup_flags(map, id);
|
||||
return !!(flags & WL_MAP_ENTRY_ZOMBIE);
|
||||
}
|
||||
|
|
@ -909,8 +900,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
|
|||
|
||||
object = wl_map_lookup(objects, id);
|
||||
if (wl_object_is_zombie(objects, id)) {
|
||||
/* references object we've already
|
||||
* destroyed client side */
|
||||
/* references object we've already destroyed */
|
||||
object = NULL;
|
||||
} else if (object == NULL && id != 0) {
|
||||
wl_log("unknown object (%u), message %s(%s)\n",
|
||||
|
|
|
|||
|
|
@ -55,11 +55,6 @@ enum wl_proxy_flag {
|
|||
WL_PROXY_FLAG_WRAPPER = (1 << 2),
|
||||
};
|
||||
|
||||
struct wl_zombie {
|
||||
int event_count;
|
||||
int *fd_count;
|
||||
};
|
||||
|
||||
struct wl_proxy {
|
||||
struct wl_object object;
|
||||
struct wl_display *display;
|
||||
|
|
@ -352,62 +347,11 @@ wl_display_create_queue(struct wl_display *display)
|
|||
return queue;
|
||||
}
|
||||
|
||||
static int
|
||||
message_count_fds(const char *signature)
|
||||
{
|
||||
unsigned int count, i, fds = 0;
|
||||
struct argument_details arg;
|
||||
|
||||
count = arg_count_for_signature(signature);
|
||||
for (i = 0; i < count; i++) {
|
||||
signature = get_next_argument(signature, &arg);
|
||||
if (arg.type == 'h')
|
||||
fds++;
|
||||
}
|
||||
|
||||
return fds;
|
||||
}
|
||||
|
||||
static struct wl_zombie *
|
||||
prepare_zombie(struct wl_proxy *proxy)
|
||||
{
|
||||
const struct wl_interface *interface = proxy->object.interface;
|
||||
const struct wl_message *message;
|
||||
int i, count;
|
||||
struct wl_zombie *zombie = NULL;
|
||||
|
||||
/* If we hit an event with an FD, ensure we have a zombie object and
|
||||
* fill the fd_count slot for that event with the number of FDs for
|
||||
* that event. Interfaces with no events containing FDs will not have
|
||||
* zombie objects created. */
|
||||
for (i = 0; i < interface->event_count; i++) {
|
||||
message = &interface->events[i];
|
||||
count = message_count_fds(message->signature);
|
||||
|
||||
if (!count)
|
||||
continue;
|
||||
|
||||
if (!zombie) {
|
||||
zombie = zalloc(sizeof(*zombie) +
|
||||
(interface->event_count * sizeof(int)));
|
||||
if (!zombie)
|
||||
return NULL;
|
||||
|
||||
zombie->event_count = interface->event_count;
|
||||
zombie->fd_count = (int *) &zombie[1];
|
||||
}
|
||||
|
||||
zombie->fd_count[i] = count;
|
||||
}
|
||||
|
||||
return zombie;
|
||||
}
|
||||
|
||||
static enum wl_iterator_result
|
||||
free_zombies(void *element, void *data, uint32_t flags)
|
||||
{
|
||||
if (flags & WL_MAP_ENTRY_ZOMBIE)
|
||||
free(element);
|
||||
wl_proxy_unref(element);
|
||||
|
||||
return WL_ITERATOR_CONTINUE;
|
||||
}
|
||||
|
|
@ -471,7 +415,7 @@ static struct wl_proxy *
|
|||
wl_proxy_create_for_id(struct wl_proxy *factory,
|
||||
uint32_t id, const struct wl_interface *interface)
|
||||
{
|
||||
struct wl_proxy *proxy;
|
||||
struct wl_proxy *proxy, *zombie;
|
||||
struct wl_display *display = factory->display;
|
||||
|
||||
proxy = zalloc(sizeof *proxy);
|
||||
|
|
@ -484,6 +428,9 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
|
|||
proxy->queue = factory->queue;
|
||||
proxy->refcount = 1;
|
||||
proxy->version = factory->version;
|
||||
zombie = wl_map_lookup(&display->objects, id);
|
||||
if (zombie && wl_object_is_zombie(&display->objects, id))
|
||||
wl_proxy_unref(zombie);
|
||||
|
||||
wl_map_insert_at(&display->objects, 0, id, proxy);
|
||||
|
||||
|
|
@ -493,25 +440,21 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
|
|||
static void
|
||||
proxy_destroy(struct wl_proxy *proxy)
|
||||
{
|
||||
proxy->flags |= WL_PROXY_FLAG_DESTROYED;
|
||||
|
||||
if (proxy->flags & WL_PROXY_FLAG_ID_DELETED) {
|
||||
wl_map_remove(&proxy->display->objects, proxy->object.id);
|
||||
} else if (proxy->object.id < WL_SERVER_ID_START) {
|
||||
struct wl_zombie *zombie = prepare_zombie(proxy);
|
||||
|
||||
/* The map now contains the zombie entry, until the delete_id
|
||||
* event arrives. */
|
||||
wl_proxy_unref(proxy);
|
||||
} else {
|
||||
/* The map now contains the zombie entry. A delete_id
|
||||
* event will clean up client generated ids eventually,
|
||||
* server generated ids will linger until they're
|
||||
* overwritten. */
|
||||
wl_map_insert_at(&proxy->display->objects,
|
||||
WL_MAP_ENTRY_ZOMBIE,
|
||||
proxy->object.id,
|
||||
zombie);
|
||||
} else {
|
||||
wl_map_insert_at(&proxy->display->objects, 0,
|
||||
proxy->object.id, NULL);
|
||||
proxy);
|
||||
}
|
||||
|
||||
proxy->flags |= WL_PROXY_FLAG_DESTROYED;
|
||||
|
||||
wl_proxy_unref(proxy);
|
||||
}
|
||||
|
||||
static void
|
||||
|
|
@ -1034,9 +977,7 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
|
|||
proxy = wl_map_lookup(&display->objects, id);
|
||||
|
||||
if (wl_object_is_zombie(&display->objects, id)) {
|
||||
/* For zombie objects, the 'proxy' is actually the zombie
|
||||
* event-information structure, which we can free. */
|
||||
free(proxy);
|
||||
wl_proxy_unref(proxy);
|
||||
wl_map_remove(&display->objects, id);
|
||||
} else if (proxy) {
|
||||
proxy->flags |= WL_PROXY_FLAG_ID_DELETED;
|
||||
|
|
@ -1411,6 +1352,7 @@ create_proxies(struct wl_proxy *sender, struct wl_closure *closure)
|
|||
if (proxy == NULL)
|
||||
return -1;
|
||||
closure->args[i].o = (struct wl_object *)proxy;
|
||||
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
|
|
@ -1447,6 +1389,30 @@ increase_closure_args_refcount(struct wl_closure *closure)
|
|||
closure->proxy->refcount++;
|
||||
}
|
||||
|
||||
static void
|
||||
inter_zombie_closure(struct wl_closure *closure)
|
||||
{
|
||||
const char *signature;
|
||||
struct argument_details arg;
|
||||
int i, count;
|
||||
struct wl_proxy *proxy;
|
||||
|
||||
signature = closure->message->signature;
|
||||
count = arg_count_for_signature(signature);
|
||||
for (i = 0; i < count; i++) {
|
||||
signature = get_next_argument(signature, &arg);
|
||||
switch (arg.type) {
|
||||
case 'n':
|
||||
proxy = (struct wl_proxy *) closure->args[i].o;
|
||||
if (proxy)
|
||||
proxy_destroy(proxy);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static int
|
||||
queue_event(struct wl_display *display, int len)
|
||||
{
|
||||
|
|
@ -1458,7 +1424,7 @@ queue_event(struct wl_display *display, int len)
|
|||
struct wl_event_queue *queue;
|
||||
struct timespec tp;
|
||||
unsigned int time;
|
||||
int num_zombie_fds;
|
||||
bool zombie = false;
|
||||
|
||||
wl_connection_copy(display->connection, p, sizeof p);
|
||||
id = p[0];
|
||||
|
|
@ -1467,29 +1433,18 @@ queue_event(struct wl_display *display, int len)
|
|||
if (len < size)
|
||||
return 0;
|
||||
|
||||
/* If our proxy is gone or a zombie, just eat the event (and any FDs,
|
||||
* if applicable). */
|
||||
/* If our proxy is gone, just eat the event. */
|
||||
proxy = wl_map_lookup(&display->objects, id);
|
||||
if (!proxy || wl_object_is_zombie(&display->objects, id)) {
|
||||
struct wl_zombie *zombie = wl_map_lookup(&display->objects, id);
|
||||
num_zombie_fds = (zombie && opcode < zombie->event_count) ?
|
||||
zombie->fd_count[opcode] : 0;
|
||||
|
||||
if (!proxy) {
|
||||
if (debug_client) {
|
||||
clock_gettime(CLOCK_REALTIME, &tp);
|
||||
time = (tp.tv_sec * 1000000L) + (tp.tv_nsec / 1000);
|
||||
|
||||
fprintf(stderr, "[%7u.%03u] discarded [%s]@%d.[event %d]"
|
||||
"(%d fd, %d byte)\n",
|
||||
fprintf(stderr, "[%7u.%03u] discarded @%d.[event %d]"
|
||||
"(%d byte)\n",
|
||||
time / 1000, time % 1000,
|
||||
zombie ? "zombie" : "unknown",
|
||||
id, opcode,
|
||||
num_zombie_fds, size);
|
||||
id, opcode, size);
|
||||
}
|
||||
if (num_zombie_fds > 0)
|
||||
wl_connection_close_fds_in(display->connection,
|
||||
num_zombie_fds);
|
||||
|
||||
wl_connection_consume(display->connection, size);
|
||||
return size;
|
||||
}
|
||||
|
|
@ -1516,6 +1471,13 @@ queue_event(struct wl_display *display, int len)
|
|||
return -1;
|
||||
}
|
||||
|
||||
zombie = wl_object_is_zombie(&display->objects, id);
|
||||
if (zombie) {
|
||||
inter_zombie_closure(closure);
|
||||
wl_closure_destroy(closure);
|
||||
return size;
|
||||
}
|
||||
|
||||
closure->proxy = proxy;
|
||||
increase_closure_args_refcount(closure);
|
||||
|
||||
|
|
|
|||
|
|
@ -232,7 +232,4 @@ zalloc(size_t s)
|
|||
return calloc(1, s);
|
||||
}
|
||||
|
||||
void
|
||||
wl_connection_close_fds_in(struct wl_connection *connection, int max);
|
||||
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -1629,3 +1629,134 @@ TEST(global_remove)
|
|||
|
||||
display_destroy(d);
|
||||
}
|
||||
|
||||
struct new_zombie_data {
|
||||
struct necromancer *necromancer;
|
||||
struct necromancer *necromancer_two;
|
||||
struct noop *temp_noop;
|
||||
};
|
||||
|
||||
static void
|
||||
server_object(void *data, struct necromancer *necro, struct noop *noop)
|
||||
{
|
||||
struct new_zombie_data *nzd = data;
|
||||
|
||||
/* We should only see this once, since one of the two noop
|
||||
* objects we create should be created as a zombie */
|
||||
assert(!nzd->temp_noop);
|
||||
nzd->temp_noop = noop;
|
||||
}
|
||||
|
||||
struct necromancer_listener necromancer_listener = {
|
||||
server_object,
|
||||
};
|
||||
|
||||
static void
|
||||
new_zombie_handle_globals(void *data, struct wl_registry *registry,
|
||||
uint32_t id, const char *intf, uint32_t ver)
|
||||
{
|
||||
struct new_zombie_data *rg = data;
|
||||
|
||||
if (!strcmp(intf, "necromancer")) {
|
||||
rg->necromancer = wl_registry_bind(registry, id, &necromancer_interface, 1);
|
||||
rg->necromancer_two = wl_registry_bind(registry, id, &necromancer_interface, 1);
|
||||
necromancer_add_listener(rg->necromancer, &necromancer_listener, NULL);
|
||||
necromancer_add_listener(rg->necromancer_two, &necromancer_listener, rg);
|
||||
}
|
||||
}
|
||||
|
||||
static const struct wl_registry_listener new_zombie_registry_listener = {
|
||||
new_zombie_handle_globals,
|
||||
NULL
|
||||
};
|
||||
|
||||
static void noop_destructor(struct wl_client *client, struct wl_resource *res)
|
||||
{
|
||||
wl_resource_destroy(res);
|
||||
}
|
||||
|
||||
static const struct noop_interface noop_server_interface = {
|
||||
noop_destructor
|
||||
};
|
||||
|
||||
static void
|
||||
send_new_object(struct wl_client *client, struct wl_resource *res)
|
||||
{
|
||||
struct wl_resource *new_res;
|
||||
|
||||
new_res = wl_resource_create(client, &noop_interface, 1, 0);
|
||||
wl_resource_set_implementation(new_res, &noop_server_interface, NULL, NULL);
|
||||
necromancer_send_server_object(res, new_res);
|
||||
}
|
||||
|
||||
static void necromancer_destructor(struct wl_client *client, struct wl_resource *res)
|
||||
{
|
||||
wl_resource_destroy(res);
|
||||
}
|
||||
|
||||
static const struct necromancer_interface necro_interface = {
|
||||
necromancer_destructor,
|
||||
send_new_object
|
||||
};
|
||||
|
||||
static void
|
||||
bind_necromancer(struct wl_client *client, void *data,
|
||||
uint32_t vers, uint32_t id)
|
||||
{
|
||||
struct wl_resource *res;
|
||||
|
||||
res = wl_resource_create(client, &necromancer_interface, vers, id);
|
||||
wl_resource_set_implementation(res, &necro_interface, NULL, NULL);
|
||||
}
|
||||
|
||||
static void
|
||||
spawn_new_zombie(void *data)
|
||||
{
|
||||
struct new_zombie_data rg = {};
|
||||
struct client *c = client_connect();
|
||||
struct wl_registry *reg;
|
||||
|
||||
reg = wl_display_get_registry(c->wl_display);
|
||||
wl_registry_add_listener(reg, &new_zombie_registry_listener, &rg);
|
||||
|
||||
/* Get the globals */
|
||||
wl_display_roundtrip(c->wl_display);
|
||||
|
||||
wl_registry_destroy(reg);
|
||||
|
||||
necromancer_create_server_object(rg.necromancer);
|
||||
necromancer_create_server_object(rg.necromancer_two);
|
||||
necromancer_destroy(rg.necromancer);
|
||||
/* We should now have two new_id events on the way,
|
||||
* and the first one will be destined for a zombie
|
||||
* when it arrives. That would violate the wl_map
|
||||
* requirement that a new id can only be one higher
|
||||
* than the current max id in the map if we simply
|
||||
* dropped the first inbound object.
|
||||
*/
|
||||
|
||||
wl_display_roundtrip(c->wl_display);
|
||||
assert(rg.temp_noop);
|
||||
noop_destroy(rg.temp_noop);
|
||||
necromancer_destroy(rg.necromancer_two);
|
||||
wl_display_roundtrip(c->wl_display);
|
||||
client_disconnect(c);
|
||||
}
|
||||
|
||||
TEST(new_zombie)
|
||||
{
|
||||
struct display *d;
|
||||
struct wl_global *g;
|
||||
|
||||
d = display_create();
|
||||
g = wl_global_create(d->wl_display, &necromancer_interface,
|
||||
1, d, bind_necromancer);
|
||||
|
||||
client_create_noarg(d, spawn_new_zombie);
|
||||
|
||||
display_run(d);
|
||||
|
||||
wl_global_destroy(g);
|
||||
|
||||
display_destroy(d);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue