linux-kernel - [PATCH 4.5 038/200] ALSA: hda - Dont trust the reported actual power state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20160503000555.860975143@linuxfoundation.org>
Date:	Mon,  2 May 2016 17:10:37 -0700
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	linux-kernel@...r.kernel.org
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	stable@...r.kernel.org, Takashi Iwai <tiwai@...e.de>
Subject: [PATCH 4.5 038/200] ALSA: hda - Dont trust the reported actual power state

4.5-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Takashi Iwai <tiwai@...e.de>

commit 50fd4987c4f3c3ebf0ce94d932732011bbdc7c71 upstream.

We've got a regression report that the recording on Mac with a cirrus
codec doesn't work any longer.  This turned out to be the missing
power up to D0 by power_save_node enablement.

After analyzing the traces, we found out that the culprit is that the
codec advertises the "actual" power state of a few nodes to be D0
while the "target" power state is D3.  This inconsistency is usually
OK, as it implies the power transition.  But in the case of cirrus
codec, this seems to be stuck to D3 while it's not actually D0.

This patch addresses the issue by checking the power state difference
more strictly.  It sends the power-state change verb unless both the
target and the actual power states show the given value.

We may introduce yet another flag indicating the possible broken
hardware power state, but it's anyway safer to set the proper power
state even in a transition (at least it's harmless as long as the
target state is same).  So this simpler change was applied now.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=116171
Signed-off-by: Takashi Iwai <tiwai@...e.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 sound/pci/hda/hda_generic.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -826,7 +826,7 @@ static hda_nid_t path_power_update(struc
 				   bool allow_powerdown)
 {
 	hda_nid_t nid, changed = 0;
-	int i, state;
+	int i, state, power;

 	for (i = 0; i < path->depth; i++) {
 		nid = path->path[i];
@@ -838,7 +838,9 @@ static hda_nid_t path_power_update(struc
 			state = AC_PWRST_D0;
 		else
 			state = AC_PWRST_D3;
-		if (!snd_hda_check_power_state(codec, nid, state)) {
+		power = snd_hda_codec_read(codec, nid, 0,
+					   AC_VERB_GET_POWER_STATE, 0);
+		if (power != (state | (state << 4))) {
 			snd_hda_codec_write(codec, nid, 0,
 					    AC_VERB_SET_POWER_STATE, state);
 			changed = nid;