| 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
| ||
|
Message-Id: <20230317195128.3911155-1-john@metanate.com>
Date: Fri, 17 Mar 2023 19:51:27 +0000
From: John Keeping <john@...anate.com>
To: Takashi Iwai <tiwai@...e.com>
Cc: John Keeping <john@...anate.com>, Jaroslav Kysela <perex@...ex.cz>,
alsa-devel@...a-project.org (moderated list:SOUND),
linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] ALSA: usb-audio: Fix recursive locking on XRUN
snd_usb_queue_pending_output_urbs() may be called from
snd_pcm_ops::ack() which means the PCM stream is locked.
For the normal case where the call back into the PCM core is via
prepare_output_urb() the "_under_stream_lock" variant of
snd_pcm_period_elapsed() is called, but when an error occurs and the
stream is stopped as XRUN then snd_pcm_xrun() tries to recursively lock
the stream which results in deadlock.
Follow the example of snd_pcm_period_elapsed() by adding
snd_pcm_xrun_under_stream_lock() and use this when the PCM substream
lock is already held.
Signed-off-by: John Keeping <john@...anate.com>
---
include/sound/pcm.h | 1 +
sound/core/pcm_native.c | 28 ++++++++++++++++++++++++----
sound/usb/endpoint.c | 18 +++++++++++-------
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 27040b472a4f..98551907453a 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -571,6 +571,7 @@ int snd_pcm_status64(struct snd_pcm_substream *substream,
int snd_pcm_start(struct snd_pcm_substream *substream);
int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
int snd_pcm_drain_done(struct snd_pcm_substream *substream);
+int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream);
int snd_pcm_stop_xrun(struct snd_pcm_substream *substream);
#ifdef CONFIG_PM
int snd_pcm_suspend_all(struct snd_pcm *pcm);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 331380c2438b..617f5dc74df0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1559,24 +1559,44 @@ int snd_pcm_drain_done(struct snd_pcm_substream *substream)
SNDRV_PCM_STATE_SETUP);
}
+/**
+ * snd_pcm_stop_xrun_under_stream_lock - stop the running stream as XRUN under the lock of
+ * the PCM substream.
+ * @substream: the PCM substream instance
+ *
+ * This stops the given running substream (and all linked substreams) as XRUN.
+ * This function assumes that the substream lock is already held.
+ *
+ * Return: Zero if successful, or a negative error core.
+ */
+int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream)
+{
+ if (substream->runtime && snd_pcm_running(substream))
+ __snd_pcm_xrun(substream);
+
+ return 0;
+}
+
/**
* snd_pcm_stop_xrun - stop the running streams as XRUN
* @substream: the PCM substream instance
*
+ * This function is similar to ``snd_pcm_stop_xrun_under_stream_lock()`` except that it
+ * acquires the substream lock itself.
+ *
* This stops the given running substream (and all linked substreams) as XRUN.
- * Unlike snd_pcm_stop(), this function takes the substream lock by itself.
*
* Return: Zero if successful, or a negative error code.
*/
int snd_pcm_stop_xrun(struct snd_pcm_substream *substream)
{
unsigned long flags;
+ int ret;
snd_pcm_stream_lock_irqsave(substream, flags);
- if (substream->runtime && snd_pcm_running(substream))
- __snd_pcm_xrun(substream);
+ ret = snd_pcm_stop_xrun_under_stream_lock(substream);
snd_pcm_stream_unlock_irqrestore(substream, flags);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(snd_pcm_stop_xrun);
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 1e0af1179ca8..83a6b6d41374 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -400,13 +400,17 @@ static int prepare_inbound_urb(struct snd_usb_endpoint *ep,
}
/* notify an error as XRUN to the assigned PCM data substream */
-static void notify_xrun(struct snd_usb_endpoint *ep)
+static void notify_xrun(struct snd_usb_endpoint *ep, bool in_stream_lock)
{
struct snd_usb_substream *data_subs;
data_subs = READ_ONCE(ep->data_subs);
- if (data_subs && data_subs->pcm_substream)
- snd_pcm_stop_xrun(data_subs->pcm_substream);
+ if (data_subs && data_subs->pcm_substream) {
+ if (in_stream_lock)
+ snd_pcm_stop_xrun_under_stream_lock(data_subs->pcm_substream);
+ else
+ snd_pcm_stop_xrun(data_subs->pcm_substream);
+ }
}
static struct snd_usb_packet_info *
@@ -498,7 +502,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
if (err == -EAGAIN)
push_back_to_ready_list(ep, ctx);
else
- notify_xrun(ep);
+ notify_xrun(ep, in_stream_lock);
return;
}
@@ -507,7 +511,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
usb_audio_err(ep->chip,
"Unable to submit urb #%d: %d at %s\n",
ctx->index, err, __func__);
- notify_xrun(ep);
+ notify_xrun(ep, in_stream_lock);
return;
}
@@ -574,7 +578,7 @@ static void snd_complete_urb(struct urb *urb)
return;
usb_audio_err(ep->chip, "cannot submit urb (err = %d)\n", err);
- notify_xrun(ep);
+ notify_xrun(ep, false);
exit_clear:
clear_bit(ctx->index, &ep->active_mask);
@@ -1762,7 +1766,7 @@ static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
usb_audio_err(ep->chip,
"next package FIFO overflow EP 0x%x\n",
ep->ep_num);
- notify_xrun(ep);
+ notify_xrun(ep, false);
return;
}
--
2.40.0
Powered by blists - more mailing lists