[PATCH 0/2] ALSA: dice: fix a fatal bug when no sound card is registered

[PATCH 0/2] ALSA: dice: fix a fatal bug when no sound card is registered

[Takashi Sakamoto]

Hi,

This patchset is for Linux 4.6-rc2.

ALSA dice driver causes kernel NULL pointer dereference in a case that
no sound card instance is registered and unit is going to be removed.
This is observed when dice-based units are connected to Linux system and
disconnected within 2 seconds, thus it's rare.

Additionally, even if the bug is fixed, the driver dumps warnings for
most dice-based models which supports a pair of isochronous streams.
This can occur regardless of the former bug. Users can see this for
unsupported dice-based models.

This patchset fixes these bugs.

Takashi Sakamoto (2):
  ALSA: dice: fix NULL pointer dereference at remove units when sound
    card is not registered
  ALSA: firewire-lib: skip releasing stream data when it's not
    initialized

 sound/firewire/amdtp-stream.c     |  4 ++++
 sound/firewire/dice/dice-stream.c | 14 ++++----------
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.7.3

[PATCH 1/2] ALSA: dice: fix NULL pointer dereference at remove units when sound card is not registered

[Takashi Sakamoto]

When sound card is not registered, in a callback of unit removal, ALSA dice
driver calls amdtp_stream_stop(). This causes a NULL pointer dereference in
__mutex_lock_slowpath() because of uninitialized mutex data.

When private data is going to be released, no userspace applications refer
to AMDTP stream data anymore. Thus no need to stop the streams. This
commit just destroys the stream data to fix the bug.

