[PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[Takashi Sakamoto]

Each FireWire sound driver causes kernel oops at unplugging during
streaming. This bug affects:
 - snd-bebob (in 3.16 or later)
 - snd-fireworks (in 3.16 or later)
 - snd-dice (in 3.19 or later)
 - snd-oxfw (in 3.19 or later)

When streaming, any ALSA character devices are opened and sound card
object is referred. Then 'struct snd_card.private_free()' is not executed
in a context of 'struct fw_driver.remove()'. It's executed in a context of
an application process in which the last ALSA character device is closed.
In this case, any operation to instances of FireWire unit is invalid
because it's already released, and causes kernel oops. I confirm GPF or
Null-pointer-dereference.

To fix this problem, this patchset do:
1.delegate reference counting of an instance of FireWire unit from AMDTP
  helper functions to each driver, then do it in
  'struct snd_card.private_free()'.
2.move stream or transaction destructor from 'struct fw_driver.remove()'
  to 'struct snd_card.private_free()'.

With this patchset, each driver keeps a reference to FireWire unit till
all of ALSA applications close its character devices. This may be against
IEEE 1394 bus driver developers' expectation to high-level drivers.

Takashi Sakamoto (4):
  ALSA: fireworks/bebob/dice/oxfw: add reference-counting for FireWire
    unit
  ALSA: firewire-lib: remove reference counting
  ALSA: fireworks/bebob/dice/oxfw: allow stream destructor after
    releasing runtime
  ALSA: fireworks/bebob/dice/oxfw: make it possible to shutdown safely

 sound/firewire/amdtp.c                      |  3 +--
 sound/firewire/bebob/bebob.c                | 20 ++++++++++++++++----
 sound/firewire/bebob/bebob_stream.c         | 16 ++++------------
 sound/firewire/dice/dice-stream.c           | 18 ++++++++++++------
 sound/firewire/dice/dice.c                  | 16 +++++++++++-----
 sound/firewire/fireworks/fireworks.c        | 20 ++++++++++++++------
 sound/firewire/fireworks/fireworks_stream.c | 19 ++++++++++---------
 sound/firewire/oxfw/oxfw-stream.c           |  6 ++++--
 sound/firewire/oxfw/oxfw.c                  | 21 ++++++++++++++-------
 9 files changed, 86 insertions(+), 53 deletions(-)

-- 
2.1.0


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk

[PATCH 1/4] ALSA: fireworks/bebob/dice/oxfw: add reference-counting for FireWire unit

[Takashi Sakamoto]

Fireworks and Dice drivers try to touch instances of FireWire unit after
sound card object is released, while references to the unit is decremented
in .remove(). When unplugging during streaming, sound card object is
released after .remove(), thus Fireworks and Dice drivers causes GPF or
Null-pointer-dereferencing to application processes because an instance of
FireWire unit was already released.

This commit adds reference-counting for FireWire unit in drivers to allow
them to touch an instance of FireWire unit after .remove(). In most case,
any operations after .remove() may be failed safely.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c         | 12 +++++++++++-
 sound/firewire/dice/dice.c           | 11 ++++++++++-
 sound/firewire/fireworks/fireworks.c | 12 +++++++++++-
 sound/firewire/oxfw/oxfw.c           | 11 ++++++++++-
 4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index fc19c99..b612599 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -116,11 +116,19 @@ end:
 	return err;
 }
 
