impl-client: check global id registry generation in client.error method

In client.error method, require that the global id in the message refers
to an existing registered global known to the sender of the message.  If
not, do not try forwarding errors to the resources.

Also emit errors on the sender client resource in this case, so that the
sender client gets to know that what it tried to do didn't work.

This addresses a race condition where session manager sends a client
error for a global id, but before server processed that message the
global got deleted and the id reused for a different object, resulting
to an error being sent to the wrong resource. See #3192.

Wireplumber & pipewire-media-session do this on non-reconnectable node
connection failures: they first delete the node, and then try to send a
client error for its id. However, the global and its resources then
usually are already deleted at that point, and there are no resources to
send messages to so that is a no-op, except in the race condition where
id gets reused and the message goes to the wrong object.

They should do it in the opposite order. That it is wrong is also
visible in that

pw-play --target badtarget -P '{ node.dont-reconnect = true }' sample.ogg

hangs instead of bailing out, which is what happens also before this
commit.
This commit is contained in:
Pauli Virtanen 2023-05-06 13:56:35 +03:00
parent 2ed65a7e36
commit a0a5320280

View file

@ -101,20 +101,45 @@ static int error_resource(void *object, void *data)
{
struct pw_resource *r = object;
struct error_data *d = data;
if (r && r->bound_id == d->id)
if (r && r->bound_id == d->id) {
pw_log_debug("%p: client error for global %u: %d (%s)",
r, d->id, d->res, d->error);
pw_resource_error(r, d->res, d->error);
}
return 0;
}
static int client_error(void *object, uint32_t id, int res, const char *error)
{
struct resource_data *data = object;
struct pw_resource *resource = data->resource;
struct pw_impl_client *sender = resource->client;
struct pw_impl_client *client = data->client;
struct error_data d = { id, res, error };
struct pw_global *global;
pw_log_debug("%p: error for global %d", client, id);
/* Check the global id provided by sender refers to a registered global
* known to the sender.
*/
if ((global = pw_context_find_global(resource->context, id)) == NULL)
goto error_no_id;
if (sender->recv_generation != 0 && global->generation > sender->recv_generation)
goto error_stale_id;
pw_log_debug("%p: sender %p: error for global %u", client, sender, id);
pw_map_for_each(&client->objects, error_resource, &d);
return 0;
error_no_id:
pw_log_debug("%p: sender %p: error for invalid global %u", client, sender, id);
pw_resource_errorf(resource, -ENOENT, "no global %u", id);
return -ENOENT;
error_stale_id:
pw_log_debug("%p: sender %p: error for stale global %u generation:%"PRIu64" recv-generation:%"PRIu64,
client, sender, id, global->generation, sender->recv_generation);
pw_resource_errorf(resource, -ESTALE, "no global %u any more", id);
return -ESTALE;
}
static bool has_key(const char * const keys[], const char *key)