Fixes: 4bdc495c87b3('ALSA: dice: handle several PCM substreams when any isochronous streams are available')
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 845d5e5..ec4db3a 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -446,18 +446,12 @@ end:
 
 void snd_dice_stream_destroy_duplex(struct snd_dice *dice)
 {
-	struct reg_params tx_params, rx_params;
-
-	snd_dice_transaction_clear_enable(dice);
+	unsigned int i;
 
-	if (get_register_params(dice, &tx_params, &rx_params) == 0) {
-		stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-		stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
+	for (i = 0; i < MAX_STREAMS; i++) {
+		destroy_stream(dice, AMDTP_IN_STREAM, i);
+		destroy_stream(dice, AMDTP_OUT_STREAM, i);
 	}
-
-	release_resources(dice);
-
-	dice->substreams_counter = 0;
 }
 
 void snd_dice_stream_update_duplex(struct snd_dice *dice)
-- 
2.7.3

[PATCH 2/2] ALSA: firewire-lib: skip releasing stream data when it's not initialized

[Takashi Sakamoto]

When any of AMDTP stream data are not initialized and private data is
going to be released, WARN_ON() in amdtp_stream_destroy() is hit and
dump messages. This may take users irritated.

This commit fixes the bug to skip releasing stream data when the data is
not initialized.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index ed29026..4484242 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -102,6 +102,10 @@ EXPORT_SYMBOL(amdtp_stream_init);
  */
 void amdtp_stream_destroy(struct amdtp_stream *s)
 {
+	/* Not initialized. */
+	if (s->protocol == NULL)
+		return;
+
 	WARN_ON(amdtp_stream_running(s));
 	kfree(s->protocol);
 	mutex_destroy(&s->mutex);
-- 
2.7.3

Re: [PATCH 0/2] ALSA: dice: fix a fatal bug when no sound card is registered

[Takashi Sakamoto]

Hi,

On Mar 26 2016 21:05, Takashi Sakamoto wrote:
> Hi,
> 
> This patchset is for Linux 4.6-rc2.
> 
> ALSA dice driver causes kernel NULL pointer dereference in a case that
> no sound card instance is registered and unit is going to be removed.
> This is observed when dice-based units are connected to Linux system and
> disconnected within 2 seconds, thus it's rare.
> 
> Additionally, even if the bug is fixed, the driver dumps warnings for
> most dice-based models which supports a pair of isochronous streams.
> This can occur regardless of the former bug. Users can see this for
> unsupported dice-based models.
> 
> This patchset fixes these bugs.

I realized that this patchset is not proper to Linux 4.6-rc2. The stream
data is confirmed to initialized in unit probe callback, and safely
released when releasing private data. I worked on the other branch and
had a confusion. So please drop this.

However, current ALSA dice driver has a bug not to release stream data
when releasing private data. This causes memory leak and should be fixed
in 4.6-rc2. I'll post a patch for the bug soon.

(I'd like to get the reason I often realize such mistakes just after
getting up in the morning, the beginning of a day, sign...)


Regards

Takashi Sakamoto

> Takashi Sakamoto (2):
>   ALSA: dice: fix NULL pointer dereference at remove units when sound
>     card is not registered
>   ALSA: firewire-lib: skip releasing stream data when it's not
>     initialized
> 
>  sound/firewire/amdtp-stream.c     |  4 ++++
>  sound/firewire/dice/dice-stream.c | 14 ++++----------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 

Re: [PATCH 0/2] ALSA: dice: fix a fatal bug when no sound card is registered

[Takashi Iwai]

On Mon, 28 Mar 2016 01:14:51 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Mar 26 2016 21:05, Takashi Sakamoto wrote:
> > Hi,
> > 
> > This patchset is for Linux 4.6-rc2.
> > 
> > ALSA dice driver causes kernel NULL pointer dereference in a case that
> > no sound card instance is registered and unit is going to be removed.
> > This is observed when dice-based units are connected to Linux system and
> > disconnected within 2 seconds, thus it's rare.
> > 
> > Additionally, even if the bug is fixed, the driver dumps warnings for
> > most dice-based models which supports a pair of isochronous streams.
> > This can occur regardless of the former bug. Users can see this for
> > unsupported dice-based models.
> > 
> > This patchset fixes these bugs.
> 
> I realized that this patchset is not proper to Linux 4.6-rc2. The stream
> data is confirmed to initialized in unit probe callback, and safely
> released when releasing private data. I worked on the other branch and
> had a confusion. So please drop this.
> 
> However, current ALSA dice driver has a bug not to release stream data
> when releasing private data. This causes memory leak and should be fixed
> in 4.6-rc2. I'll post a patch for the bug soon.

So, I don't need to pick up this patchset after applying your fix
([PATCH v2] ALSA: dice: fix memory leak when unplugging)?


thanks,

Takashi

Re: [PATCH 0/2] ALSA: dice: fix a fatal bug when no sound card is registered

[Takashi Sakamoto]

Hi,

On 2016年03月28日 16:36, Takashi Iwai wrote:
> On Mon, 28 Mar 2016 01:14:51 +0200,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> On Mar 26 2016 21:05, Takashi Sakamoto wrote:
>>> Hi,
>>>
>>> This patchset is for Linux 4.6-rc2.
>>>
>>> ALSA dice driver causes kernel NULL pointer dereference in a case that
>>> no sound card instance is registered and unit is going to be removed.
>>> This is observed when dice-based units are connected to Linux system and
>>> disconnected within 2 seconds, thus it's rare.
>>>
>>> Additionally, even if the bug is fixed, the driver dumps warnings for
>>> most dice-based models which supports a pair of isochronous streams.
>>> This can occur regardless of the former bug. Users can see this for
>>> unsupported dice-based models.
>>>
>>> This patchset fixes these bugs.
>>
>> I realized that this patchset is not proper to Linux 4.6-rc2. The stream
>> data is confirmed to initialized in unit probe callback, and safely
>> released when releasing private data. I worked on the other branch and
>> had a confusion. So please drop this.
>>
>> However, current ALSA dice driver has a bug not to release stream data
>> when releasing private data. This causes memory leak and should be fixed
>> in 4.6-rc2. I'll post a patch for the bug soon.
>
> So, I don't need to pick up this patchset after applying your fix
> ([PATCH v2] ALSA: dice: fix memory leak when unplugging)?

Exactly. Sorry to trouble you.

> thanks,
>
> Takashi


Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel