alsa-lib cannot find its plugins

  • Done
  • quality assurance status badge
Details
3 participants
  • Danny Milosavljevic
  • Leo Famulari
  • Ludovic Courtès
Owner
unassigned
Submitted by
Leo Famulari
Severity
normal
L
L
Leo Famulari wrote on 24 Apr 2020 23:37
Audacity does not work with PulseAudio
(address . bug-guix@gnu.org)
20200424213727.GA11710@jasmine.lan
Since I can remember, Guix's Audacity doesn't work with PulseAudio on my
foreign distro (Debian).

Debian's Audacity does work correctly in this regard.

In practice, this means that I cannot play or record audio in Audacity
while any other application is using sound on the system. If I close or
stop those other applications, then Audacity is able to select the
'sysdefault' output sound device and can start working.

I found a few discussions about similar issues [0], and it seems that
Audacity needs alsa-plugins in order to make this work.

I tried installing Audacity along side alsa-plugins,
alsa-plugins:pulseaudio, and pulseaudio, as well as building with them
as dependencies, but it still didn't work.

L
L
Leo Famulari wrote on 25 Apr 2020 01:15
(address . 40832@debbugs.gnu.org)
20200424231524.GA16696@jasmine.lan
When Audacity starts, it prints this line:

------
ALSA lib conf.c:3683:(snd_config_hooks_call) Cannot open shared library libasound_module_conf_pulse.so (/gnu/store/nyylgcnzmbw8wrn4sna2crl0g7zxxh33-alsa-lib-1.2.2/lib/alsa-lib/libasound_module_conf_pulse.so: libasound_module_conf_pulse.so: cannot open shared object file: No such file or directory)
------

But, this file exists in the "pulseaudio" output of alsa-plugins, not
alsa-lib:

/gnu/store/pwsz9hf66na0s9x3ay9qk02vk8l4v8vi-alsa-plugins-1.2.2-pulseaudio/lib/alsa-lib/libasound_module_conf_pulse.so

On Debian, this library is found at:

/usr/lib/x86_64-linux-gnu/alsa-lib/libasound_module_conf_pulse.so
L
L
Leo Famulari wrote on 25 Apr 2020 06:03
(address . 40832@debbugs.gnu.org)
20200425040341.GA24170@jasmine.lan
On Fri, Apr 24, 2020 at 07:15:24PM -0400, Leo Famulari wrote:
Toggle quote (4 lines)
> ------
> ALSA lib conf.c:3683:(snd_config_hooks_call) Cannot open shared library libasound_module_conf_pulse.so (/gnu/store/nyylgcnzmbw8wrn4sna2crl0g7zxxh33-alsa-lib-1.2.2/lib/alsa-lib/libasound_module_conf_pulse.so: libasound_module_conf_pulse.so: cannot open shared object file: No such file or directory)
> ------

alsa-lib looks for this based on the compile-time constant
ALSA_PLUGIN_DIR, set during configure using --with-plugindir.

This is tricky because alsa-plugins depends on alsa-lib, and there are
also other packages that can provide plugins, like bluez-alsa.

Nixpkgs used to patch alsa-lib to look things up at runtime with an
environment variable, but stopped for some reason. That discussion even
points back to Guix periodically, but no solutions:

L
L
Leo Famulari wrote on 25 Apr 2020 19:48
(no subject)
(address . control@debbugs.gnu.org)
20200425174824.GA31652@jasmine.lan
retitle 40832 alsa-lib cannot find its plugins
L
L
Leo Famulari wrote on 26 Apr 2020 22:03
Re: alsa-lib cannot find its plugins
(address . 40832@debbugs.gnu.org)
20200426200334.GB555@jasmine.lan
I propose we make alsa-lib respect an environment variable
ALSA_PLUGIN_DIRS, and make that a Guix package search path that matches
'lib/alsa-lib'. I think this will do what we need. Any feedback?
L
L
Ludovic Courtès wrote on 28 Apr 2020 23:25
Re: bug#40832: Audacity does not work with PulseAudio
(name . Leo Famulari)(address . leo@famulari.name)(address . 40832@debbugs.gnu.org)
87imhjs3oz.fsf@gnu.org
Hi,

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (11 lines)
> When Audacity starts, it prints this line:
>
> ------
> ALSA lib conf.c:3683:(snd_config_hooks_call) Cannot open shared library libasound_module_conf_pulse.so (/gnu/store/nyylgcnzmbw8wrn4sna2crl0g7zxxh33-alsa-lib-1.2.2/lib/alsa-lib/libasound_module_conf_pulse.so: libasound_module_conf_pulse.so: cannot open shared object file: No such file or directory)
> ------
>
> But, this file exists in the "pulseaudio" output of alsa-plugins, not
> alsa-lib:
>
> /gnu/store/pwsz9hf66na0s9x3ay9qk02vk8l4v8vi-alsa-plugins-1.2.2-pulseaudio/lib/alsa-lib/libasound_module_conf_pulse.so

Could it be that the problem is in Audacity and not in alsa-lib?

I can do this with mpg123:

Toggle snippet (7 lines)
$ cat ~/.asoundrc
pcm.!default {
type pulse
}
$ mpg123 -o alsa …

and the sound goes through PulseAudio.

Ludo’.
L
L
Leo Famulari wrote on 29 Apr 2020 00:39
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 40832@debbugs.gnu.org)
20200428223908.GB31840@jasmine.lan
On Tue, Apr 28, 2020 at 11:25:32PM +0200, Ludovic Courtès wrote:
Toggle quote (2 lines)
> Could it be that the problem is in Audacity and not in alsa-lib?

I'm not 100% sure but I don't think so.

The function snd_config_hooks_call() is from alsa-lib I can't find any
way in alsa-lib for it work in this case, even though it aims to work by
default on systems with plugins in '/usr/lib/alsa-lib' or similar.

The lookup is performed in alsa-lib's 'src/dlmisc.c', by the function
snd_dlopen(), and it only looks in the hard-coded path provided by the
ALSA_PLUGIN_DIR C object macro, which ends up being alsa-lib's own store
directory.

Toggle quote (12 lines)
> I can do this with mpg123:
>
> --8<---------------cut here---------------start------------->8---
> $ cat ~/.asoundrc
> pcm.!default {
> type pulse
> }
> $ mpg123 -o alsa …
> --8<---------------cut here---------------end--------------->8---
>
> and the sound goes through PulseAudio.

Is that on Guix System or another distro? On Guix System, this is
handled by the service alsa-service-type.

On Debian, using mpg123 from Guix, and with your ~/.asoundrc, it fails
in the same way as Audacity:

------
% mpg123 -o alsa ...
High Performance MPEG 1.0/2.0/2.5 Audio Player for Layers 1, 2 and 3
version 1.25.13; written and copyright by Michael Hipp and others
free software (LGPL) without any warranty but with best wishes
ALSA lib conf.c:3683:(snd_config_hooks_call) Cannot open shared library libasound_module_conf_pulse.so (/gnu/store/nyylgcnzmbw8wrn4sna2crl0g7zxxh33-alsa-lib-1.2.2/lib/alsa-lib/libasound_module_conf_pulse.so: libasound_module_conf_pulse.so: cannot open shared object file: No such file or directory)
ALSA lib pcm.c:2642:(snd_pcm_open_noupdate) Unknown PCM default
[src/libout123/modules/alsa.c:181] error: cannot open device default
[src/libout123/libout123.c:455] error: Found no driver out of [alsa] working with device <default>.
main: [src/mpg123.c:314] error: out123 error 3: failure loading driver module
------
L
L
Leo Famulari wrote on 9 May 2020 00:45
(address . 40832@debbugs.gnu.org)
20200508224518.GA3682@jasmine.lan
An update on this:

To begin, I installed alsa-plugins and alsa-plugins:pulseaudio and
configured the build of alsa-lib like this:

"--with-plugindir=/var/guix/profiles/per-user/leo/guix-profile/lib/alsa-lib"

Everything worked that way, but obviously it's not a solution.

Now, I am working on making alsa-lib respect ALSA_PLUGIN_DIRS, which
would be a search path specified by packages that provide ALSA plugins,
such as alsa-plugins. However, so far my attempt fails in another part
of alsa-lib, like this:

------
$ mpg123 -o alsa ~/file.mp3
ALSA lib dlmisc.c:204:(snd_dlsym_verify) unable to verify version for symbol conf_pulse_hook_load_if_running
ALSA lib conf.c:3686:(snd_config_hooks_call) symbol conf_pulse_hook_load_if_running is not defined inside libasound_module_conf_pulse.so
ALSA lib pcm.c:2685:(snd_pcm_open_noupdate) Unknown PCM default
[src/libout123/modules/alsa.c:181] error: cannot open device default
[src/libout123/libout123.c:455] error: Found no driver out of [alsa] working with device <default>.
main: [src/mpg123.c:314] error: out123 error 3: failure loading driver module
------

I don't know why snd_dlsym_verify fails with my patch but succeeds when
using '--with-plugindir'.

My current patch is attached...
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAl614PoACgkQJkb6MLrK
fwgxNA//YqRuh+ircNhGnq7hP9qGwGMDngvFBgGmjPZFGc0iy+dZBaOA27Gi8ALc
WJHExw51N6aC6ZfQ1OCE5x3j4eJhh7LnqGoGho/x8jwNxBbz0eIHcUtk3IXXxXXJ
huCLkQ70xKBdvAR8l+qor0ksAwg2GvKXW6h6bNbYx2k5HNdGHVuB/FlY2j/shd+K
KVV5UnrlsnvJ8EzNypP3RIV5zVkNtK5mR0toUuD+DjZdvfpeqg3geeod3dCnNHXh
tYYMo5yUZj02riZn8HbEZMSa5IOZl3d1lHdDiZ9aw+N6mqJO1/i+9otMlYZKSIrR
is9zFhDYu3VHMFm/j1WgUj0W5nk+0DG1ebx1dH7orTyI0VhVa4ytPp03IbVmYpV9
0hxb09KvlzSPtQ1Dm8+8yvG3DGzP5E/f3Dfs6AXCwNAEa7kEbvAQGE6tLzMZveQw
Bcr2TeMzGYOcRQB0zEo8XYUBlbyBslwKw533hR5mfGfVL/Jru+motEwzZfsV6nVn
lM262fQ4Rr5Xr61+ISFIgFuO4itr5S3OQrdAn/MJyH+nAeS524noR+v4QfiPNmhX
R8zVlcnDFTwLDwOJWvfkE6H4YmWmRWB4S5jr0xnUznx6dTCtsewrkYnHSrTz4+KM
aZFnTujPtd6dm7I0dlitTbJ+Fw4Zrg3iqEMqAbAX4mvcn711W/c=
=b/jo
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 9 May 2020 07:24
(address . 40832@debbugs.gnu.org)
20200509052415.GA937@jasmine.lan
On Fri, May 08, 2020 at 06:45:18PM -0400, Leo Famulari wrote:
Toggle quote (2 lines)
> My current patch is attached...

I already found a lot of problems with patch 3/3... Don't look too
closely at it :)
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAl62PnwACgkQJkb6MLrK
fwic6BAArAFsRCI18olLm+8iFQhS9ARCGyYx0UKXH/5HYCyadM4UoaOn3VL58fZM
OJPaiZxLKiHHYlgegpuWcK5InPcIvlV6lGz9xneYVwJpRpGHpGrc3WEVcSsQKC/K
obTPkzQZx6+AZn3dgp8jA8PslehOvgCFkwrUgUxSM4GJ0wxWolJFu1fQEAN95Psg
9GTQJJC2B0yLcZcH7e9GrDN1q1tp+Om9BSIjwv7LMD+ncggqMNaBAajs1sWyrq9Y
K/9ur3UvrdVnEY+l3GEztSddqPCOjC27seHMEGd92txus16xsP/F/N3H9ZpfLOUZ
oXF9NcWcoqKRDG0DMYvvuEZFbvqx/uTj9a719od7t3GuhS/562NyGJIp/t1FyJHa
tN2xhqK2dSZQsbi6fXziaqW6CJKrUBvPszyVrqjLnuOcW/M8RhnfOCICH4PjkpsA
xmxu/1UK4g2e4e1iwcQnitK0kjO4s3yXcJ+hstSmkvXhuwnfPb27yQ0yL856mbdX
t5ss0Tf0vYC5Wfo9bygPE2AKMhsmV0COBLlnjUTHN1AY0VacGHKnmCcxX/73t9jb
BDG9kkcF+nXu8w39PMi508uLpUA70aAGMDlIkEvlx+z9B1d7UUCtqNGsEImtMTe7
Bcs/rqU+mGlvTaQ4bksddTxcphjWd234zDhqhIDfroSNSlGFQ00=
=zzME
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 16 May 2020 21:34
[PATCH 2/2] gnu: Help alsa-lib find its plugins on foreign distros.
(address . 40832@debbugs.gnu.org)
30199c186ff45e84c629765a2d23e04ca6737d70.1589657580.git.leo@famulari.name

* gnu/packages/linux.scm (alsa-lib)[replacement]: New field.
(alsa-lib/fixed): New variable.
* gnu/packages/patches/alsa-lib-plugin-dirs.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
gnu/local.mk | 1 +
gnu/packages/linux.scm | 10 ++
.../patches/alsa-lib-plugin-dirs.patch | 149 ++++++++++++++++++
3 files changed, 160 insertions(+)
create mode 100644 gnu/packages/patches/alsa-lib-plugin-dirs.patch

