[pulseaudio-discuss] [PATCH] pipe-sink,

[Tanu Kaskinen]

We recently changed the umask of the daemon from 022 to 077, which broke
module-pipe-sink in the system mode, because nobody was allowed to read
from the pipe.

module-pipe-source in the system mode was probably always broken,
because the old umask of 022 should prevent anyone from writing to the
pipe.

This patch uses chmod() after the file creation to set the permissions
to 0666, which is what the fkfifo() call tried to set.

Bug link: https://bugs.freedesktop.org/show_bug.cgi?id=107070
---
src/modules/module-pipe-sink.c | 8 +++++++-
src/modules/module-pipe-source.c | 6 ++++++
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/modules/module-pipe-sink.c b/src/modules/module-pipe-sink.c
index 765e75195..213924fdc 100644
--- a/src/modules/module-pipe-sink.c
+++ b/src/modules/module-pipe-sink.c
@@ -466,9 +466,15 @@ int pa__init(pa_module *m) {
pa_log("mkfifo('%s'): %s", u->filename, pa_cstrerror(errno));
goto fail;
}
- } else
+ } else {
u->do_unlink_fifo = true;

+ /* Our umask is 077, so the pipe won't be created with the requested
+ * permissions. Let's fix the permissions with chmod(). */
+ if (chmod(u->filename, 0666) < 0)
+ pa_log_warn("chomd('%s'): %s", u->filename, pa_cstrerror(errno));
+ }
+
if ((u->fd = pa_open_cloexec(u->filename, O_RDWR, 0)) < 0) {
pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
goto fail;
diff --git a/src/modules/module-pipe-source.c b/src/modules/module-pipe-source.c
index f8284c161..74ec0551a 100644
--- a/src/modules/module-pipe-source.c
+++ b/src/modules/module-pipe-source.c
@@ -243,7 +243,13 @@ int pa__init(pa_module *m) {
if (mkfifo(u->filename, 0666) < 0) {
pa_log("mkfifo('%s'): %s", u->filename, pa_cstrerror(errno));
goto fail;
+ } else {
+ /* Our umask is 077, so the pipe won't be created with the requested
+ * permissions. Let's fix the permissions with chmod(). */
+ if (chmod(u->filename, 0666) < 0)
+ pa_log_warn("chomd('%s'): %s", u->filename, pa_cstrerror(errno));
}
+
if ((u->fd = pa_open_cloexec(u->filename, O_RDWR, 0)) < 0) {
pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
goto fail;

[Georg Chini]

Post by Tanu Kaskinen
We recently changed the umask of the daemon from 022 to 077, which broke
module-pipe-sink in the system mode, because nobody was allowed to read
from the pipe.
module-pipe-source in the system mode was probably always broken,
because the old umask of 022 should prevent anyone from writing to the
pipe.
This patch uses chmod() after the file creation to set the permissions
to 0666, which is what the fkfifo() call tried to set.
Bug link: https://bugs.freedesktop.org/show_bug.cgi?id=107070
---

Should the permissions really be 666? Would not 660 be better,
so that there is at least some control who may access the pipe?

[Tanu Kaskinen]

Post by Georg Chini

Post by Tanu Kaskinen
We recently changed the umask of the daemon from 022 to 077, which broke
module-pipe-sink in the system mode, because nobody was allowed to read
from the pipe.
module-pipe-source in the system mode was probably always broken,
because the old umask of 022 should prevent anyone from writing to the
pipe.
This patch uses chmod() after the file creation to set the permissions
to 0666, which is what the fkfifo() call tried to set.
Bug link: https://bugs.freedesktop.org/show_bug.cgi?id=107070
---

Should the permissions really be 666? Would not 660 be better,
so that there is at least some control who may access the pipe?

If the mode were 660, the bug that was reported would not be fixed. In
the system mode the owner and group are "pulse", so nobody would be
able to access the pipe.

I agree that it's questionable to give everyone access, but that's what
we've always done (or at least we've always given read access, but the
intention has been to give write access as well).

If we want to tighten the permissions, that can be done in a separate
patch. We could make the mode configurable and default to 600 in the
user mode and 666 in the system mode. We could also make the group
configurable with "pulse-access" as the default group, then we could
default to 660 in the system mode.

We could also remove write access in case of module-pipe-sink and read
access in case of module-pipe-source.

[Georg Chini]

Post by Tanu Kaskinen

Post by Georg Chini

Post by Tanu Kaskinen
We recently changed the umask of the daemon from 022 to 077, which broke
module-pipe-sink in the system mode, because nobody was allowed to read
from the pipe.
module-pipe-source in the system mode was probably always broken,
because the old umask of 022 should prevent anyone from writing to the
pipe.
This patch uses chmod() after the file creation to set the permissions
to 0666, which is what the fkfifo() call tried to set.
Bug link: https://bugs.freedesktop.org/show_bug.cgi?id=107070
---

Should the permissions really be 666? Would not 660 be better,
so that there is at least some control who may access the pipe?

If the mode were 660, the bug that was reported would not be fixed. In
the system mode the owner and group are "pulse", so nobody would be
able to access the pipe.
I agree that it's questionable to give everyone access, but that's what
we've always done (or at least we've always given read access, but the
intention has been to give write access as well).

OK, then your patch is fine for me.

Post by Tanu Kaskinen
If we want to tighten the permissions, that can be done in a separate
patch.
We could make the mode configurable and default to 600 in the
user mode and 666 in the system mode. We could also make the group
configurable with "pulse-access" as the default group, then we could
default to 660 in the system mode.
We could also remove write access in case of module-pipe-sink and read
access in case of module-pipe-source.