[alsa-devel] [PATCH] ASoC: Jack: Fix NULL pointer dereference in snd_soc_jack_report

[alsa-devel] [PATCH] ASoC: Jack: Fix NULL pointer dereference in snd_soc_jack_report

[Pawel Harlozinski]

Check for existance of jack before tracing.
NULL pointer dereference has been reported by KASAN while unloading
machine driver (snd_soc_cnl_rt274).

Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com>

---
 sound/soc/soc-jack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index a71d2340eb05..b5748dcd490f 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -82,10 +82,9 @@ void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask)
 	unsigned int sync = 0;
 	int enable;
 
-	trace_snd_soc_jack_report(jack, mask, status);
-
 	if (!jack)
 		return;
+	trace_snd_soc_jack_report(jack, mask, status);
 
 	dapm = &jack->card->dapm;
 
-- 
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH] ASoC: rt274: Disable jack report IRQ with disabling jack

[Pawel Harlozinski]

When machine driver is unloaded there is nothing to handle
jack detect reports. This were causing flood of messages
from unhandled IRQs.

Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com>

---
 sound/soc/codecs/rt274.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt274.c b/sound/soc/codecs/rt274.c
index cbb5e176d11a..993fd365524a 100644
--- a/sound/soc/codecs/rt274.c
+++ b/sound/soc/codecs/rt274.c
@@ -408,13 +408,13 @@ static int rt274_mic_detect(struct snd_soc_component *component,
 		/* Disable jack detection */
 		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
 					RT274_IRQ_EN, RT274_IRQ_DIS);
-
+		disable_irq(rt274->i2c->irq);
 		return 0;
 	}
 
 	regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
 				RT274_IRQ_EN, RT274_IRQ_EN);
-
+	enable_irq(rt274->i2c->irq);
 	/* Send an initial report */
 	rt274_irq(0, rt274);
 
@@ -1197,6 +1197,8 @@ static int rt274_i2c_probe(struct i2c_client *i2c,
 				"Failed to reguest IRQ: %d\n", ret);
 			return ret;
 		}
+		/* Gets re-enabled by .set_jack = rt274_mic_detect  */
+		disable_irq(rt274->i2c->irq);
 	}
 
 	ret = devm_snd_soc_register_component(&i2c->dev,
-- 
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH] ASoC: rt274: Disable jack report IRQ with disabling jack

[Mark Brown]

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

On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:

>  		/* Disable jack detection */
>  		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
>  					RT274_IRQ_EN, RT274_IRQ_DIS);
> -
> +		disable_irq(rt274->i2c->irq);
>  		return 0;

Shouldn't the register update above be suppressing interrupts?
disable_irq() is a bit of a hammer and interferes with things like
possible share use.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: Jack: Fix NULL pointer dereference in snd_soc_jack_report" to the asoc tree

[Mark Brown]

The patch

   ASoC: Jack: Fix NULL pointer dereference in snd_soc_jack_report

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 8f157d4ff039e03e2ed4cb602eeed2fd4687a58f Mon Sep 17 00:00:00 2001
From: Pawel Harlozinski <pawel.harlozinski@linux.intel.com>
Date: Tue, 12 Nov 2019 14:02:36 +0100
Subject: [PATCH] ASoC: Jack: Fix NULL pointer dereference in
 snd_soc_jack_report

Check for existance of jack before tracing.
NULL pointer dereference has been reported by KASAN while unloading
machine driver (snd_soc_cnl_rt274).

Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com>
Link: https://lore.kernel.org/r/20191112130237.10141-1-pawel.harlozinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-jack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index a71d2340eb05..b5748dcd490f 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -82,10 +82,9 @@ void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask)
 	unsigned int sync = 0;
 	int enable;
 
-	trace_snd_soc_jack_report(jack, mask, status);
-
 	if (!jack)
 		return;
+	trace_snd_soc_jack_report(jack, mask, status);
 
 	dapm = &jack->card->dapm;
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH] ASoC: rt274: Disable jack report IRQ with disabling jack

[Harlozinski, Pawel]

On 11/12/2019 6:10 PM, Mark Brown wrote:
> On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:
>
>>   		/* Disable jack detection */
>>   		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
>>   					RT274_IRQ_EN, RT274_IRQ_DIS);
>> -
>> +		disable_irq(rt274->i2c->irq);
>>   		return 0;
> Shouldn't the register update above be suppressing interrupts?
For rt274 disable_irq is also needed, otherwise we're getting flood of 
irq's in case of not loaded machine board.
> disable_irq() is a bit of a hammer and interferes with things like
> possible share use.

This irq should be handled in codec code anyway - control of jack detect 
events from non-codec code is done with set_jack.
Similar solutions for jack report irq enable/disable flow are 
implemented in rt5640 and rt5651.

> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH] ASoC: rt274: Disable jack report IRQ with disabling jack

[Mark Brown]

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

On Wed, Nov 13, 2019 at 02:55:53PM +0100, Harlozinski, Pawel wrote:
> On 11/12/2019 6:10 PM, Mark Brown wrote:
> > On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:

> > >   		/* Disable jack detection */
> > >   		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
> > >   					RT274_IRQ_EN, RT274_IRQ_DIS);
> > > -
> > > +		disable_irq(rt274->i2c->irq);
> > >   		return 0;

> > Shouldn't the register update above be suppressing interrupts?
> For rt274 disable_irq is also needed, otherwise we're getting flood of irq's
> in case of not loaded machine board.

Through what mechanism is it needed?  If your machine driver is having
an impact on this I'm rather worried.

> > disable_irq() is a bit of a hammer and interferes with things like
> > possible share use.

> This irq should be handled in codec code anyway - control of jack detect
> events from non-codec code is done with set_jack.

The issue isn't that this is in CODEC code, the issue is that it's
usually very worrying to need to explicitly disable and enable an
interrupt at the controller level when the device appears to have
controls that should stop it asserting the interrupt when it's not
wanted.

> Similar solutions for jack report irq enable/disable flow are implemented in
> rt5640 and rt5651.

This may be an indication that those drivers should be improved rather
than that they should be copied.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel