From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id 8CtxGFi7IF+3DgAA0tVLHw (envelope-from ) for ; Tue, 28 Jul 2020 23:57:12 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id mBUgFFi7IF+ZWQAAbx9fmQ (envelope-from ) for ; Tue, 28 Jul 2020 23:57:12 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 768E69403AA for ; Tue, 28 Jul 2020 23:57:11 +0000 (UTC) Received: from localhost ([::1]:36806 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1k0ZT3-0006ku-Vo for larch@yhetil.org; Tue, 28 Jul 2020 19:57:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56878) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1k0ZSw-0006kc-1B for bug-guix@gnu.org; Tue, 28 Jul 2020 19:57:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:47919) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1k0ZSv-0002hw-O2 for bug-guix@gnu.org; Tue, 28 Jul 2020 19:57:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1k0ZSv-0007Yw-Mm for bug-guix@gnu.org; Tue, 28 Jul 2020 19:57:01 -0400 X-Loop: help-debbugs@gnu.org Subject: bug#40832: alsa-lib cannot find its plugins Resent-From: Leo Famulari Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Tue, 28 Jul 2020 23:57:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 40832 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Danny Milosavljevic Received: via spool by 40832-submit@debbugs.gnu.org id=B40832.159598060029031 (code B ref 40832); Tue, 28 Jul 2020 23:57:01 +0000 Received: (at 40832) by debbugs.gnu.org; 28 Jul 2020 23:56:40 +0000 Received: from localhost ([127.0.0.1]:59461 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1k0ZSa-0007YB-7u for submit@debbugs.gnu.org; Tue, 28 Jul 2020 19:56:40 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:38067) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1k0ZSV-0007Xu-LU for 40832@debbugs.gnu.org; Tue, 28 Jul 2020 19:56:39 -0400 Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 46FDD5C014E; Tue, 28 Jul 2020 19:56:30 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 28 Jul 2020 19:56:30 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=famulari.name; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=mesmtp; bh=3JDJVli4b/v0Fe+sFl5tT2cS 080zXf+0mlV7sMOhKM0=; b=ea+zXHWFAvYxtyWgKMymXnwzUSelg5uNUCrae7Dr BreZ5YKNErTulHvPJGYRoJlU7SXo/d9RjznpeEK335KINvXCDEjUAi1CwMGFA7pK md0WPdKeh+9JY3zpdRZkyciKYxNPqot/QXwjmln3F7FuIfuZEDmlNyFPU4GZO7Eg bVM= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=3JDJVl i4b/v0Fe+sFl5tT2cS080zXf+0mlV7sMOhKM0=; b=MzGG9/d1RcuwyUHxZdxTzg QduYb5sSJ+7+ulslBYPAcXCeODhWKA2uENQd5RtTzv7dORUyV7MD24QaUdunQR7/ CgG3hNlemwlzAxu/8XkQ2rnk9X/b9q8smoljNh6sbukqsO0CYZQpQ/iMXHYi4kmM M+hopKidkrnS0rmCzuDmB89EmPalmtwrHL568sEyXTMJ8GwPklcbZGNMkIgfOGiy AYSI75wdVAmG+UZ2Cj/seTkLQQnuisZITGnBnPMtssuO5dP2fQ/g6YoE5pfjls+R N9cK0+iFtHvc0U9pqpCEDd+rlE//nXYvfgjO74in/xlUkxbAuC0CHiZ81YdVBNdw == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrieefgddvjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtjeenucfhrhhomhepnfgvohcuhfgr mhhulhgrrhhiuceolhgvohesfhgrmhhulhgrrhhirdhnrghmvgeqnecuggftrfgrthhtvg hrnhepiefghfeffedtffekveektddtieekfeffledtgfevkeekteeufedtfefhgefhkeef necukfhppeejfedrudeguddruddvjedrudegieenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehlvghosehfrghmuhhlrghrihdrnhgrmhgv X-ME-Proxy: Received: from localhost (c-73-141-127-146.hsd1.pa.comcast.net [73.141.127.146]) by mail.messagingengine.com (Postfix) with ESMTPA id 4D4D4306005F; Tue, 28 Jul 2020 19:56:29 -0400 (EDT) Date: Tue, 28 Jul 2020 19:56:23 -0400 From: Leo Famulari Message-ID: <20200728235623.GA1936@jasmine.lan> References: <20200424213727.GA11710@jasmine.lan> <20200728125241.4ff41418@scratchpost.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3V7upXqbjpZ4EhLz" Content-Disposition: inline In-Reply-To: <20200728125241.4ff41418@scratchpost.org> X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -1.7 (-) X-BeenThere: bug-guix@gnu.org List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: 40832@debbugs.gnu.org Errors-To: bug-guix-bounces+larch=yhetil.org@gnu.org Sender: "bug-Guix" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=fail (rsa verify failed) header.d=famulari.name header.s=mesmtp header.b=ea+zXHWF; dkim=fail (rsa verify failed) header.d=messagingengine.com header.s=fm3 header.b=MzGG9/d1; dmarc=none; spf=pass (aspmx1.migadu.com: domain of bug-guix-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=bug-guix-bounces@gnu.org X-Spam-Score: -2.11 X-TUID: F9luY6L1SvHs --3V7upXqbjpZ4EhLz Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 n= ot > checked for overflow). That said, if they do it, we can do it that way, = too. This confirms what I felt =E2=80=94 it's hard to feel confident about the b= ounds 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 req= uired > to be NULL on the first call to strtok_r() that is being used to parse s= tr.". > So I'd use "char* saveptr =3D 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! --3V7upXqbjpZ4EhLz Content-Type: application/pgp-signature; name="signature.asc" -----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----- --3V7upXqbjpZ4EhLz--