From patchwork Fri Mar 24 17:18:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Grund X-Patchwork-Id: 13187054 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1B73C76196 for ; Fri, 24 Mar 2023 17:18:45 +0000 (UTC) Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by mx.groups.io with SMTP id smtpd.web10.2681.1679678317352753385 for ; Fri, 24 Mar 2023 10:18:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=j8QFHIGn; spf=pass (domain: gmail.com, ip: 209.85.221.54, mailfrom: theflamefire89@gmail.com) Received: by mail-wr1-f54.google.com with SMTP id h17so2531071wrt.8 for ; Fri, 24 Mar 2023 10:18:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679678316; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=o2l1eB1PcN679Kcv7LS9Zuc1IPxKvG4ruJNIe6Hy0OA=; b=j8QFHIGnF/epo1aoqsHoUy9rebBkB/T0HPFSYlFLqWCTaDskF5aIRI+90f7ywx0YPg sIW7DHGZjaoJbIqFck1Y3L1cz5MOkrO6FWaFcLnRo7TJdmnj0GGP7Rb5ak0Y2ZqpRN/1 y19UArxqWs2pytCPpMGxT7f9x1aaBdSWHRNltOUi310opmapR19jBWGN8PYatTG7DmUu u/SqfPbnSWfqy0PEkBczxFGqZZwLwRqErg9ka/e3+oUotgZ/XvyyKVIwWIYA8YwdWesm zIOKk7OngjZ793hxfMhh/pJ7kimP0mMztMI9/Yfx4KEoBiZHFDlujCBi9Ow+pyEsdXKp qGIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679678316; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=o2l1eB1PcN679Kcv7LS9Zuc1IPxKvG4ruJNIe6Hy0OA=; b=R+6bvaxEch9kcMdpwYDhdDevj3EgORh1+b5SZ2BcR2b3BtmDIqburGdI7IlSN2A9Tr fb9bcsX6RPDmkgfm1iis5DfqmUgA65MQIxpjtYocjGBhqcpjDr9euIQZbLcX9s8Y7u7u JvMWV7i5GjTHi2tdqBrEUpIkq6qweWU4OCmAsxPSEU8HeAjxpSwYCXbddkYG8cgAvocV sjWH+3phMdIUhlH2JEP8WAhnueqPswe0aLndzmMACeDT4ugGrmBKCUF3xvZWiOeT3KUk dhy9LMKDDreivfYo9jr9u/jWOoEOPscx8P/yQL7rw6Jd7SPfYoWI0xpCXA9VbIpzS1Ml ng3g== X-Gm-Message-State: AAQBX9cXwD58s2yX40QQaThnEG23LdmHpxQgaoImrUNp9srymkJCf6T6 gW4dgARGwD36uMRbDldKkyHXMy1DNUE= X-Google-Smtp-Source: AKy350ZY9PDhViuEEnBnAiQvc2jp8j/A7mZQbCj4dd8desoDw8HU5Dt8jlKY08kLILPPBkaSGBSY3Q== X-Received: by 2002:a5d:674e:0:b0:2c7:dad:5630 with SMTP id l14-20020a5d674e000000b002c70dad5630mr2844494wrw.27.1679678315763; Fri, 24 Mar 2023 10:18:35 -0700 (PDT) Received: from alex-Mint.fritz.box (p200300f6af150c00d964c2cc0a52faf5.dip0.t-ipconnect.de. [2003:f6:af15:c00:d964:c2cc:a52:faf5]) by smtp.googlemail.com with ESMTPSA id k22-20020a05600c1c9600b003eda46d6792sm340662wms.32.2023.03.24.10.18.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Mar 2023 10:18:35 -0700 (PDT) From: Alexander Grund To: cip-dev@lists.cip-project.org Cc: uli+cip@fpond.eu Subject: [PATCH 4.4 1/3] ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations Date: Fri, 24 Mar 2023 18:18:10 +0100 Message-Id: <20230324171812.221086-2-theflamefire89@gmail.com> X-Mailer: git-send-email 2.40.0 In-Reply-To: <20230324171812.221086-1-theflamefire89@gmail.com> References: <20230324171812.221086-1-theflamefire89@gmail.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Fri, 24 Mar 2023 17:18:45 -0000 X-Groupsio-URL: https://lists.cip-project.org/g/cip-dev/message/11117 From: Takashi Sakamoto ALSA control core handles ELEM_READ/ELEM_WRITE requests within lock acquisition of a counting semaphore. The lock is acquired in helper functions in the end of call path before calling implementations of each driver. ioctl(2) with SNDRV_CTL_ELEM_READ ... ->snd_ctl_ioctl() ->snd_ctl_elem_read_user() ->snd_ctl_elem_read() ->down_read(controls_rwsem) ->snd_ctl_find_id() ->struct snd_kcontrol.get() ->up_read(controls_rwsem) ioctl(2) with SNDRV_CTL_ELEM_WRITE ... ->snd_ctl_ioctl() ->snd_ctl_elem_write_user() ->snd_ctl_elem_write() ->down_read(controls_rwsem) ->snd_ctl_find_id() ->struct snd_kcontrol.put() ->up_read(controls_rwsem) This commit moves the lock acquisition to middle of the call graph to simplify the helper functions. As a result: ioctl(2) with SNDRV_CTL_ELEM_READ ... ->snd_ctl_ioctl() ->snd_ctl_elem_read_user() ->down_read(controls_rwsem) ->snd_ctl_elem_read() ->snd_ctl_find_id() ->struct snd_kcontrol.get() ->up_read(controls_rwsem) ioctl(2) with SNDRV_CTL_ELEM_WRITE ... ->snd_ctl_ioctl() ->snd_ctl_elem_write_user() ->down_read(controls_rwsem) ->snd_ctl_elem_write() ->snd_ctl_find_id() ->struct snd_kcontrol.put() ->up_read(controls_rwsem) Change-Id: I6b39209aaf08afcbeca7c759b77bc96c67db4c77 Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai Fixes: e8064dec769e6 "ALSA: pcm: Move rwsem lock inside snd_ctl_elem_read to prevent UAF" Signed-off-by: Alexander Grund --- sound/core/control.c | 78 +++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 43c8eac250b8a..a042a30d6a728 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -877,24 +877,18 @@ static int snd_ctl_elem_read(struct snd_card *card, struct snd_kcontrol *kctl; struct snd_kcontrol_volatile *vd; unsigned int index_offset; - int result; - down_read(&card->controls_rwsem); kctl = snd_ctl_find_id(card, &control->id); - if (kctl == NULL) { - result = -ENOENT; - } else { - index_offset = snd_ctl_get_ioff(kctl, &control->id); - vd = &kctl->vd[index_offset]; - if ((vd->access & SNDRV_CTL_ELEM_ACCESS_READ) && - kctl->get != NULL) { - snd_ctl_build_ioff(&control->id, kctl, index_offset); - result = kctl->get(kctl, control); - } else - result = -EPERM; - } - up_read(&card->controls_rwsem); - return result; + if (kctl == NULL) + return -ENOENT; + + index_offset = snd_ctl_get_ioff(kctl, &control->id); + vd = &kctl->vd[index_offset]; + if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_READ) && kctl->get == NULL) + return -EPERM; + + snd_ctl_build_ioff(&control->id, kctl, index_offset); + return kctl->get(kctl, control); } static int snd_ctl_elem_read_user(struct snd_card *card, @@ -909,8 +903,11 @@ static int snd_ctl_elem_read_user(struct snd_card *card, snd_power_lock(card); result = snd_power_wait(card, SNDRV_CTL_POWER_D0); - if (result >= 0) + if (result >= 0) { + down_read(&card->controls_rwsem); result = snd_ctl_elem_read(card, control); + up_read(&card->controls_rwsem); + } snd_power_unlock(card); if (result >= 0) if (copy_to_user(_control, control, sizeof(*control))) @@ -927,30 +924,28 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file, unsigned int index_offset; int result; - down_read(&card->controls_rwsem); kctl = snd_ctl_find_id(card, &control->id); - if (kctl == NULL) { - result = -ENOENT; - } else { - index_offset = snd_ctl_get_ioff(kctl, &control->id); - vd = &kctl->vd[index_offset]; - if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_WRITE) || - kctl->put == NULL || - (file && vd->owner && vd->owner != file)) { - result = -EPERM; - } else { - snd_ctl_build_ioff(&control->id, kctl, index_offset); - result = kctl->put(kctl, control); - } - if (result > 0) { - struct snd_ctl_elem_id id = control->id; - up_read(&card->controls_rwsem); - snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &id); - return 0; - } + if (kctl == NULL) + return -ENOENT; + + index_offset = snd_ctl_get_ioff(kctl, &control->id); + vd = &kctl->vd[index_offset]; + if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_WRITE) || kctl->put == NULL || + (file && vd->owner && vd->owner != file)) { + return -EPERM; } - up_read(&card->controls_rwsem); - return result; + + snd_ctl_build_ioff(&control->id, kctl, index_offset); + result = kctl->put(kctl, control); + if (result < 0) + return result; + + if (result > 0) { + struct snd_ctl_elem_id id = control->id; + snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &id); + } + + return 0; } static int snd_ctl_elem_write_user(struct snd_ctl_file *file, @@ -967,8 +962,11 @@ static int snd_ctl_elem_write_user(struct snd_ctl_file *file, card = file->card; snd_power_lock(card); result = snd_power_wait(card, SNDRV_CTL_POWER_D0); - if (result >= 0) + if (result >= 0) { + down_read(&card->controls_rwsem); result = snd_ctl_elem_write(card, file, control); + up_read(&card->controls_rwsem); + } snd_power_unlock(card); if (result >= 0) if (copy_to_user(_control, control, sizeof(*control))) From patchwork Fri Mar 24 17:18:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Grund X-Patchwork-Id: 13187055 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2A82C76195 for ; Fri, 24 Mar 2023 17:18:45 +0000 (UTC) Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by mx.groups.io with SMTP id smtpd.web10.2683.1679678319157811799 for ; Fri, 24 Mar 2023 10:18:39 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=kWERMfaH; spf=pass (domain: gmail.com, ip: 209.85.128.48, mailfrom: theflamefire89@gmail.com) Received: by mail-wm1-f48.google.com with SMTP id bg16-20020a05600c3c9000b003eb34e21bdfso3642253wmb.0 for ; Fri, 24 Mar 2023 10:18:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679678317; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vvJBQTcO1qDmdJsJeQoY7Ecsv4DcspIdbB4amlE6qcU=; b=kWERMfaHrc+XcIzlqMwTFAJh5qq+8+Atg9mOnYD+TYLnzznOtgINWvWlUmkTw2VfM/ M6L8f8xYIpkYNE1Mox0ExfcZ+foJ5QkKRWEQSYGHf4iK7RLtEz6e7HDfd7qcqNpWSkf7 1w2VJ9aYjS5DPqguBljtfD3d5LNE6tSwo9W2UEGIidy0lIfGtLZQQFPA+UV3QX+a0ars OO0z3F7S4ubYhoXdq+TOaxO9bLg5OVkTLxeuz8mIaai9LjltImYfcbwE+BODajgaJTk7 4K2ZeOECytWTqQv1VaVoWbMntYaX+3Qe8+k50w3w+ZxA161/tFXrQcS1RtU6gWtgRWse n/QQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679678317; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vvJBQTcO1qDmdJsJeQoY7Ecsv4DcspIdbB4amlE6qcU=; b=L5nQhcRl4gXKlzSsi3OYS8RMiYDOc81wpnROeQJCVyB0L+Bc5uxLN6uZ8rNVa7dGzO b3a4ZxFaSUFhkAAdIZ8IUT6cM79zDLpnIdoy14VvqZfTfrhNdsjFplwhRrdUsDCkLzaw 9dZnR1rvHw4H7bjFQqztnop4yanogGi4X2VfPdYcR3VInLQvzeba4rXAUiN27GCLMXLE sEiUtOI3rUioyIv2e1sE7+fRFtGA4o1jSaDv907C2HQwrpkhIdfdp1sjU0KTWzPAxgxF aChZ3eWWkqiL9GD7pcYHGzoHeCEG1nsoe0F0ilNYenE4oZ7KS5ALWfINY3yjZyBkhFzX Qxrw== X-Gm-Message-State: AO0yUKVQ4i26uNtJYq+bFU1hC6oR/Xj+y/aIL6KZBcQ4Wn+KqyyLD2G7 m+6ybssLLV6ezZUghuWqpBCh3z9j7To= X-Google-Smtp-Source: AK7set9gnHvejJwb1oQEo6F8SCBILMaFszPCQE6WTtj5IKWik5OCP9qS8jimVSf/AAkuP4AgAR4eMA== X-Received: by 2002:a1c:ed0e:0:b0:3dc:55d9:ec8 with SMTP id l14-20020a1ced0e000000b003dc55d90ec8mr2864895wmh.41.1679678317581; Fri, 24 Mar 2023 10:18:37 -0700 (PDT) Received: from alex-Mint.fritz.box (p200300f6af150c00d964c2cc0a52faf5.dip0.t-ipconnect.de. [2003:f6:af15:c00:d964:c2cc:a52:faf5]) by smtp.googlemail.com with ESMTPSA id k22-20020a05600c1c9600b003eda46d6792sm340662wms.32.2023.03.24.10.18.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Mar 2023 10:18:37 -0700 (PDT) From: Alexander Grund To: cip-dev@lists.cip-project.org Cc: uli+cip@fpond.eu Subject: [PATCH 4.4 2/3] ALSA: control: Fix memory corruption risk in snd_ctl_elem_read Date: Fri, 24 Mar 2023 18:18:11 +0100 Message-Id: <20230324171812.221086-3-theflamefire89@gmail.com> X-Mailer: git-send-email 2.40.0 In-Reply-To: <20230324171812.221086-1-theflamefire89@gmail.com> References: <20230324171812.221086-1-theflamefire89@gmail.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Fri, 24 Mar 2023 17:18:45 -0000 X-Groupsio-URL: https://lists.cip-project.org/g/cip-dev/message/11118 From: Richard Fitzgerald commit 5a23699a39abc5328921a81b89383d088f6ba9cc upstream. The patch "ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations" introduced a potential for kernel memory corruption due to an incorrect if statement allowing non-readable controls to fall through and call the get function. For TLV controls a driver can omit SNDRV_CTL_ELEM_ACCESS_READ to ensure that only the TLV get function can be called. Instead the normal get() can be invoked unexpectedly and as the driver expects that this will only be called for controls <= 512 bytes, potentially try to copy >512 bytes into the 512 byte return array, so corrupting kernel memory. The problem is an attempt to refactor the snd_ctl_elem_read function to invert the logic so that it conditionally aborted if the control is unreadable instead of conditionally executing. But the if statement wasn't inverted correctly. The correct inversion of if (a && !b) is if (!a || b) Fixes: becf9e5d553c2 ("ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations") Signed-off-by: Richard Fitzgerald Cc: Signed-off-by: Takashi Iwai Signed-off-by: Greg Kroah-Hartman Signed-off-by: Alexander Grund --- sound/core/control.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/core/control.c b/sound/core/control.c index a042a30d6a728..3ca81e85a1492 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -884,7 +884,7 @@ static int snd_ctl_elem_read(struct snd_card *card, index_offset = snd_ctl_get_ioff(kctl, &control->id); vd = &kctl->vd[index_offset]; - if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_READ) && kctl->get == NULL) + if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_READ) || kctl->get == NULL) return -EPERM; snd_ctl_build_ioff(&control->id, kctl, index_offset); From patchwork Fri Mar 24 17:18:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Grund X-Patchwork-Id: 13187056 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1B27C6FD1C for ; Fri, 24 Mar 2023 17:18:45 +0000 (UTC) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by mx.groups.io with SMTP id smtpd.web10.2688.1679678320990569979 for ; Fri, 24 Mar 2023 10:18:41 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=A3GRw9Sb; spf=pass (domain: gmail.com, ip: 209.85.128.52, mailfrom: theflamefire89@gmail.com) Received: by mail-wm1-f52.google.com with SMTP id s13so1562507wmr.4 for ; Fri, 24 Mar 2023 10:18:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679678319; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=f1rgbf1nN6IXvclcq60zzZ83i7TOhjFvAGymV/bBlkA=; b=A3GRw9SbIQscWeT5+le5wXhMN5qz019FbeITALoJQtLQ1JynqyJm/ukbmZuGWFAaVH 7O1nTSpesSmyPFzdPyupVNl/wnoSzmptMBVmcRUbpdPFMRrRBA0D2bX3SJqzhAnrf+TX DZPtw+dBXlolaSNK1g1F4VwVwWH1QTQVCDsLOtIponNK04VQTGJhn5Pa4MqcxWLc9iAf flXxl0UVxNp+GcjrYSo59mVyoJAEBnmUwlj7W9Xsxuytd6XWZ3mjLv5WGhNGiW113oxm 2q0SXsFpHWqA60ODWNsIy43TROstGyU7dVXP9sx7HLoMUfhtGH2QbSV+Aq0zhcG5w3HU 8oWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679678319; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=f1rgbf1nN6IXvclcq60zzZ83i7TOhjFvAGymV/bBlkA=; b=H4sCO+NazKoZc1EOk4W+zlxWW07g2UN6k1BYqLkxRI0RuFJyziKh2HWs/l+Flr4rga zka/bIKwmtAEcoSbg1gcvjC2xNogs7oTT6gR4j9simyDp91Unbm7U+F4HGDSch/k3CMl 1zNV5egdoYfQtYelUemhETMbjn7XVu9vPjcbRtB17CmjP1E/paMx2LEGX0FJkxXn+ahD YOlcy+zPwLXr5owfuWtXFfMiN4150pGFkQBYGCuomWurHpaCXcme3+5W2zx6nPs7CzqY +QXfr0Hf30Z1xN47OiCmhBlSz065L3uUM/25NiSgGyY6fQiAKLqnKNzplRGfMnrZJ2dv xDrA== X-Gm-Message-State: AO0yUKUWa8uaExkn/kD1bTOTGPw+MoemZYaEV7+nQmul6gK41E7cMKzQ DTY9r48niOSzOs0oQLjDIyGtx1VrNlY= X-Google-Smtp-Source: AK7set9ARAujKWfmVHBeK6CO+3TuuSbmafTFpiHgdlnLK1ugc7R00McYO46/335TebRz1OYn4JiQQg== X-Received: by 2002:a05:600c:2312:b0:3dc:4fd7:31e9 with SMTP id 18-20020a05600c231200b003dc4fd731e9mr3512327wmo.7.1679678319366; Fri, 24 Mar 2023 10:18:39 -0700 (PDT) Received: from alex-Mint.fritz.box (p200300f6af150c00d964c2cc0a52faf5.dip0.t-ipconnect.de. [2003:f6:af15:c00:d964:c2cc:a52:faf5]) by smtp.googlemail.com with ESMTPSA id k22-20020a05600c1c9600b003eda46d6792sm340662wms.32.2023.03.24.10.18.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Mar 2023 10:18:38 -0700 (PDT) From: Alexander Grund To: cip-dev@lists.cip-project.org Cc: uli+cip@fpond.eu Subject: [PATCH 4.4 3/3] ALSA: control: use counting semaphore as write lock for ELEM_WRITE operation Date: Fri, 24 Mar 2023 18:18:12 +0100 Message-Id: <20230324171812.221086-4-theflamefire89@gmail.com> X-Mailer: git-send-email 2.40.0 In-Reply-To: <20230324171812.221086-1-theflamefire89@gmail.com> References: <20230324171812.221086-1-theflamefire89@gmail.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Fri, 24 Mar 2023 17:18:45 -0000 X-Groupsio-URL: https://lists.cip-project.org/g/cip-dev/message/11119 From: Takashi Sakamoto In ALSA control interface, applications can execute two types of request for value of members on each element; ELEM_READ and ELEM_WRITE. In ALSA control core, these two requests are handled within read lock of a counting semaphore, therefore several processes can run to execute these two requests at the same time. This has an issue because ELEM_WRITE requests have an effect to change state of the target element. Concurrent access should be controlled for each of ELEM_READ/ELEM_WRITE case. This commit uses the counting semaphore as write lock for ELEM_WRITE requests, while use it as read lock for ELEM_READ requests. The state of a target element is maintained exclusively between ELEM_WRITE/ELEM_READ operations. There's a concern. If the counting semaphore is acquired for read lock in implementations of 'struct snd_kcontrol.put()' in each driver, this commit shall cause dead lock. As of v4.13-rc5, 'snd-mixer-oss.ko', 'snd-emu10k1.ko' and 'snd-soc-sst-atom-hifi2-platform.ko' includes codes for read locks, but these are not in a call graph from 'struct snd_kcontrol.put(). Therefore, this commit is safe. In current implementation, the same solution is applied for the other operations to element; e.g. ELEM_LOCK and ELEM_UNLOCK. There's another discussion about an overhead to maintain concurrent access to an element during operating the other elements on the same card instance, because the lock primitive is originally implemented to maintain a list of elements on the card instance. There's a substantial difference between per-element-list lock and per-element lock. Here, let me investigate another idea to add per-element lock to maintain the concurrent accesses with inquiry/change requests to an element. It's not so frequent for applications to operate members on elements, while adding a new lock primitive to structure increases memory footprint for all of element sets somehow. Experimentally, inquiry operation is more frequent than change operation and usage of counting semaphore for the inquiry operation brings no blocking to the other inquiry operations. Thus the overhead is not so critical for usual applications. For the above reasons, in this commit, the per-element lock is not introduced. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai Signed-off-by: Alexander Grund --- sound/core/control.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 3ca81e85a1492..04b939df3768b 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -963,9 +963,9 @@ static int snd_ctl_elem_write_user(struct snd_ctl_file *file, snd_power_lock(card); result = snd_power_wait(card, SNDRV_CTL_POWER_D0); if (result >= 0) { - down_read(&card->controls_rwsem); + down_write(&card->controls_rwsem); result = snd_ctl_elem_write(card, file, control); - up_read(&card->controls_rwsem); + up_write(&card->controls_rwsem); } snd_power_unlock(card); if (result >= 0)