linux-kernel - [PATCHv2] ASoC: fsl: imx-ssi: omit ssi counter to avoid harm in unbalanced situation

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]

Date:	16 Nov 2013 07:52:25 +0000
From:	"Oskar Schirmer" <oskar@...ra.com>
To:	"Liam Girdwood" <lgirdwood@...il.com>
Cc:	"Fabio Estevam" <fabio.estevam@...escale.com>,
	"Mark Brown" <broonie@...nel.org>,
	"Sascha Hauer" <s.hauer@...gutronix.de>,
	alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Oskar Schirmer" <oskar@...ra.com>
Subject: [PATCHv2] ASoC: fsl: imx-ssi: omit ssi counter to avoid harm in unbalanced situation

Unbalanced calls to imx_ssi_trigger() may result in endless
SSI activity and thus provoke eternal sound. While on the first glance,
the switch statement looks pretty symmetric, the SUSPEND/RESUME
pair is not: the suspend case comes along snd_pcm_suspend_all(),
which for fsl/imx-pcm-fiq is called only at snd_soc_suspend(),
but the resume case originates straight from the SNDRV_PCM_IOCTL_RESUME.
This way userland may provoke an unbalanced resume, which might cause
the ssi->enabled counter to increase and never return to zero again,
so eventually SSI_SCR_SSIEN is never disabled.

As the information on whether to enable the SSI or not is contained
in the two bits for TE/RE, we save all the software mirroring of
hardware state here and simply use the hardware register itself
to keep the state of whether someone is currently playing or capturing.

This is essentially the same stuff as in sound/soc/fsl/imx-pcm-fiq.c
which I send a patch for three days ago. Astonishing enough this
highly fragile scheme is used twice in parallel to serve the very
same control function, synchronously: Once out of sync you are lost
until reboot.

Note, that these fixes wont prevent state machine distortion on alsa
level to cut sound or the like. It just makes sure we have a chance
to synchronise again later on.

Signed-off-by: Oskar Schirmer <oskar@...ra.com>
---
v2: drop all software flag handling in favour of hardware registers

 sound/soc/fsl/imx-ssi.c |    5 ++---
 sound/soc/fsl/imx-ssi.h |    1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
index f5f248c..2eb2691 100644
--- a/sound/soc/fsl/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -304,8 +304,7 @@ static int imx_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 			scr |= SSI_SCR_RE;
 		sier |= sier_bits;

-		if (++ssi->enabled == 1)
-			scr |= SSI_SCR_SSIEN;
+		scr |= SSI_SCR_SSIEN;

 		break;

@@ -318,7 +317,7 @@ static int imx_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 			scr &= ~SSI_SCR_RE;
 		sier &= ~sier_bits;

-		if (--ssi->enabled == 0)
+		if (!(scr & (SSI_SCR_TE | SSI_SCR_RE)))
 			scr &= ~SSI_SCR_SSIEN;

 		break;
diff --git a/sound/soc/fsl/imx-ssi.h b/sound/soc/fsl/imx-ssi.h
index 560c40f..be65623 100644
--- a/sound/soc/fsl/imx-ssi.h
+++ b/sound/soc/fsl/imx-ssi.h
@@ -213,7 +213,6 @@ struct imx_ssi {

 	int fiq_init;
 	int dma_init;
-	int enabled;
 };

 #endif /* _IMX_SSI_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/