[pulseaudio-discuss] [PATCH 2/2] pulsecore: srbchannel: Introduce per-client SHM files
David Henningsson
david.henningsson at canonical.com
Fri Aug 28 05:58:01 PDT 2015
Looks good as a start, but notice that this will not fix security, as
the audio is still routed over the ordinary mempool.
Would be interesting to know how this affects memory usage though.
On 2015-08-28 14:48, Ahmed S. Darwish wrote:
> The PA daemon currently uses a system-wide SHM file for all clients
> sending and receiving commands using the srbchannel low-latency
> mechanism.
>
> To be able to safely run PA daemon in system mode later using memfds,
> and to provide the necessary ground work for policy and sandboxing,
> create the srbchannel SHM files on a per-client basis.
>
> Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
> ---
> src/pulsecore/client.c | 5 +++++
> src/pulsecore/client.h | 7 +++++++
> src/pulsecore/core.c | 15 +++++----------
> src/pulsecore/core.h | 6 ++----
> src/pulsecore/protocol-native.c | 16 ++++++++++++----
> 5 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/src/pulsecore/client.c b/src/pulsecore/client.c
> index 003bcf8..5d60ad6 100644
> --- a/src/pulsecore/client.c
> +++ b/src/pulsecore/client.c
> @@ -69,6 +69,8 @@ pa_client *pa_client_new(pa_core *core, pa_client_new_data *data) {
> c->sink_inputs = pa_idxset_new(NULL, NULL);
> c->source_outputs = pa_idxset_new(NULL, NULL);
>
> + c->rw_mempool = NULL;
> +
> c->userdata = NULL;
> c->kill = NULL;
> c->send_event = NULL;
> @@ -105,6 +107,9 @@ void pa_client_free(pa_client *c) {
> pa_assert(pa_idxset_isempty(c->source_outputs));
> pa_idxset_free(c->source_outputs, NULL);
>
> + if (c->rw_mempool)
> + pa_mempool_free(c->rw_mempool);
> +
> pa_proplist_free(c->proplist);
> pa_xfree(c->driver);
> pa_xfree(c);
> diff --git a/src/pulsecore/client.h b/src/pulsecore/client.h
> index cd55348..67337a4 100644
> --- a/src/pulsecore/client.h
> +++ b/src/pulsecore/client.h
> @@ -43,6 +43,13 @@ struct pa_client {
> pa_idxset *sink_inputs;
> pa_idxset *source_outputs;
>
> + /* This is the per-client read/write memory pool used for srbchannel
> + * support. Both server and client can write to this.
> + *
> + * This can be NULL if our connection with the client does not
> + * support srbchannel communication. */
> + pa_mempool *rw_mempool;
> +
> void *userdata;
>
> void (*kill)(pa_client *c);
> diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c
> index d198e48..a75e4a8 100644
> --- a/src/pulsecore/core.c
> +++ b/src/pulsecore/core.c
> @@ -126,11 +126,6 @@ pa_core* pa_core_new(pa_mainloop_api *m, bool shared, size_t shm_size) {
> c->shm_size = shm_size;
> pa_silence_cache_init(&c->silence_cache);
>
> - if (shared && !(c->rw_mempool = pa_mempool_new(shared, shm_size)))
> - pa_log_warn("Failed to allocate shared writable memory pool.");
> - if (c->rw_mempool)
> - pa_mempool_set_is_remote_writable(c->rw_mempool, true);
> -
> c->exit_event = NULL;
> c->scache_auto_unload_event = NULL;
>
> @@ -217,8 +212,6 @@ static void core_free(pa_object *o) {
> pa_assert(!c->default_sink);
>
> pa_silence_cache_done(&c->silence_cache);
> - if (c->rw_mempool)
> - pa_mempool_free(c->rw_mempool);
> pa_mempool_free(c->mempool);
>
> for (j = 0; j < PA_CORE_HOOK_MAX; j++)
> @@ -262,13 +255,14 @@ int pa_core_exit(pa_core *c, bool force, int retval) {
>
> void pa_core_maybe_vacuum(pa_core *c) {
> pa_assert(c);
> + pa_client *client;
> + uint32_t idx;
>
> if (pa_idxset_isempty(c->sink_inputs) && pa_idxset_isempty(c->source_outputs)) {
> pa_log_debug("Hmm, no streams around, trying to vacuum.");
> } else {
> pa_sink *si;
> pa_source *so;
> - uint32_t idx;
>
> idx = 0;
> PA_IDXSET_FOREACH(si, c->sinks, idx)
> @@ -285,8 +279,9 @@ void pa_core_maybe_vacuum(pa_core *c) {
>
> pa_mempool_vacuum(c->mempool);
>
> - if (c->rw_mempool)
> - pa_mempool_vacuum(c->rw_mempool);
> + PA_IDXSET_FOREACH(client, c->clients, idx)
> + if (client->rw_mempool)
> + pa_mempool_vacuum(client->rw_mempool);
> }
>
> pa_time_event* pa_core_rttime_new(pa_core *c, pa_usec_t usec, pa_time_event_cb_t cb, void *userdata) {
> diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
> index 29680cb..fe2d9b6 100644
> --- a/src/pulsecore/core.h
> +++ b/src/pulsecore/core.h
> @@ -178,10 +178,8 @@ struct pa_core {
> PA_LLIST_HEAD(pa_subscription_event, subscription_event_queue);
> pa_subscription_event *subscription_event_last;
>
> - /* The mempool is used for data we write to, it's readonly for the client.
> - The rw_mempool is used for data writable by both server and client (and
> - can be NULL in some cases). */
> - pa_mempool *mempool, *rw_mempool;
> + /* The mempool is used for data we write to, it's readonly for the client. */
> + pa_mempool *mempool;
>
> /* Shared memory size, as specified either by daemon configuration
> * or PA daemon defaults (~ 64 MiB). */
> diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
> index ec223be..5a062a6 100644
> --- a/src/pulsecore/protocol-native.c
> +++ b/src/pulsecore/protocol-native.c
> @@ -2585,6 +2585,7 @@ static void command_exit(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_ta
> }
>
> static void setup_srbchannel(pa_native_connection *c) {
> + pa_mempool *rw_mempool = NULL;
> pa_srbchannel_template srbt;
> pa_srbchannel *srb;
> pa_memchunk mc;
> @@ -2606,15 +2607,17 @@ static void setup_srbchannel(pa_native_connection *c) {
> return;
> }
>
> - if (!c->protocol->core->rw_mempool) {
> - pa_log_debug("Disabling srbchannel, reason: No rw memory pool");
> + if (!(rw_mempool = pa_mempool_new(true, c->protocol->core->shm_size))) {
> + pa_log_warn("Disabling srbchannel, reason: Failed to allocate shared "
> + "writable memory pool.");
> return;
> }
> + pa_mempool_set_is_remote_writable(rw_mempool, true);
>
> - srb = pa_srbchannel_new(c->protocol->core->mainloop, c->protocol->core->rw_mempool);
> + srb = pa_srbchannel_new(c->protocol->core->mainloop, rw_mempool);
> if (!srb) {
> pa_log_debug("Failed to create srbchannel");
> - return;
> + goto free_rw_mempool;
> }
> pa_log_debug("Enabling srbchannel...");
> pa_srbchannel_export(srb, &srbt);
> @@ -2633,7 +2636,12 @@ static void setup_srbchannel(pa_native_connection *c) {
> mc.length = pa_memblock_get_length(srbt.memblock);
> pa_pstream_send_memblock(c->pstream, 0, 0, 0, &mc);
>
> + c->client->rw_mempool = rw_mempool;
> c->srbpending = srb;
> + return;
> +
> +free_rw_mempool:
> + pa_mempool_free(rw_mempool);
> }
>
> static void command_enable_srbchannel(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) {
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
More information about the pulseaudio-discuss
mailing list