[pulseaudio-discuss] [PATCH] core-util: Blame systemd for not setting up XDG_RUNTIME_DIR correctly

[David Henningsson]

I never got any reply about this from Lennart, but I've heard that since 
the patch was written, this issue has been fixed upstream in systemd.

Probably here:
http://cgit.freedesktop.org/systemd/systemd/commit/?id=baae0358f349870544884e405e82e4be7d8add9f

Then the question becomes whether we need this patch because people are 
running old systemd versions. Do you have a gut feeling for that? Have 
you seen people complaining?

On 2014-06-26 13:15, Tanu Kaskinen wrote:
> Hi David,
>
> Do you still plan to apply this patch? It looks good to me (except
> writing SYSTEMD in all-caps in the log message). The commit message
> probably needs some editing...
>
> -- Tanu
>
>
> On Tue, 2013-11-12 at 07:52 +0100, David Henningsson wrote:
>> Hi Lennart,
>>
>> I think you're wrong about this bug:
>> https://bugzilla.redhat.com/show_bug.cgi?id=753882
>>
>> You're claiming that PA will only work if XDG_RUNTIME_DIR remains the same
>> as the original user. However, PA will use the PULSE_SERVER X11 property instead of
>> using XDG_RUNTIME_DIR, so this environment variable does not matter. Usually.
>>
>> If this property is not available, or if one is using the pacmd cli protocol,
>> the client will go ahead and call pa_make_secure_dir on XDG_RUNTIME_DIR/pulse.
>> This will either fail (if you're another regular user), or succeed (if you're root).
>> Both scenarios are bad - failing will cause the connection to fail, and succeeding
>> is even worse, as it can cause *other* connections to fail (as the directory
>> ownership has changed).
>>
>> Long story short, inserting the original user's XDG_RUNTIME_DIR does not help PA
>> in any use case I can think of - it only makes things worse.
>>
>> Until this behaviour is changed, I suggest PulseAudio defend itself against
>> this bug using the patch below.
>>
>> BugLink: https://bugs.launchpad.net/bugs/1197395
>> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
>> ---
>>   src/pulsecore/core-util.c |    8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
>> index e925918..f8d01e0 100644
>> --- a/src/pulsecore/core-util.c
>> +++ b/src/pulsecore/core-util.c
>> @@ -1758,6 +1758,14 @@ char *pa_get_runtime_dir(void) {
>>       /* Use the XDG standard for the runtime directory. */
>>       d = getenv("XDG_RUNTIME_DIR");
>>       if (d) {
>> +        struct stat st;
>> +        if (stat(d, &st) == 0 && st.st_uid != getuid()) {
>> +            pa_log("XDG_RUNTIME_DIR (%s) is not owned by us (uid %d), but by uid %d.\n"
>> +                   "Most likely this is a bug in systemd. Please report this issue to the SYSTEMD developers.",
>> +                   d, getuid(), st.st_uid);
>> +            goto fail;
>> +        }
>> +
>>           k = pa_sprintf_malloc("%s" PA_PATH_SEP "pulse", d);
>>
>>           if (pa_make_secure_dir(k, m, (uid_t) -1, (gid_t) -1, true) < 0) {
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic