All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
@ 2017-04-03  6:31 Kuninori Morimoto
  2017-04-03  8:28 ` Kuninori Morimoto
  2017-04-05 17:30   ` Mark Brown
  0 siblings, 2 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2017-04-03  6:31 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-Renesas, Linux-ALSA, Lars-Peter, Simon

Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
components. If system uses Kernel modules, we can disable these drivers
by using rmmod command. In such case, we can't disable
CPU/Codec/Platform driver without disabling Sound Card driver.

But on the other hand, we can disable these drivers by using unbind
command. In such case, we can disable these drivers randomly.
In this case, we can create dirty Sound Card which is missing necessary
components.

(1) If user disabled Sound Card first, but did nothing to other drivers,
user can't use Sound because Sound Card is no longer exists.
(2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
to Sound Card, user still be able to use Sound Card, because dirty Sound
Card still exists. In this case, Sound system will be crashed if user
started sound playback/capture. But we can't block such random unbind
now.

To avoid Sound Card crash in (2) case, we need to unregister Sound Card
whenever CPU/Codec/Platform component were unregistered.
This patch solves this issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 07e4eec..52760bd 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3236,6 +3236,11 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
 
 static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
+	if (card)
+		snd_soc_unregister_card(card);
+
 	list_del(&component->list);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-04-03  6:31 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
@ 2017-04-03  8:28 ` Kuninori Morimoto
  2017-04-05 17:30   ` Mark Brown
  1 sibling, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2017-04-03  8:28 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-Renesas, Linux-ALSA, Lars-Peter, Simon


Hi Mark.

Sorry, this is v2 patch.

	- Subject: Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
	+ Subject: Re: [RFC][PATCH v2] ASoC: soc-core: verify Sound Card normality

> Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
> components. If system uses Kernel modules, we can disable these drivers
> by using rmmod command. In such case, we can't disable
> CPU/Codec/Platform driver without disabling Sound Card driver.
> 
> But on the other hand, we can disable these drivers by using unbind
> command. In such case, we can disable these drivers randomly.
> In this case, we can create dirty Sound Card which is missing necessary
> components.
> 
> (1) If user disabled Sound Card first, but did nothing to other drivers,
> user can't use Sound because Sound Card is no longer exists.
> (2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
> to Sound Card, user still be able to use Sound Card, because dirty Sound
> Card still exists. In this case, Sound system will be crashed if user
> started sound playback/capture. But we can't block such random unbind
> now.
> 
> To avoid Sound Card crash in (2) case, we need to unregister Sound Card
> whenever CPU/Codec/Platform component were unregistered.
> This patch solves this issue.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 07e4eec..52760bd 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -3236,6 +3236,11 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
>  
>  static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
>  {
> +	struct snd_soc_card *card = component->card;
> +
> +	if (card)
> +		snd_soc_unregister_card(card);
> +
>  	list_del(&component->list);
>  }
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Applied "ASoC: soc-core: verify Sound Card normality" to the asoc tree
  2017-04-03  6:31 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
@ 2017-04-05 17:30   ` Mark Brown
  2017-04-05 17:30   ` Mark Brown
  1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2017-04-05 17:30 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Mark Brown, Mark Brown, Linux-Renesas, Linux-ALSA, Lars-Peter,
	Simon, alsa-devel

The patch

   ASoC: soc-core: verify Sound Card normality

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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 c12c1aad98bb75b435e79c6208b56d2018b42f8b Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Mon, 3 Apr 2017 06:31:22 +0000
Subject: [PATCH] ASoC: soc-core: verify Sound Card normality

Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
components. If system uses Kernel modules, we can disable these drivers
by using rmmod command. In such case, we can't disable
CPU/Codec/Platform driver without disabling Sound Card driver.

But on the other hand, we can disable these drivers by using unbind
command. In such case, we can disable these drivers randomly.
In this case, we can create dirty Sound Card which is missing necessary
components.

(1) If user disabled Sound Card first, but did nothing to other drivers,
user can't use Sound because Sound Card is no longer exists.
(2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
to Sound Card, user still be able to use Sound Card, because dirty Sound
Card still exists. In this case, Sound system will be crashed if user
started sound playback/capture. But we can't block such random unbind
now.

To avoid Sound Card crash in (2) case, we need to unregister Sound Card
whenever CPU/Codec/Platform component were unregistered.
This patch solves this issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f1901bb1466e..de6d5609c252 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3076,6 +3076,11 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
 
 static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
+	if (card)
+		snd_soc_unregister_card(card);
+
 	list_del(&component->list);
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Applied "ASoC: soc-core: verify Sound Card normality" to the asoc tree
@ 2017-04-05 17:30   ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2017-04-05 17:30 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-Renesas, alsa-devel, Mark Brown, Lars-Peter, Simon

The patch

   ASoC: soc-core: verify Sound Card normality

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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 c12c1aad98bb75b435e79c6208b56d2018b42f8b Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Mon, 3 Apr 2017 06:31:22 +0000
Subject: [PATCH] ASoC: soc-core: verify Sound Card normality

Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
components. If system uses Kernel modules, we can disable these drivers
by using rmmod command. In such case, we can't disable
CPU/Codec/Platform driver without disabling Sound Card driver.

But on the other hand, we can disable these drivers by using unbind
command. In such case, we can disable these drivers randomly.
In this case, we can create dirty Sound Card which is missing necessary
components.

(1) If user disabled Sound Card first, but did nothing to other drivers,
user can't use Sound because Sound Card is no longer exists.
(2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
to Sound Card, user still be able to use Sound Card, because dirty Sound
Card still exists. In this case, Sound system will be crashed if user
started sound playback/capture. But we can't block such random unbind
now.

To avoid Sound Card crash in (2) case, we need to unregister Sound Card
whenever CPU/Codec/Platform component were unregistered.
This patch solves this issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f1901bb1466e..de6d5609c252 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3076,6 +3076,11 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
 
 static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
+	if (card)
+		snd_soc_unregister_card(card);
+
 	list_del(&component->list);
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-04-03  8:26         ` Kuninori Morimoto
@ 2017-04-03  8:37           ` Takashi Iwai
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2017-04-03  8:37 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-Renesas, Linux-ALSA, Mark Brown, Lars-Peter, Simon

On Mon, 03 Apr 2017 10:26:05 +0200,
Kuninori Morimoto wrote:
> 
> 
> Hi Takashi-san
> 
> > > So, we can call snd_soc_unregister_card() whenever CPU/Codec/Platform
> > > were unregsiterd.
> > 
> > In theory yes, but you should be careful to do so, e.g. make sure that
> > it won't be called again by the removal/unbind of other components /
> > drivers.
> > 
> > I suggested snd_card_disconnect() because it doesn't release resources
> > by itself, but it just disconnects from the further accesses.  So,
> > double-free won't happen in this case.  It makes the hotunplug safer
> > as long as the drivers manage the resource releases properly.
> 
> I had checked many unbind/bind pattern/order on 2nd [RFC] patch which I posted.
> At first, I believe Oops on unbind/bind issue was solved on it.
> 2nd, if my understanding was correct, it doesn't have double-free issue,
> or something like that.
> But, I'm not 100% sure about 2nd, thus it has [RFC] on patch.

Ah, I see that snd_soc_unregister_card() has the check of
card->instantiated, so it should be fine to call multiple times.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-03-30 21:53 ` Mark Brown
@ 2017-03-31  0:30   ` Kuninori Morimoto
  2017-03-31  7:48   ` [alsa-devel] " Takashi Iwai
  1 sibling, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2017-03-31  0:30 UTC (permalink / raw)
  To: Mark Brown; +Cc: Lars-Peter, Simon, Linux-Renesas, Linux-ALSA


Hi Mark

Thank you for your feedback

> > To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> > flag on Sound Card, and avoid to open Sound Card.
> > This patch solved this issue.
> 
> I think this is a good direction to at least start to mitigate these
> problems (which we really should be doing) and hopefully make it easier
> to do further improvements in future.  There's obviously more places
> where we should be checking the flag (controls for example) but they can
> be added later.  One thing I would like to see is instead of setting the
> flag directly when we see a problem call a function to do it.  That way
> if we want to improve things in the future we can do that without having
> to update the callers again.

I guess you are pointing about snd_soc_dapm_shutdown() in snd_soc_unregister_card() ?
If so, I agree. Actually, I don't like this kind of adhoc handling.
I want to explain about it.

Order
[1] Disable Sound Card         -> Disable CPU/Codec/Platform or do nothing
[2] Disable CPU/Codec/Platform -> Disable Card or do nothing

Operation
1) shutdown all Card connected DAPM
2) cleanup Card resource.

In case of order [1], operation 1) -> 2) is no problem, because all card connected DAPM
exists on Card.
But, in case of order [2], operation 1) will try to call disconnected DAPM.
The DAPM disconnection from Card is done by snd_soc_dapm_free() which is called from
soc_remove_component().
This soc_remove_component() will be called with "remove_order" from operation 2).
One note is that reordering operation to 2) -> 1) will avoid this crash issue,
but it is no meaning, because 1) will do nothing ;)

Here, other solution is calling soc_remove_component() without "remove_order"
or calling snd_soc_dapm_free() when CPU/Codec/Platform are disabled.
But, 1 bad-point is that disabled CPU/Codec/Platform have no chance to
be called about snd_soc_dapm_set_bias_level() (= 1) operation).
Of course, adhoc function can solve this issue too.

If you are OK, I can work for this idea and remove adhoc operation
from snd_soc_unregister_card()

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-03-29  2:45 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
  2017-03-29  7:29   ` Geert Uytterhoeven
@ 2017-03-30 21:53 ` Mark Brown
  2017-03-31  0:30   ` Kuninori Morimoto
  2017-03-31  7:48   ` [alsa-devel] " Takashi Iwai
  1 sibling, 2 replies; 11+ messages in thread
From: Mark Brown @ 2017-03-30 21:53 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Lars-Peter, Simon, Linux-Renesas, Linux-ALSA

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Wed, Mar 29, 2017 at 02:45:37AM +0000, Kuninori Morimoto wrote:

> To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> flag on Sound Card, and avoid to open Sound Card.
> This patch solved this issue.

I think this is a good direction to at least start to mitigate these
problems (which we really should be doing) and hopefully make it easier
to do further improvements in future.  There's obviously more places
where we should be checking the flag (controls for example) but they can
be added later.  One thing I would like to see is instead of setting the
flag directly when we see a problem call a function to do it.  That way
if we want to improve things in the future we can do that without having
to update the callers again.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-03-29  7:29   ` Geert Uytterhoeven
  (?)
@ 2017-03-29  8:47   ` Kuninori Morimoto
  -1 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2017-03-29  8:47 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Lars-Peter, Simon, Linux-Renesas, Linux-ALSA


Hi Geert

> > Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
> > drivers. If system uses Kernel modules, we can disable these drivers
> > by using rmmod command. In such case, we can't disable
> > CPU/Codec/Platform driver without disabling Sound Card driver.
> >
> > But on the other hand, we can disable these drivers by using unbind
> > command. In such case, we can disable these drivers randomly.
> > In this case, we can create dirty Sound Card which is missing necessary
> > components.
> >
> > (1) If user disabled Sound Card first, but did nothing to other drivers,
> > user can't use Sound because Sound Card is no longer exists.
> > (2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
> > to Sound Card, user still be able to use Sound Card, because dirty Sound
> > Card which is missing necessary components still exists.
> > In this case, Sound system will be crashed if user started sound
> > playback/capture. But we can't block such random unbind now.
> >
> > To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> > flag on Sound Card, and avoid to open Sound Card.
> > This patch solved this issue.
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> > --- a/sound/soc/soc-core.c
> > +++ b/sound/soc/soc-core.c
> 
> > @@ -3236,6 +3244,8 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
> >
> >  static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
> >  {
> > +       if (component->card)
> > +               component->card->dirty = 1;
> 
> Currently this is the only assignment to .dirty.
> Can this be undone later, when the driver is bound again?

Does this "undone" mean "card->dirty = 0" ?

Card itself will be freed if Card was unbind.
Thus, card will be re-created if it was bind again.
.dirty will be automatically 0 cleared.

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
  2017-03-29  2:45 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
@ 2017-03-29  7:29   ` Geert Uytterhoeven
  2017-03-30 21:53 ` Mark Brown
  1 sibling, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2017-03-29  7:29 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Mark Brown, Lars-Peter, Simon, Linux-Renesas, Linux-ALSA

Hi Morimoto-san,

On Wed, Mar 29, 2017 at 4:45 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
> drivers. If system uses Kernel modules, we can disable these drivers
> by using rmmod command. In such case, we can't disable
> CPU/Codec/Platform driver without disabling Sound Card driver.
>
> But on the other hand, we can disable these drivers by using unbind
> command. In such case, we can disable these drivers randomly.
> In this case, we can create dirty Sound Card which is missing necessary
> components.
>
> (1) If user disabled Sound Card first, but did nothing to other drivers,
> user can't use Sound because Sound Card is no longer exists.
> (2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
> to Sound Card, user still be able to use Sound Card, because dirty Sound
> Card which is missing necessary components still exists.
> In this case, Sound system will be crashed if user started sound
> playback/capture. But we can't block such random unbind now.
>
> To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> flag on Sound Card, and avoid to open Sound Card.
> This patch solved this issue.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c

> @@ -3236,6 +3244,8 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
>
>  static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
>  {
> +       if (component->card)
> +               component->card->dirty = 1;

Currently this is the only assignment to .dirty.
Can this be undone later, when the driver is bound again?

>         list_del(&component->list);
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
@ 2017-03-29  7:29   ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2017-03-29  7:29 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Linux-Renesas, Linux-ALSA, Mark Brown, Lars-Peter, Simon

Hi Morimoto-san,

On Wed, Mar 29, 2017 at 4:45 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
> drivers. If system uses Kernel modules, we can disable these drivers
> by using rmmod command. In such case, we can't disable
> CPU/Codec/Platform driver without disabling Sound Card driver.
>
> But on the other hand, we can disable these drivers by using unbind
> command. In such case, we can disable these drivers randomly.
> In this case, we can create dirty Sound Card which is missing necessary
> components.
>
> (1) If user disabled Sound Card first, but did nothing to other drivers,
> user can't use Sound because Sound Card is no longer exists.
> (2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
> to Sound Card, user still be able to use Sound Card, because dirty Sound
> Card which is missing necessary components still exists.
> In this case, Sound system will be crashed if user started sound
> playback/capture. But we can't block such random unbind now.
>
> To avoid Sound Card crash in (2) case, what we can do now is, add dirty
> flag on Sound Card, and avoid to open Sound Card.
> This patch solved this issue.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c

> @@ -3236,6 +3244,8 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
>
>  static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
>  {
> +       if (component->card)
> +               component->card->dirty = 1;

Currently this is the only assignment to .dirty.
Can this be undone later, when the driver is bound again?

>         list_del(&component->list);
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC][PATCH] ASoC: soc-core: verify Sound Card normality
@ 2017-03-29  2:45 Kuninori Morimoto
  2017-03-29  7:29   ` Geert Uytterhoeven
  2017-03-30 21:53 ` Mark Brown
  0 siblings, 2 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2017-03-29  2:45 UTC (permalink / raw)
  To: Mark Brown, Lars-Peter; +Cc: Simon, Linux-Renesas, Linux-ALSA

Current ALSA SoC Sound Card basically consists of CPU/Codec/Platform
drivers. If system uses Kernel modules, we can disable these drivers
by using rmmod command. In such case, we can't disable
CPU/Codec/Platform driver without disabling Sound Card driver.

But on the other hand, we can disable these drivers by using unbind
command. In such case, we can disable these drivers randomly.
In this case, we can create dirty Sound Card which is missing necessary
components.

(1) If user disabled Sound Card first, but did nothing to other drivers,
user can't use Sound because Sound Card is no longer exists.
(2) If user disabled CPU/Codec/Platform driver randomly, but did nothing
to Sound Card, user still be able to use Sound Card, because dirty Sound
Card which is missing necessary components still exists.
In this case, Sound system will be crashed if user started sound
playback/capture. But we can't block such random unbind now.

To avoid Sound Card crash in (2) case, what we can do now is, add dirty
flag on Sound Card, and avoid to open Sound Card.
This patch solved this issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
Mark, Lars-Peter

This is still RFC, but I think we need to consider about this issue.
I tested this patch on my system, and it can block to sound card crash
for me

 include/sound/soc.h  |  6 ++++--
 sound/soc/soc-core.c | 12 +++++++++++-
 sound/soc/soc-pcm.c  | 20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 316fdce..4813fc5 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1106,8 +1106,6 @@ struct snd_soc_card {
 	struct mutex mutex;
 	struct mutex dapm_mutex;
 
-	bool instantiated;
-
 	int (*probe)(struct snd_soc_card *card);
 	int (*late_probe)(struct snd_soc_card *card);
 	int (*remove)(struct snd_soc_card *card);
@@ -1197,6 +1195,10 @@ struct snd_soc_card {
 	u32 pop_time;
 
 	void *drvdata;
+
+	/* bit field */
+	u32 instantiated:1;
+	u32 dirty:1;
 };
 
 /* SoC machine DAI configuration, glues a codec and cpu DAI together */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 07e4eec..a18e106 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2882,7 +2882,15 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
 {
 	if (card->instantiated) {
 		card->instantiated = false;
-		snd_soc_dapm_shutdown(card);
+		if (!card->dirty) {
+			/*
+			 * In case of unbind, CPU/Codec/Platform component can
+			 * be unbinded *before* Card unbind. In such case
+			 * (= card->dirty) Card connected DAPMs are already
+			 * doesn't exist.
+			 */
+			snd_soc_dapm_shutdown(card);
+		}
 		soc_cleanup_card_resources(card);
 		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
 	}
@@ -3236,6 +3244,8 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
 
 static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
 {
+	if (component->card)
+		component->card->dirty = 1;
 	list_del(&component->list);
 }
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index efc5831..c861aba 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -455,6 +455,16 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	const char *codec_dai_name = "multicodec";
 	int i, ret = 0;
 
+	if (rtd->card->dirty) {
+		/*
+		 * In case of unbind, each CPU/Codec/Platform component can
+		 * be unbinded randomly. In such case (= card->dirty)
+		 * Sound Card is no longer safety. Don't open it.
+		 */
+		dev_warn(cpu_dai->dev, "Sound SoC Card is dirty, Recheck your system\n");
+		return -ENXIO;
+	}
+
 	pinctrl_pm_select_default_state(cpu_dai->dev);
 	for (i = 0; i < rtd->num_codecs; i++)
 		pinctrl_pm_select_default_state(rtd->codec_dais[i]->dev);
@@ -2577,6 +2587,16 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream)
 	int ret;
 	int stream = fe_substream->stream;
 
+	if (fe->card->dirty) {
+		/*
+		 * In case of unbind, each CPU/Codec/Platform component can
+		 * be unbinded randomly. In such case (= card->dirty)
+		 * Sound Card is no longer safety. Don't open it.
+		 */
+		dev_warn(fe->dev, "Sound SoC Card is dirty, Recheck your system\n");
+		return -ENXIO;
+	}
+
 	mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
 	fe->dpcm[stream].runtime = fe_substream->runtime;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-04-05 17:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-03  6:31 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
2017-04-03  8:28 ` Kuninori Morimoto
2017-04-05 17:30 ` Applied "ASoC: soc-core: verify Sound Card normality" to the asoc tree Mark Brown
2017-04-05 17:30   ` Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2017-03-29  2:45 [RFC][PATCH] ASoC: soc-core: verify Sound Card normality Kuninori Morimoto
2017-03-29  7:29 ` Geert Uytterhoeven
2017-03-29  7:29   ` Geert Uytterhoeven
2017-03-29  8:47   ` Kuninori Morimoto
2017-03-30 21:53 ` Mark Brown
2017-03-31  0:30   ` Kuninori Morimoto
2017-03-31  7:48   ` [alsa-devel] " Takashi Iwai
2017-04-03  6:29     ` Kuninori Morimoto
2017-04-03  6:41       ` Takashi Iwai
2017-04-03  8:26         ` Kuninori Morimoto
2017-04-03  8:37           ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.