+/*
+ * This module releases the FireWire unit data after all ALSA character devices
+ * are released by applications. This is for releasing stream data or finishing
+ * transactions safely. Thus at returning from .remove(), this module still keep
+ * references for the unit.
+ */
 static void
 bebob_card_free(struct snd_card *card)
 {
 	struct snd_bebob *bebob = card->private_data;
 
+	fw_unit_put(bebob->unit);
+
 	if (bebob->card_index >= 0) {
 		mutex_lock(&devices_mutex);
 		clear_bit(bebob->card_index, devices_used);
@@ -205,7 +213,7 @@ bebob_probe(struct fw_unit *unit,
 	card->private_free = bebob_card_free;
 
 	bebob->card = card;
-	bebob->unit = unit;
+	bebob->unit = fw_unit_get(unit);
 	bebob->spec = spec;
 	mutex_init(&bebob->mutex);
 	spin_lock_init(&bebob->lock);
@@ -310,6 +318,8 @@ static void bebob_remove(struct fw_unit *unit)
 
 	snd_bebob_stream_destroy_duplex(bebob);
 	snd_card_disconnect(bebob->card);
+
+	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(bebob->card);
 }
 
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 90d8f40..797f072 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -226,11 +226,19 @@ static void dice_card_strings(struct snd_dice *dice)
 	strcpy(card->mixername, "DICE");
 }
 
+/*
+ * This module releases the FireWire unit data after all ALSA character devices
+ * are released by applications. This is for releasing stream data or finishing
+ * transactions safely. Thus at returning from .remove(), this module still keep
+ * references for the unit.
+ */
 static void dice_card_free(struct snd_card *card)
 {
 	struct snd_dice *dice = card->private_data;
 
 	snd_dice_transaction_destroy(dice);
+	fw_unit_put(dice->unit);
+
 	mutex_destroy(&dice->mutex);
 }
 
@@ -251,7 +259,7 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 
 	dice = card->private_data;
 	dice->card = card;
-	dice->unit = unit;
+	dice->unit = fw_unit_get(unit);
 	card->private_free = dice_card_free;
 
 	spin_lock_init(&dice->lock);
@@ -309,6 +317,7 @@ static void dice_remove(struct fw_unit *unit)
 
 	snd_dice_stream_destroy_duplex(dice);
 
+	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(dice->card);
 }
 
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index 3e2ed8e..1e33394 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -173,11 +173,19 @@ end:
 	return err;
 }
 