Toggle diff (197 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 39267f2765..78e63fe7ff 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -741,6 +741,7 @@ dist_patch_DATA = \
%D%/packages/patches/akonadi-not-relocatable.patch \
%D%/packages/patches/akonadi-timestamps.patch \
%D%/packages/patches/allegro-mesa-18.2.5-and-later.patch \
+ %D%/packages/patches/alsa-lib-plugin-dirs.patch \
%D%/packages/patches/amule-crypto-6.patch \
%D%/packages/patches/anki-mpv-args.patch \
%D%/packages/patches/antiword-CVE-2014-8123.patch \
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 4fb29b8490..8ea5bb909a 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -1771,6 +1771,7 @@ intercept and print the system calls executed by the program.")
(define-public alsa-lib
(package
(name "alsa-lib")
+ (replacement alsa-lib/fixed)
(version "1.2.2")
(source (origin
(method url-fetch)
@@ -1792,6 +1793,15 @@ intercept and print the system calls executed by the program.")
MIDI functionality to the Linux-based operating system.")
(license license:lgpl2.1+)))
+(define alsa-lib/fixed
+ (package
+ (inherit alsa-lib)
+ (source (origin
+ (inherit (package-source alsa-lib))
+ (patches (append
+ (origin-patches (package-source alsa-lib))
+ (search-patches "alsa-lib-plugin-dirs.patch")))))))
+
(define-public alsa-utils
(package
(name "alsa-utils")
diff --git a/gnu/packages/patches/alsa-lib-plugin-dirs.patch b/gnu/packages/patches/alsa-lib-plugin-dirs.patch
new file mode 100644
index 0000000000..0d6cb57f4e
--- /dev/null
+++ b/gnu/packages/patches/alsa-lib-plugin-dirs.patch
@@ -0,0 +1,149 @@
+From 5bc1a490fa68187bce15eb9e8305b88ff6fbdbe5 Mon Sep 17 00:00:00 2001
+From: Leo Famulari <leo@famulari.name>
+Date: Mon, 27 Apr 2020 13:09:54 -0400
+Subject: [PATCH] Search for plugins in $GUIX_ALSA_PLUGIN_DIRS.
+
+* src/control/control.c (snd_ctl_open_conf): If ALSA plugins cannot be found in
+the default locations, look in the directories in $GUIX_ALSA_PLUGIN_DIRS.
+* src/dlmisc.c (snd_dlopen): Likewise.
+* src/pcm/pcm.c (snd_pcm_open_conf): Likewise.
+---
+ src/control/control.c | 35 ++++++++++++++++++++++++++++++++++-
+ src/dlmisc.c | 26 +++++++++++++++++++++-----
+ src/pcm/pcm.c | 35 ++++++++++++++++++++++++++++++++++-
+ 3 files changed, 89 insertions(+), 7 deletions(-)
+
+diff --git a/src/control/control.c b/src/control/control.c
+index 27f42135..108a560b 100644
+--- a/src/control/control.c
++++ b/src/control/control.c
+@@ -1342,8 +1342,41 @@ static int snd_ctl_open_conf(snd_ctl_t **ctlp, const char *name,
+ err = -ENOMEM;
+ goto _err;
+ }
++ sprintf(buf1, "%s/libasound_module_pcm_%s.so", ALSA_PLUGIN_DIR, str);
++ if (access(buf1, F_OK) != 0) {
++ const char *plugindirs = getenv("GUIX_ALSA_PLUGIN_DIRS");
++
++ if (plugindirs) {
++ char *plugindirs_copy = alloca(strlen(plugindirs) + 1);
++ if (plugindirs_copy == NULL) {
++ err = -ENOMEM;
++ goto _err;
++ }
++ strcpy(plugindirs_copy, plugindirs);
++ char *saveptr;
++ while (1) {
++ char *dir_tok = strtok_r(plugindirs_copy, ":", &saveptr);
++ if (dir_tok == NULL)
++ break;
++ char *so_file = malloc(strlen(dir_tok) + 1 + strlen(str) + 32);
++ if (so_file == NULL) {
++ err = -ENOMEM;
++ goto _err;
++ }
++
++ sprintf(so_file, "%s/libasound_module_ctl_%s.so", dir_tok, str);
++
++ if (access(so_file, F_OK) == 0) {
++ buf1 = so_file;
++ break;
++ } else {
++ free (so_file);
++ }
++ plugindirs_copy = NULL;
++ }
++ }
++ }
+ lib = buf1;
+- sprintf(buf1, "%s/libasound_module_ctl_%s.so", ALSA_PLUGIN_DIR, str);
+ }
+ }
+ #ifndef PIC
+diff --git a/src/dlmisc.c b/src/dlmisc.c
+index 8c8f3ff7..b115447c 100644
+--- a/src/dlmisc.c
++++ b/src/dlmisc.c
+@@ -82,11 +82,27 @@ void *snd_dlopen(const char *name, int mode, char *errbuf, size_t errbuflen)
+ char *filename = NULL;
+
+ if (name && name[0] != '/') {
+- filename = alloca(sizeof(ALSA_PLUGIN_DIR) + 1 + strlen(name) + 1);
+- if (filename) {
+- strcpy(filename, ALSA_PLUGIN_DIR);
+- strcat(filename, "/");
+- strcat(filename, name);
++ const char *plugindirs = getenv("GUIX_ALSA_PLUGIN_DIRS");
++ if (plugindirs) {
++ char *plugindirs_copy = alloca(strlen(plugindirs) + 1);
++ if (plugindirs_copy == NULL)
++ goto errpath;
++ strcpy(plugindirs_copy, plugindirs);
++ char *saveptr;
++ while (1) {
++ char *dir_tok = strtok_r(plugindirs_copy, ":", &saveptr);
++ if (dir_tok == NULL)
++ break;
++ char *sofilename = malloc(strlen(dir_tok) + 1 + strlen(name) + 1);
++ sprintf(sofilename, "%s/%s" ,dir_tok, name);
++ if (access(sofilename, F_OK) == 0) {
++ filename = sofilename;
++ break;
++ } else {
++ free (sofilename);
++ }
++ plugindirs_copy = NULL;
++ }
+ handle = dlopen(filename, mode);
+ if (!handle) {
+ /* if the filename exists and cannot be opened */
+diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
+index 1064044c..90ba00ac 100644
+--- a/src/pcm/pcm.c
++++ b/src/pcm/pcm.c
+@@ -2578,8 +2578,41 @@ static int snd_pcm_open_conf(snd_pcm_t **pcmp, const char *name,
+ err = -ENOMEM;
+ goto _err;
+ }
+- lib = buf1;
+ sprintf(buf1, "%s/libasound_module_pcm_%s.so", ALSA_PLUGIN_DIR, str);
++ if (access(buf1, F_OK) != 0) {
++ const char *plugindirs = getenv("GUIX_ALSA_PLUGIN_DIRS");
++
++ if (plugindirs) {
++ char *plugindirs_copy = alloca(strlen(plugindirs) + 1);
++ if (plugindirs_copy == NULL) {
++ err = -ENOMEM;
++ goto _err;
++ }
++ strcpy(plugindirs_copy, plugindirs);
++ char *saveptr;
++ while (1) {
++ char *dir_tok = strtok_r(plugindirs_copy, ":", &saveptr);
++ if (dir_tok == NULL)
++ break;
++ char *so_file = malloc(strlen(dir_tok) + 1 + strlen(str) + 32);
++ if (so_file == NULL) {
++ err = -ENOMEM;
++ goto _err;
++ }
++
++ sprintf(so_file, "%s/libasound_module_pcm_%s.so", dir_tok, str);
++
++ if (access(so_file, F_OK) == 0) {
++ buf1 = so_file;
++ break;
++ } else {
++ free (so_file);
++ }
++ plugindirs_copy = NULL;
++ }
++ }
++ }
++ lib = buf1;
+ }
+ }
+ #ifndef PIC
+--
+2.26.2
+
--
2.26.2
L
L
Leo Famulari wrote on 16 May 2020 21:34
[PATCH 1/2] gnu: alsa-plugins: Add GUIX_ALSA_PLUGIN_DIRS search path specification.
(address . 40832@debbugs.gnu.org)
de108ca666c1b4f27945dfb3de06bcfc2f3f4b6c.1589657580.git.leo@famulari.name
* gnu/packages/linux.scm (alsa-plugins)[native-search-paths]: New field.
---
gnu/packages/linux.scm | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (17 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 40323a85d6..4fb29b8490 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -1858,6 +1858,10 @@ MIDI functionality to the Linux-based operating system.")
(base32
"0z9k3ssbfk2ky2w13avgyf202j1drsz9sv3834bp33cj1i2hc3qw"))))
(build-system gnu-build-system)
+ (native-search-paths
+ (list (search-path-specification
+ (variable "GUIX_ALSA_PLUGIN_DIRS")
+ (files '("lib/alsa-lib")))))
;; TODO: Split libavcodec and speex if possible. It looks like they can not
;; be split, there are references to both in files.
;; TODO: Remove OSS related plugins, they add support to run native
--
2.26.2
L
L
Leo Famulari wrote on 16 May 2020 21:33
[PATCH 0/2] Help alsa-lib find its plugins
(address . 40832@debbugs.gnu.org)
cover.1589657580.git.leo@famulari.name
These patches work well for me on Debian. I'm currently reconfiguring my
Guix System machine to test them here. Feedback welcome

Leo Famulari (2):
gnu: alsa-plugins: Add GUIX_ALSA_PLUGIN_DIRS search path
specification.
gnu: Help alsa-lib find its plugins on foreign distros.

gnu/local.mk | 1 +
gnu/packages/linux.scm | 14 ++
.../patches/alsa-lib-plugin-dirs.patch | 149 ++++++++++++++++++
3 files changed, 164 insertions(+)
create mode 100644 gnu/packages/patches/alsa-lib-plugin-dirs.patch

--
2.26.2
L
L
Leo Famulari wrote on 17 May 2020 20:19
(address . 40832@debbugs.gnu.org)
20200517181907.GA26362@jasmine.lan
On Sat, May 16, 2020 at 03:33:59PM -0400, Leo Famulari wrote:
Toggle quote (3 lines)
> These patches work well for me on Debian. I'm currently reconfiguring my
> Guix System machine to test them here. Feedback welcome

I tested this on Guix System and it does not interfere with the
default alsa-service, which sets up /etc/asound.conf

I'd like to push this to master soon-ish, with a followup ungraft on
staging.
D
D
Danny Milosavljevic wrote on 28 Jul 2020 12:52
alsa-lib cannot find its plugins
(address . 40832@debbugs.gnu.org)
20200728125241.4ff41418@scratchpost.org
Hi Leo,

some comments on the lastest patch:

* The entire alsa-lib seems to use the idiom "malloc and then strcpy", or
"malloc and then sprintf", or, worse, "malloc, strcpy and multiple strcat".
These are a buffer overflow waiting to happen (when changing part of those
while doing ongoing maintenance; also the places where they use "+" is not
checked for overflow). That said, if they do it, we can do it that way, too.
* The environment variable GUIX_ALSA_PLUGIN_DIRS is only checked if the
respective file does not exist in alsa-lib. That is not how environment
variables usually work--it should be possible to override built-in things
by setting this environment variable, too.
* Instead of alloca and strcpy, can just use strdupa.
* strtok_r man page states that the first argument should be NULL on the
non-first calls. You do that already, but maybe add a comment why that
is done where it's set to NULL.
* strtok_r man page states that "On some implementations, *saveptr is required
to be NULL on the first call to strtok_r() that is being used to parse str.".
So I'd use "char* saveptr = NULL;"
* Instead of malloc and sprintf, could just use asprintf. But they don't,
so let's not either, for easier review. Also, magical value 32... sigh.
Well, they do it, too.
* If GUIX_ALSA_PLUGIN_DIRS contained for example "a:" then it would search
"a" and "/", right? OK as long as we want that.

Otherwise LGTM!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl8gA3kACgkQ5xo1VCww
uqX71wgAnuA4W4hCgGgf4voKKgP75dr7156T3uabZ2cqpwboRuMIsF/LzsWLOe/x
vuow284NYaDdSvIbCcmji0bXy+FIfyqaSR/w161BUDCRsOIlwf+4yk2F3TsnxOxf
0mYZPDMaTjl4uTEyi/CcpGTIX667Zsy0gHtmALwbAByH3m3onf6F2VqSFvMXchlk
CkLmyNV6yaOsSCXeNdT3pqkJKBDcLYTqqTxe9PHLp0/gjAyv18hzBnyb4aKNJr0J
SrRRF3RZ05PwtH+51Yd/zIY9+iu0z8yx498bF5wNhrRiHsvs5mv6hTf46+EAXr6X
KYZCAva+/iFV0UAMfv4Z27XuVl2LkQ==
=PlR0
-----END PGP SIGNATURE-----


D
D
Danny Milosavljevic wrote on 28 Jul 2020 12:56
(address . 40832@debbugs.gnu.org)
20200728125619.44c2ad23@scratchpost.org
* src/control/control.c patch uses ALSA_PLUGIN_DIR and then, if necessary,
GUIX_ALSA_PLUGIN_DIRS. But src/dlmisc.c uses only GUIX_ALSA_PLUGIN_DIRS,
no ALSA_PLUGIN_DIR. src/pcm/pcm.c uses ALSA_PLUGIN_DIR and then, if necessary,
GUIX_ALSA_PLUGIN_DIRS. Is that discrepancy on purpose?
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl8gBFMACgkQ5xo1VCww
uqUKswf/Zr0bjy30jriVSw5u2kw5W5iyaB2EJyUZ8NEQM7XGYeZLi+dctn6Q+6Ah
vCkR+qr1w/v8Wnzdv67uUZ0bYB/cMis4IP7Mdk0BgOoaNOiEFY0wksg35NeYiyjo
YVs6KcRfeubQGcjFMeT+iNEtPQbpFOh+I3HwOo6cTnAsaf2MQp5Ss2dVqDh43DaS
XN9i7u1GXsQUgldDYLz/mySaXETf8NdSbWLYGQ/xZjicSRBEaFLcSnME8QpDww5p
YQdsSOn/jDxxpqQg64AFXzo/1NoGocobPKN4WbWi/s9ylvMLpK2UHhlW96uKb5Al
+fTlReetYM7+h6UXIE18IXZS0ceEUw==
=FIjY
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 29 Jul 2020 01:56
Re: bug#40832: alsa-lib cannot find its plugins
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 40832@debbugs.gnu.org)
20200728235623.GA1936@jasmine.lan
On Tue, Jul 28, 2020 at 12:52:41PM +0200, Danny Milosavljevic wrote:
Toggle quote (2 lines)
> some comments on the lastest patch:

Thank you for reviewing the patch!

Toggle quote (6 lines)
> * The entire alsa-lib seems to use the idiom "malloc and then strcpy", or
> "malloc and then sprintf", or, worse, "malloc, strcpy and multiple strcat".
> These are a buffer overflow waiting to happen (when changing part of those
> while doing ongoing maintenance; also the places where they use "+" is not
> checked for overflow). That said, if they do it, we can do it that way, too.

This confirms what I felt — it's hard to feel confident about the bounds
checking in this code. It seems to be based on the names of the plugin
libraries not exceeding some magic length. It's hard to balance "doing
the right thing" and using upstream's idioms.

When writing the patch, my investigation into the code made decide that
it would not overflow, but now I don't remember why I thought that.

Toggle quote (5 lines)
> * The environment variable GUIX_ALSA_PLUGIN_DIRS is only checked if the
> respective file does not exist in alsa-lib. That is not how environment
> variables usually work--it should be possible to override built-in things
> by setting this environment variable, too.

Good point. I don't remember now if I specifically decided to do things
this way or if it was a side effect of where I inserted the new code.

Toggle quote (2 lines)
> * Instead of alloca and strcpy, can just use strdupa.

I didn't know about this function, thanks.

Toggle quote (4 lines)
> * strtok_r man page states that the first argument should be NULL on the
> non-first calls. You do that already, but maybe add a comment why that
> is done where it's set to NULL.

Right.

Toggle quote (4 lines)
> * strtok_r man page states that "On some implementations, *saveptr is required
> to be NULL on the first call to strtok_r() that is being used to parse str.".
> So I'd use "char* saveptr = NULL;"

My Linux 4.16 man pages from Debian don't contain this note. Good to
know!

Toggle quote (4 lines)
> * Instead of malloc and sprintf, could just use asprintf. But they don't,
> so let's not either, for easier review. Also, magical value 32... sigh.
> Well, they do it, too.

Right...

Toggle quote (3 lines)
> * If GUIX_ALSA_PLUGIN_DIRS contained for example "a:" then it would search
> "a" and "/", right? OK as long as we want that.

I don't remember how it behaves anymore... I'll look into this and
decide.

Thanks again for the review!
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAl8guyQACgkQJkb6MLrK
fwgo5g//QbfZWOEhJGAIerkFEk8gFSlRdcViVdqcDSd7GPhm/Q88B1d3YlRytgJ0
iyxQ8lll5uZ9uM1DZOrkwrmJBSi3WoADV+4Rkbao+9Ak3acrzn2tWBueng4o5EG8
I+lzlwu09miMoQEEXuAjpDE1wa1vHDCpUM1Z43ng1+pzsIAiFNFxx2MZnX57CSDK
pjDyLMvE2coiEaw2geuqjPZnGl16ZQgmIhZEIqDFxlS+eVDQcHDaACnB/FFw/whs
cbN9pBm+GuS1JAKAhZcYjCt63Lv2Lkvwv0RpPkaY5h7gCepkB5J4zEPSzA2HQlsa
m3VmZ1PMPWYB9TC5Lu395yPJFd7RVU5ouU11/5+jWGwRh5N9MrheyQRu4/N3GnxW
sCfcvv+DlE7stlP3BTj2idHlqn2f7Qz590yz2aPNZp3/kdgOPZP9mUWF8N1LkqPj
RG+rlHlFaCf4OoFsd6ptW/8ZyW3a34+XUHsXmkmSMO/t0v5zHbquVOmaoNrz6qFr
JPOCi3MdQXg5kF6OEb18JTJrGcinSj1r/MRt4XDJeUpl4zgXIhFC3t/co8LZ8lPn
mYfCdNk4JN47j8JKvKGnGTaSiTceNPoIjoF7QeKyXO8AdRADwSZ/nDaoJSInDxie
apnXHOnnC+glFpsxBi6TW/lXvSoB5FPtjM8eQANKY/TlrdOFg7o=
=Wkny
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 29 Jul 2020 01:56
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 40832@debbugs.gnu.org)
20200728235649.GB1936@jasmine.lan
On Tue, Jul 28, 2020 at 12:56:19PM +0200, Danny Milosavljevic wrote:
Toggle quote (5 lines)
> * src/control/control.c patch uses ALSA_PLUGIN_DIR and then, if necessary,
> GUIX_ALSA_PLUGIN_DIRS. But src/dlmisc.c uses only GUIX_ALSA_PLUGIN_DIRS,
> no ALSA_PLUGIN_DIR. src/pcm/pcm.c uses ALSA_PLUGIN_DIR and then, if necessary,
> GUIX_ALSA_PLUGIN_DIRS. Is that discrepancy on purpose?

I will look into it, thanks for noticing!
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAl8gu0EACgkQJkb6MLrK
fwgE1RAA2OS1LAHD7gkgKUqZcFskRcX4KM3tJvI6zWkYJXyUzbO9OQw+jG/2iJf3
wawyIhdKbKt0lps1gwH9gIKrVBrWuljQOunLEmAUWjSx33IqaOcgx4xkzU9/QEGR
RRWDUmMeKgj51j/PxDFPv0QzoYQA4TkKLtpBkPAo5Dg0yx/ZgVPQ/UKvpkWMfmdJ
gdTODlLxxex2C7zpLoWYDCmqpvQJ3Qi/kxahCffEfX+xP+dccr5140JlhO11cLw+
OJPK2RyZdkhvJ7gOR+V2oenIO8e1yK1+8g/pc7KSXGXuWEA+KT8Iidpf35YbvfLg
jAxVNyvXYpOfyl27AQV9yU9Ww4bYL400BkEjjfic4LNU3LgRudrhma/0xVc2ZRM1
uuiiQFDOgG78FolwIBdeFfY7h8mvPVKAN1QoRjlcShz+GL2BanRUKHDSKwsqydR6
+XTw2Y0pFU0f5SqpJAf+DOcqPEZXkZPhaflX1k0x+jXg8oHoYyyQyQO89I3SI6Ze
KdiKv5JQXEfxItlOp0v032nRKgbOfw4kF+U6mFP/rRLyiaco9dLI/8/u8gwBXAtW
+drB8iTpnDX/wDwkmwPBNhpVG8+N443P83Dbz+1G33/aB6snijoBW3G0zTr8SToQ
DeuZn8nMUvq1BkTCMDep6mfPU6X6uTcLrwqTJbc8mHKO8Vbnuc8=
=1R2O
-----END PGP SIGNATURE-----


D
D
Danny Milosavljevic wrote on 29 Jul 2020 13:18
(name . Leo Famulari)(address . leo@famulari.name)(address . 40832@debbugs.gnu.org)
20200729131844.18b08be2@scratchpost.org
Hi Leo,

On Tue, 28 Jul 2020 19:56:23 -0400
Leo Famulari <leo@famulari.name> wrote:

Toggle quote (16 lines)
> On Tue, Jul 28, 2020 at 12:52:41PM +0200, Danny Milosavljevic wrote:
> > some comments on the lastest patch:
>
> Thank you for reviewing the patch!
>
> > * The entire alsa-lib seems to use the idiom "malloc and then strcpy", or
> > "malloc and then sprintf", or, worse, "malloc, strcpy and multiple strcat".
> > These are a buffer overflow waiting to happen (when changing part of those
> > while doing ongoing maintenance; also the places where they use "+" is not
> > checked for overflow). That said, if they do it, we can do it that way, too.
>
> This confirms what I felt — it's hard to feel confident about the bounds
> checking in this code. It seems to be based on the names of the plugin
> libraries not exceeding some magic length. It's hard to balance "doing
> the right thing" and using upstream's idioms.

After thinking about it more, I think it's much worse if the thing that is
stuck into the malloced block is the value of an environment variable.

When it's a compile-time variable you basically trust the code and the
distribution package not to have too-long paths in there that could overflow
the "+" in malloc(... + ). A distribution or upstream could do much worse
things than that, so that is not a credible threat to worry about.

For a runtime variable like the environment variable (that anyone can set to
anything), I am very much in favor of not using malloc(... + ) and instead
using asprintf, in order to prevent an exploitable buffer overflow just by
setting up an environment variable.

Toggle quote (3 lines)
> When writing the patch, my investigation into the code made decide that
> it would not overflow, but now I don't remember why I thought that.

Thanks for that remark. It made me think and I came to the recommendation
above.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl8hWxQACgkQ5xo1VCww
uqU6ggf9GepLAqJmnyYpilvAyGOkD4AC1Fj+V7d8EuNqjAOsFmO+3pHaECHAJBne
DKo7h6NL4ynsR+OfPoc/O6zIsFoOjiUVqHX8HpUZCOCvrl7lPyPyoFJOOGChNcXh
x1lkj8bOnaW2uLPpZ2wL5ff3R/k2geqDWha6rEREs/rKgxLswgNOWownozpfYH7M
PVJAqIgggp2FeZ7+Glm2YfFdDrJdz9vgFthk3bhyMCbI41FVPiEu3Iwmq2SQlkGP
pR+ae8tbFkk8Acn8I71F+Pyj/yU+S+nyaHVHR9AK2y3uvsZYKyy6cAw8g8cIFeBF
dt1W6Zh3MRxGj9onS1VPP/iBy73Iag==
=cqCo
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 13 Oct 2020 18:02
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 40832@debbugs.gnu.org)
20201013160259.GA1702@jasmine.lan
Upstream has implemented (but not yet released) a potential solution:


I haven't tested it yet but my understanding is that it supports
specifying a single plugin directory via the ALSA_PLUGIN_DIR environment
variable.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAl+Fz7MACgkQJkb6MLrK
fwhVMg/+KBJJOBW9HY/3P4PDlSn4X236dSX/m+Iv/kkj8cOssvAFU7SNELHGp6eg
uu8N5cNQOn1/WrwWetsoRJi/bo5kYVs1Xks37uue1JRJzqv9yVsp+W1CGLMjdK7D
IWN5KOCV2B1fQbxUvntnMcbAXdrhHXfAPDpvGqLUoR/Gv6MPI6M+9EZOKPDYbQOU
jsv49DcVQzMqqNlXDjAKY8ReGSMViQmBSk6vkl7GqKZka82FS/4ZOlOCUF6RM7VX
Jl964H1GCAMnQUxy5xURbiJf5MXVWIxXUwEjK7or33k8alv2FKUF0mtLVjsESkF+
kdu6tcQWbjz4mHcvPzgVJ3qI+teiruWFipUyZ/9mUJvOwKzbU7CVYD/rpe7AjOBH
oTg3CH/Fw2L0vK3asNW/0oVtuCf9yMJGpPfYDQVLBOjIKD2WDOvUTwNtfVpSA7ug
PTx0hIbk4bOloPansYcq6hYoRytXEhGhPHcAScuomNDeyNpUmAYNZSoxZXaXpR8R
4lnphMg2P9EvFbGaR+v8Z+pGtdurWBtgNZV2vic6QMKooc6q1Y8NvXDQsxTj3urE
UHempsYDSJlrA2bJ4h3R0zOy4lYw1Gsy/66GdTFVFeSmtUx1+ouqpD2TdND97wvM
dWdwyw8xijDE850kNE0mjQlXA+mOJ1ZHXpmbncWlRvI6lcb0jfY=
=w2eK
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 1 Feb 2021 21:51
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 40832-done@debbugs.gnu.org)
YBhpu+4AS96FlK9x@jasmine.lan
On Tue, Oct 13, 2020 at 12:02:59PM -0400, Leo Famulari wrote:
Toggle quote (8 lines)
> Upstream has implemented (but not yet released) a potential solution:
>
> https://github.com/alsa-project/alsa-lib/commit/8580c081c25678d11278efcb61bd15cf44d0a225
>
> I haven't tested it yet but my understanding is that it supports
> specifying a single plugin directory via the ALSA_PLUGIN_DIR environment
> variable.

I'm very happy to report that this fix has been deployed for Guix users
in commit ed1c72c3d7d6b6d3b110817fcc037cd5582ac848
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmAYabUACgkQJkb6MLrK
fwio2hAA5d/6ZDlge4l+o6PB7y562n8vT39BO5ngLWmUR7EA5+H9d0aJXumz4yVu
7auQ+XYUdphmPEgK23XWg5JYKA4tpW73sYizKrkG278TYV3S3fjcEXTC3MMExCVb
zfGg21b+cfke1+wTlO+0auGV3gXkme+77azx4cIbsc2XaPPvg8CJuPSQzoyQEVZS
50TWMBodWaDQ0nijyWWUD7f9FeiYW85sDmV0FCCTIeLGrZxhrB+57IVOlIxckF1z
MIuGApbIIaVB96eirX+f/kZlOv5HcZ1qNWYWw4/UK0yvPttofbotPxWvQ9yFTPHW
9nlUw6+ECxf4x4b27l1i0gFFuFCb3A0PUIbXQgHqb87FHeQu3jWlJ+nDQpsJch+H
YdEsdM7J0TUbC5EK5gCpsHMLwGgMzpGPhnvmJeIj2qFP5d6YDAiP1Yt+jEB1+8Hz
alkUDHazlGtKGLO9QDp6rWcSd13yyFszbpGHVjAZWlPpwHYh/aF4oOEATS+fwCbs
awydUQgNRtjTFB/kLzvQdrOYqjuoxHfjJqTK7RfdBourrE/1vXcs5iSQ/ntUVlwr
DacoXIU7UfQo2Dz9C4URhKKX4WFPgisra3Kj+o7REAe5m4qdXY3PNTZUYazJcIun
c8IeLR2hxOxTj3Crl04UumQPFYNRvXXBHIb+bpmNsNu5FRU0FnM=
=XvpU
-----END PGP SIGNATURE-----


Closed
?