bug#40832: Audacity does not work with PulseAudio

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

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.

[0] e.g. https://forum.audacityteam.org/viewtopic.php?t=89278

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

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

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

On Fri, Apr 24, 2020 at 07:15:24PM -0400, Leo Famulari wrote:
> ------
> 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:

https://github.com/NixOS/nixpkgs/issues/6860

bug#40832: alsa-lib cannot find its plugins

[Leo Famulari]

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?

bug#40832: Audacity does not work with PulseAudio

[Ludovic Courtès]

Hi,

Leo Famulari <leo@famulari.name> skribis:

> 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:

--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.

Ludo’.

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

On Tue, Apr 28, 2020 at 11:25:32PM +0200, Ludovic Courtès wrote:
> 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.

> 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
------

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]

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...

[-- Attachment #1.2: alsa-lib-plugin-dirs.patch --]
[-- Type: text/plain, Size: 10363 bytes --]

From 2dc0cf223a71d2a22ca19eff6c59d55d72028c64 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 26 Apr 2020 20:13:01 -0400
Subject: [PATCH 1/3] gnu: alsa-plugins: Add ALSA_PLUGIN_DIRS search path
 specification.

* gnu/packages/linux.scm (alsa-plugins)[native-search-paths]: New field.
---
 gnu/packages/linux.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 40323a85d6..b451f591ea 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 "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


From a9ce47575add9eeb015eb6c605bde01948c5e341 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Mon, 27 Apr 2020 01:05:41 -0400
Subject: [PATCH 2/3] gnu: Add the old Nix alsa-lib patch with a graft.

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

diff --git a/gnu/local.mk b/gnu/local.mk
index 39267f2765..de17670beb 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.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 b451f591ea..e0ed8c040d 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.patch")))))))
+
 (define-public alsa-utils
   (package
     (name "alsa-utils")
diff --git a/gnu/packages/patches/alsa-lib.patch b/gnu/packages/patches/alsa-lib.patch
new file mode 100644
index 0000000000..3cee02aa0c
--- /dev/null
+++ b/gnu/packages/patches/alsa-lib.patch
@@ -0,0 +1,110 @@
+diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
+index 74d1d1a..17ffb12 100644
+--- a/src/pcm/pcm.c
++++ b/src/pcm/pcm.c
+@@ -2042,6 +2042,19 @@ static const char *const build_in_pcms[] = {
+ 	NULL
+ };
+ 
++
++// helper funcion used below
++int file_exists(const char * filename)
++{
++	FILE * file;
++	if (file = fopen(filename, "r"))
++	{
++		fclose(file);
++		return 1;
++	}
++	return 0;
++}
++
+ static int snd_pcm_open_conf(snd_pcm_t **pcmp, const char *name,
+ 			     snd_config_t *pcm_root, snd_config_t *pcm_conf,
+ 			     snd_pcm_stream_t stream, int mode)
+@@ -2141,8 +2154,38 @@ 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 (!file_exists(buf1)){
++				// try to locate plugin in one of ALSA_PLUGIN_DIRS which is colon separated list of paths
++				char * pdirs = getenv("ALSA_PLUGIN_DIRS");
++
++				if (pdirs){ // env var set?
++					char * saveptr;
++					while (1) {
++						char * dir_tok = strtok_r(pdirs, "::::", &saveptr); // "::::" to work around bug in glibc and -O2 ? ":" seems to cause a segfault
++						if (dir_tok == NULL)
++                            break;
++						char * so_file = malloc(strlen(str) + strlen(dir_tok) + 32);
++						if (so_file == NULL) {
++							err = -ENOMEM;
++							goto _err;
++						}
++
++						sprintf(so_file, "%s/libasound_module_pcm_%s.so", dir_tok, str);
++
++						if (file_exists(so_file)){
++
++							free(buf1);
++							buf1 = so_file;
++							break;
++						} else {
++							free (so_file);
++						}
++						pdirs = NULL;
++					}
++				}
++			}
++			lib = buf1;
+ 		}
+ 	}
+ #ifndef PIC
+
+
+diff --git a/src/control/control.c b/src/control/control.c
+index c090797..137fe57 100644
+--- a/src/control/control.c
++++ b/src/control/control.c
+@@ -854,8 +854,38 @@ 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 (!file_exists(buf1)){
++				// try to locate plugin in one of ALSA_PLUGIN_DIRS which is colon separated list of paths
++				char * pdirs = getenv("ALSA_PLUGIN_DIRS");
++
++				if (pdirs){ // env var set?
++					char * saveptr;
++					while (1) {
++						char * dir_tok = strtok_r(pdirs, "::::", &saveptr); // "::::" to work around bug in glibc and -O2 ? ":" seems to cause a segfault
++						if (dir_tok == NULL)
++                            break;
++						char * so_file = malloc(strlen(str) + strlen(dir_tok) + 32);
++						if (so_file == NULL) {
++							err = -ENOMEM;
++							goto _err;
++						}
++
++						sprintf(so_file, "%s/libasound_module_ctl_%s.so", dir_tok, str);
++
++						if (file_exists(so_file)){
++
++							free(buf1);
++							buf1 = so_file;
++							break;
++						} else {
++							free (so_file);
++						}
++						pdirs = NULL;
++					}
++				}
++			}
+ 			lib = buf1;
+-			sprintf(buf1, "%s/libasound_module_ctl_%s.so", ALSA_PLUGIN_DIR, str);
+ 		}
+ 	}
+ #ifndef PIC
-- 
2.26.2


From 71c5ccb8f24165bea6154d566056b2474adeebc7 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Fri, 8 May 2020 18:27:00 -0400
Subject: [PATCH 3/3] WIP: Make alsa-lib look for ALSA plugins at run-time.

Currently it doesn't work :(

Test on foreign distro with `mpg123 -o alsa ~/file.mp3`.

* gnu/packages/patches/alsa-lib.patch: Make alsa-lib look up plugins in
the directories named in $ALSA_PLUGIN_DIRS.
---
 gnu/packages/patches/alsa-lib.patch | 76 +++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/gnu/packages/patches/alsa-lib.patch b/gnu/packages/patches/alsa-lib.patch
index 3cee02aa0c..7ca6b7960a 100644
--- a/gnu/packages/patches/alsa-lib.patch
+++ b/gnu/packages/patches/alsa-lib.patch
@@ -108,3 +108,79 @@ index c090797..137fe57 100644
  		}
  	}
  #ifndef PIC
+diff --git a/src/dlmisc.c b/src/dlmisc.c
+index 8c8f3ff7..274d4b84 100644
+--- a/src/dlmisc.c
++++ b/src/dlmisc.c
+@@ -82,19 +82,52 @@ 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);
+-			handle = dlopen(filename, mode);
+-			if (!handle) {
+-				/* if the filename exists and cannot be opened */
+-				/* return immediately */
+-				if (access(filename, X_OK) == 0)
+-					goto errpath;
++                // 'name' is the full library.so name, e.g. libasound_module_conf_pulse.so
++		fprintf(stderr, "XXX name is %s\n", name);
++		char * plugindirs = getenv("ALSA_PLUGIN_DIRS");
++		fprintf(stderr, "XXX plugindirs are %s\n" ,plugindirs);
++		if (plugindirs) {
++			char * saveptr;
++			while (1) {
++				// See comment in src/control/control.c about "::::"
++				char * dir_tok = strtok_r(plugindirs, "::::", &saveptr);
++				if (dir_tok == NULL)
++					break;
++				fprintf(stderr, "XXX dir token is %s\n" ,dir_tok);
++				char * so_file = malloc(strlen(name) + strlen(dir_tok) + 32);
++				sprintf(so_file, "%s/%s" ,dir_tok, name);
++				// TODO Check if so_file == NULL here
++				if (file_exists(so_file)) {
++					fprintf(stderr, "XXX Found the library %s\n" ,so_file);
++					handle = dlopen(filename, mode);
++					if (!handle) {
++						/* if the filename exists and cannot be opened */
++						/* return immediately */
++						if (access(filename, X_OK) == 0)
++							goto errpath;
++					}
++					break;
++				} else {
++					fprintf(stderr, "XXX Did not find the library %s\n" ,so_file);
++					break;
++				}
+ 			}
+ 		}
++// Cut here:
++//		filename = alloca(sizeof(ALSA_PLUGIN_DIR) + 1 + strlen(name) + 1);
++//		if (filename) {
++//			strcpy(filename, ALSA_PLUGIN_DIR);
++//			strcat(filename, "/");
++//			strcat(filename, name);
++//			handle = dlopen(filename, mode);
++//			if (!handle) {
++//				/* if the filename exists and cannot be opened */
++//				/* return immediately */
++//				if (access(filename, X_OK) == 0)
++//					goto errpath;
++//			}
++//		}
++// ... to here.
+ 	}
+ 	if (!handle) {
+ 		handle = dlopen(name, mode);
+@@ -104,6 +137,7 @@ void *snd_dlopen(const char *name, int mode, char *errbuf, size_t errbuflen)
+ 	return handle;
+ errpath:
+ 	if (errbuf)
++		fprintf(stderr, "XXX couldn't find your thing!\n");
+ 		snprintf(errbuf, errbuflen, "%s: %s", filename, dlerror());
+ #endif
+ 	return NULL;
-- 
2.26.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#40832: Audacity does not work with PulseAudio

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 181 bytes --]

On Fri, May 08, 2020 at 06:45:18PM -0400, Leo Famulari wrote:
> My current patch is attached...

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#40832: [PATCH 0/2] Help alsa-lib find its plugins

[Leo Famulari]

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

bug#40832: [PATCH 1/2] gnu: alsa-plugins: Add GUIX_ALSA_PLUGIN_DIRS search path specification.

[Leo Famulari]

* gnu/packages/linux.scm (alsa-plugins)[native-search-paths]: New field.
---
 gnu/packages/linux.scm | 4 ++++
 1 file changed, 4 insertions(+)

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

bug#40832: [PATCH 2/2] gnu: Help alsa-lib find its plugins on foreign distros.

[Leo Famulari]

Fixes <https://bugs.gnu.org/40832>.

* 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

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

bug#40832: [PATCH 0/2] Help alsa-lib find its plugins

[Leo Famulari]

On Sat, May 16, 2020 at 03:33:59PM -0400, Leo Famulari wrote:
> 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.

bug#40832: alsa-lib cannot find its plugins

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

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!

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

* 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?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

On Tue, Jul 28, 2020 at 12:56:19PM +0200, Danny Milosavljevic wrote:
> * 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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

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.

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.

> * 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.

> * Instead of alloca and strcpy, can just use strdupa.

I didn't know about this function, thanks.

> * 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.

> * 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!

> * 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...

> * 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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]

Hi Leo,

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

> 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.

> 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.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#40832: alsa-lib cannot find its plugins

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

On Tue, Oct 13, 2020 at 12:02:59PM -0400, Leo Famulari wrote:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]