+/*
+ * This module releases the FireWire unit data after all ALSA character devices
+ * are released by applications. This is for releasing stream data or finishing
+ * transactions safely. Thus at returning from .remove(), this module still keep
+ * references for the unit.
+ */
 static void
 efw_card_free(struct snd_card *card)
 {
 	struct snd_efw *efw = card->private_data;
 
+	fw_unit_put(efw->unit);
+
 	if (efw->card_index >= 0) {
 		mutex_lock(&devices_mutex);
 		clear_bit(efw->card_index, devices_used);
@@ -218,7 +226,7 @@ efw_probe(struct fw_unit *unit,
 	card->private_free = efw_card_free;
 
 	efw->card = card;
-	efw->unit = unit;
+	efw->unit = fw_unit_get(unit);
 	mutex_init(&efw->mutex);
 	spin_lock_init(&efw->lock);
 	init_waitqueue_head(&efw->hwdep_wait);
@@ -293,6 +301,8 @@ static void efw_remove(struct fw_unit *unit)
 	snd_efw_transaction_remove_instance(efw);
 
 	snd_card_disconnect(efw->card);
+
+	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(efw->card);
 }
 
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 60e5cad..1607b26 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -104,11 +104,19 @@ end:
 	return err;
 }
 
+/*
+ * This module releases the FireWire unit data after all ALSA character devices
+ * are released by applications. This is for releasing stream data or finishing
+ * transactions safely. Thus at returning from .remove(), this module still keep
+ * references for the unit.
+ */
 static void oxfw_card_free(struct snd_card *card)
 {
 	struct snd_oxfw *oxfw = card->private_data;
 	unsigned int i;
 
+	fw_unit_put(oxfw->unit);
+
 	for (i = 0; i < SND_OXFW_STREAM_FORMAT_ENTRIES; i++) {
 		kfree(oxfw->tx_stream_formats[i]);
 		kfree(oxfw->rx_stream_formats[i]);
@@ -136,7 +144,7 @@ static int oxfw_probe(struct fw_unit *unit,
 	oxfw = card->private_data;
 	oxfw->card = card;
 	mutex_init(&oxfw->mutex);
-	oxfw->unit = unit;
+	oxfw->unit = fw_unit_get(unit);
 	oxfw->device_info = (const struct device_info *)id->driver_data;
 	spin_lock_init(&oxfw->lock);
 	init_waitqueue_head(&oxfw->hwdep_wait);
@@ -218,6 +226,7 @@ static void oxfw_remove(struct fw_unit *unit)
 	if (oxfw->has_output)
 		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
 
+	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(oxfw->card);
 }
 
-- 
2.1.0


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk

[PATCH 2/4] ALSA: firewire-lib: remove reference counting

[Takashi Sakamoto]

AMDTP helper functions increment/decrement reference counter for an
instance of FireWire unit, while it's complicated for each driver to
process error state.

In previous commit, each driver has the role of reference counting. This
commit removes this role from the helper function.

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

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 0d58018..911341b 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -78,7 +78,7 @@ static void pcm_period_tasklet(unsigned long data);
 int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
 		      enum amdtp_stream_direction dir, enum cip_flags flags)
 {
-	s->unit = fw_unit_get(unit);
+	s->unit = unit;
 	s->direction = dir;
 	s->flags = flags;
 	s->context = ERR_PTR(-1);
@@ -102,7 +102,6 @@ void amdtp_stream_destroy(struct amdtp_stream *s)
 {
 	WARN_ON(amdtp_stream_running(s));
 	mutex_destroy(&s->mutex);
-	fw_unit_put(s->unit);
 }
 EXPORT_SYMBOL(amdtp_stream_destroy);
 
-- 
2.1.0


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk

[PATCH 3/4] ALSA: fireworks/bebob/dice/oxfw: allow stream destructor after releasing runtime

[Takashi Sakamoto]

Currently stream destructor in each driver has a problem to be called in
a context in which sound card object is released, because the destructors
call amdtp_stream_pcm_abort() and touch PCM runtime data.

The PCM runtime data is destroyed in application's context with
snd_pcm_close(), on the other hand PCM substream data is destroyed after
sound card object is released, in most case after all of ALSA character
devices are released. When PCM runtime is destroyed and PCM substream is
remained, amdtp_stream_pcm_abort() touches PCM runtime data and causes
Null-pointer-dereference.

This commit changes stream destructors and allows each driver to call
it after releasing runtime.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob_stream.c         | 12 ++++--------
 sound/firewire/dice/dice-stream.c           | 18 ++++++++++++------
 sound/firewire/fireworks/fireworks_stream.c | 15 ++++++++++-----
 sound/firewire/oxfw/oxfw-stream.c           |  6 ++++--
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 0ebcabf..fcca3ee 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -410,8 +410,6 @@ break_both_connections(struct snd_bebob *bebob)
 static void
 destroy_both_connections(struct snd_bebob *bebob)
 {
-	break_both_connections(bebob);
-
 	cmp_connection_destroy(&bebob->in_conn);
 	cmp_connection_destroy(&bebob->out_conn);
 }
@@ -712,16 +710,14 @@ void snd_bebob_stream_update_duplex(struct snd_bebob *bebob)
 	mutex_unlock(&bebob->mutex);
 }
 
+/*
+ * This function should be called before starting streams or after stopping
+ * streams.
+ */
 void snd_bebob_stream_destroy_duplex(struct snd_bebob *bebob)
 {
 	mutex_lock(&bebob->mutex);
 
-	amdtp_stream_pcm_abort(&bebob->rx_stream);
-	amdtp_stream_pcm_abort(&bebob->tx_stream);
-
-	amdtp_stream_stop(&bebob->rx_stream);
-	amdtp_stream_stop(&bebob->tx_stream);
-
 	amdtp_stream_destroy(&bebob->rx_stream);
 	amdtp_stream_destroy(&bebob->tx_stream);
 
diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index fa9cf76..07dbd01 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -311,14 +311,21 @@ end:
 	return err;
 }
 
+/*
+ * This function should be called before starting streams or after stopping
+ * streams.
+ */
 static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 {
-	amdtp_stream_destroy(stream);
+	struct fw_iso_resources *resources;
 
 	if (stream == &dice->tx_stream)
-		fw_iso_resources_destroy(&dice->tx_resources);
+		resources = &dice->tx_resources;
 	else
-		fw_iso_resources_destroy(&dice->rx_resources);
+		resources = &dice->rx_resources;
+
+	amdtp_stream_destroy(stream);
+	fw_iso_resources_destroy(resources);
 }
 
 int snd_dice_stream_init_duplex(struct snd_dice *dice)
@@ -332,6 +339,8 @@ int snd_dice_stream_init_duplex(struct snd_dice *dice)
 		goto end;
 
 	err = init_stream(dice, &dice->rx_stream);
+	if (err < 0)
+		destroy_stream(dice, &dice->tx_stream);
 end:
 	return err;
 }
@@ -340,10 +349,7 @@ void snd_dice_stream_destroy_duplex(struct snd_dice *dice)
 {
 	snd_dice_transaction_clear_enable(dice);
 
-	stop_stream(dice, &dice->tx_stream);
 	destroy_stream(dice, &dice->tx_stream);
-
-	stop_stream(dice, &dice->rx_stream);
 	destroy_stream(dice, &dice->rx_stream);
 
 	dice->substreams_counter = 0;
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
index 4f440e1..f817b7a 100644
--- a/sound/firewire/fireworks/fireworks_stream.c
+++ b/sound/firewire/fireworks/fireworks_stream.c
@@ -100,17 +100,22 @@ end:
 	return err;
 }
 
+/*
+ * This function should be called before starting the stream or after stopping
+ * the streams.
+ */
 static void
 destroy_stream(struct snd_efw *efw, struct amdtp_stream *stream)
 {
-	stop_stream(efw, stream);
-
-	amdtp_stream_destroy(stream);
+	struct cmp_connection *conn;
 
 	if (stream == &efw->tx_stream)
-		cmp_connection_destroy(&efw->out_conn);
+		conn = &efw->out_conn;
 	else
-		cmp_connection_destroy(&efw->in_conn);
+		conn = &efw->in_conn;
+
+	amdtp_stream_destroy(stream);
+	cmp_connection_destroy(&efw->out_conn);
 }
 
 static int
diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c
index bda845a..29ccb36 100644
--- a/sound/firewire/oxfw/oxfw-stream.c
+++ b/sound/firewire/oxfw/oxfw-stream.c
@@ -337,6 +337,10 @@ void snd_oxfw_stream_stop_simplex(struct snd_oxfw *oxfw,
 	stop_stream(oxfw, stream);
 }
 
+/*
+ * This function should be called before starting the stream or after stopping
+ * the streams.
+ */
 void snd_oxfw_stream_destroy_simplex(struct snd_oxfw *oxfw,
 				     struct amdtp_stream *stream)
 {
@@ -347,8 +351,6 @@ void snd_oxfw_stream_destroy_simplex(struct snd_oxfw *oxfw,
 	else
 		conn = &oxfw->in_conn;
 
-	stop_stream(oxfw, stream);
-
 	amdtp_stream_destroy(stream);
 	cmp_connection_destroy(conn);
 }
-- 
2.1.0


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk

[PATCH 4/4] ALSA: fireworks/bebob/dice/oxfw: make it possible to shutdown safely

[Takashi Sakamoto]

A part of these drivers, especially BeBoB driver, are programmed to wait
some events. Thus the drivers should not destroy any data in .remove()
context.

This commit moves some destructors from 'struct fw_driver.remove()' to
'struct snd_card.private_free()' to shutdown safely.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c                | 10 ++++++----
 sound/firewire/bebob/bebob_stream.c         |  4 ----
 sound/firewire/dice/dice.c                  |  5 +----
 sound/firewire/fireworks/fireworks.c        | 10 ++++------
 sound/firewire/fireworks/fireworks_stream.c |  4 ----
 sound/firewire/oxfw/oxfw.c                  | 10 ++++------
 6 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index b612599..611b7da 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -127,8 +127,11 @@ bebob_card_free(struct snd_card *card)
 {
 	struct snd_bebob *bebob = card->private_data;
 
+	snd_bebob_stream_destroy_duplex(bebob);
 	fw_unit_put(bebob->unit);
 
+	kfree(bebob->maudio_special_quirk);
+
 	if (bebob->card_index >= 0) {
 		mutex_lock(&devices_mutex);
 		clear_bit(bebob->card_index, devices_used);
@@ -314,10 +317,9 @@ static void bebob_remove(struct fw_unit *unit)
 	if (bebob == NULL)
 		return;
 
-	kfree(bebob->maudio_special_quirk);
-
-	snd_bebob_stream_destroy_duplex(bebob);
-	snd_card_disconnect(bebob->card);
+	/* Awake bus-reset waiters. */
+	if (!completion_done(&bebob->bus_reset))
+		complete_all(&bebob->bus_reset);
 
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(bebob->card);
diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index fcca3ee..98e4fc8 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -716,14 +716,10 @@ void snd_bebob_stream_update_duplex(struct snd_bebob *bebob)
  */
 void snd_bebob_stream_destroy_duplex(struct snd_bebob *bebob)
 {
-	mutex_lock(&bebob->mutex);
-
 	amdtp_stream_destroy(&bebob->rx_stream);
 	amdtp_stream_destroy(&bebob->tx_stream);
 
 	destroy_both_connections(bebob);
-
-	mutex_unlock(&bebob->mutex);
 }
 
 /*
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 797f072..70a111d 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -236,6 +236,7 @@ static void dice_card_free(struct snd_card *card)
 {
 	struct snd_dice *dice = card->private_data;
 
+	snd_dice_stream_destroy_duplex(dice);
 	snd_dice_transaction_destroy(dice);
 	fw_unit_put(dice->unit);
 
@@ -313,10 +314,6 @@ static void dice_remove(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
-	snd_card_disconnect(dice->card);
-
-	snd_dice_stream_destroy_duplex(dice);
-
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(dice->card);
 }
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index 1e33394..2682e7e 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -184,8 +184,12 @@ efw_card_free(struct snd_card *card)
 {
 	struct snd_efw *efw = card->private_data;
 
+	snd_efw_stream_destroy_duplex(efw);
+	snd_efw_transaction_remove_instance(efw);
 	fw_unit_put(efw->unit);
 
+	kfree(efw->resp_buf);
+
 	if (efw->card_index >= 0) {
 		mutex_lock(&devices_mutex);
 		clear_bit(efw->card_index, devices_used);
@@ -193,7 +197,6 @@ efw_card_free(struct snd_card *card)
 	}
 
 	mutex_destroy(&efw->mutex);
-	kfree(efw->resp_buf);
 }
 
 static int
@@ -297,11 +300,6 @@ static void efw_remove(struct fw_unit *unit)
 {
 	struct snd_efw *efw = dev_get_drvdata(&unit->device);
 
-	snd_efw_stream_destroy_duplex(efw);
-	snd_efw_transaction_remove_instance(efw);
-
-	snd_card_disconnect(efw->card);
-
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(efw->card);
 }
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
index f817b7a..c55db1b 100644
--- a/sound/firewire/fireworks/fireworks_stream.c
+++ b/sound/firewire/fireworks/fireworks_stream.c
@@ -324,12 +324,8 @@ void snd_efw_stream_update_duplex(struct snd_efw *efw)
 
 void snd_efw_stream_destroy_duplex(struct snd_efw *efw)
 {
-	mutex_lock(&efw->mutex);
-
 	destroy_stream(efw, &efw->rx_stream);
 	destroy_stream(efw, &efw->tx_stream);
-
-	mutex_unlock(&efw->mutex);
 }
 
 void snd_efw_stream_lock_changed(struct snd_efw *efw)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 1607b26..8c6ce01 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -115,6 +115,10 @@ static void oxfw_card_free(struct snd_card *card)
 	struct snd_oxfw *oxfw = card->private_data;
 	unsigned int i;
 
+	snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream);
+	if (oxfw->has_output)
+		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
+
 	fw_unit_put(oxfw->unit);
 
 	for (i = 0; i < SND_OXFW_STREAM_FORMAT_ENTRIES; i++) {
@@ -220,12 +224,6 @@ static void oxfw_remove(struct fw_unit *unit)
 {
 	struct snd_oxfw *oxfw = dev_get_drvdata(&unit->device);
 
-	snd_card_disconnect(oxfw->card);
-
-	snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream);
-	if (oxfw->has_output)
-		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
-
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(oxfw->card);
 }
-- 
2.1.0


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk

Re: [PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[Clemens Ladisch]

Takashi Sakamoto wrote:
> With this patchset, each driver keeps a reference to FireWire unit till
> all of ALSA applications close its character devices. This may be against
> IEEE 1394 bus driver developers' expectation to high-level drivers.

The fw_unit reference counting was designed for situations like this.


Regards,
Clemens

Re: [PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[Takashi Iwai]

At Sun, 22 Feb 2015 00:06:32 +0100,
Clemens Ladisch wrote:
> 
> Takashi Sakamoto wrote:
> > With this patchset, each driver keeps a reference to FireWire unit till
> > all of ALSA applications close its character devices. This may be against
> > IEEE 1394 bus driver developers' expectation to high-level drivers.
> 
> The fw_unit reference counting was designed for situations like this.

"This" means which?


Takashi

Re: [PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[Clemens Ladisch]

Takashi Iwai wrote:
> Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> With this patchset, each driver keeps a reference to FireWire unit till
>>> all of ALSA applications close its character devices. This may be against
>>> IEEE 1394 bus driver developers' expectation to high-level drivers.
>>
>> The fw_unit reference counting was designed for situations like this.
>
> "This" means which?

That the driver keeps a reference to the fw_unit even after the device
has been unplugged.


Regards,
Clemens

Re: [PATCH 0/4] ALSA: fireworks/bebob/dice/oxfw: fix kernel oops at unplugging during streaming

[Takashi Iwai]

At Sat, 21 Feb 2015 23:54:56 +0900,
Takashi Sakamoto wrote:
> 
> Each FireWire sound driver causes kernel oops at unplugging during
> streaming. This bug affects:
>  - snd-bebob (in 3.16 or later)
>  - snd-fireworks (in 3.16 or later)
>  - snd-dice (in 3.19 or later)
>  - snd-oxfw (in 3.19 or later)
> 
> When streaming, any ALSA character devices are opened and sound card
> object is referred. Then 'struct snd_card.private_free()' is not executed
> in a context of 'struct fw_driver.remove()'. It's executed in a context of
> an application process in which the last ALSA character device is closed.
> In this case, any operation to instances of FireWire unit is invalid
> because it's already released, and causes kernel oops. I confirm GPF or
> Null-pointer-dereference.
> 
> To fix this problem, this patchset do:
> 1.delegate reference counting of an instance of FireWire unit from AMDTP
>   helper functions to each driver, then do it in
>   'struct snd_card.private_free()'.
> 2.move stream or transaction destructor from 'struct fw_driver.remove()'
>   to 'struct snd_card.private_free()'.
> 
> With this patchset, each driver keeps a reference to FireWire unit till
> all of ALSA applications close its character devices. This may be against
> IEEE 1394 bus driver developers' expectation to high-level drivers.

Thanks, now applied all four patches.


Takashi

> 
> Takashi Sakamoto (4):
>   ALSA: fireworks/bebob/dice/oxfw: add reference-counting for FireWire
>     unit
>   ALSA: firewire-lib: remove reference counting
>   ALSA: fireworks/bebob/dice/oxfw: allow stream destructor after
>     releasing runtime
>   ALSA: fireworks/bebob/dice/oxfw: make it possible to shutdown safely
> 
>  sound/firewire/amdtp.c                      |  3 +--
>  sound/firewire/bebob/bebob.c                | 20 ++++++++++++++++----
>  sound/firewire/bebob/bebob_stream.c         | 16 ++++------------
>  sound/firewire/dice/dice-stream.c           | 18 ++++++++++++------
>  sound/firewire/dice/dice.c                  | 16 +++++++++++-----
>  sound/firewire/fireworks/fireworks.c        | 20 ++++++++++++++------
>  sound/firewire/fireworks/fireworks_stream.c | 19 ++++++++++---------
>  sound/firewire/oxfw/oxfw-stream.c           |  6 ++++--
>  sound/firewire/oxfw/oxfw.c                  | 21 ++++++++++++++-------
>  9 files changed, 86 insertions(+), 53 deletions(-)
> 
> -- 
> 2.1.0
>