* [PATCH 0/2] audio: prevent a class of guest-triggered aborts
@ 2022-09-17 13:15 Volker Rümelin
2022-09-17 13:16 ` [PATCH 1/2] Revert "audio: Log context for audio bug" Volker Rümelin
2022-09-17 13:16 ` [PATCH 2/2] audio: remove abort() in audio_bug() Volker Rümelin
0 siblings, 2 replies; 3+ messages in thread
From: Volker Rümelin @ 2022-09-17 13:15 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Philippe Mathieu-Daudé
The issues with guest-triggered aborts started with commit ab32b78cd1
"audio: Simplify audio_bug() removing old code" which introduced an
abort() in function audio_bug(). The abort() was there before, but it
was only compiled in for debugging purposes.
After this commit issue https://bugs.launchpad.net/bugs/1910603 showed
up. This bug was mitigated with commits a2cd86a94a ("hw/audio/sb16:
Avoid assertion by restricting I/O sampling rate range") and 60e543f5ce
("hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h").
The issue was only mitigated because I can still trigger the same abort.
To reproduce start a FreeDOS QEMU guest with:
./qemu-system-i386 -machine pc,pcspk-audiodev=audio0 \
-device sb16,audiodev=audio0 \
-audiodev
pa,id=audio0,timer-period=170,out.mixing-engine=on,out.buffer-length=181 \
-drive ...
On the guest enter the out port sequence from launchpad bug #1910603:
C:\> debug
-o 22c 41
-o 22c 0
-o 22c 4
-o 22c 1c
On the host:
A bug was just triggered in audio_calloc
Save all your work and restart without audio
I am sorry
Context:
audio_pcm_sw_alloc_resources_out passed invalid arguments to audio_calloc
nmemb=0 size=16 (len=0)
Aborted (core dumped)
Here is another example to trigger the same abort. Start a Linux guest
with an AC97 audio device:
./qemu-system-x86_64 -machine q35,pcspk-audiodev=audio0 \
-device AC97,bus=pcie.0,addr=0x1b,audiodev=audio0 \
-audiodev pa,id=audio0 \
- ...
Open a shell on the guest:
~>sudo lspci -s '00:1b.0' -nn -vv
00:1b.0 Multimedia audio controller [0401]: Intel Corporation 82801AA
AC'97 Audio Controller [8086:2415] (rev 01)
Subsystem: Red Hat, Inc. QEMU Virtual Machine [1af4:1100]
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR+ FastB2B- DisINTx-
Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0
Interrupt: pin A routed to IRQ 16
Region 0: I/O ports at c000 [size=1K]
Region 1: I/O ports at c400 [size=256]
Kernel driver in use: snd_intel8x0
Kernel modules: snd_intel8x0
~># IOBAR0 + 0x2c
~>sudo outw 0xc02c 1
On the host:
A bug was just triggered in audio_calloc
Save all your work and restart without audio
I am sorry
Context:
audio_pcm_sw_alloc_resources_out passed invalid arguments to audio_calloc
nmemb=0 size=16 (len=0)
Aborted (core dumped)
Remove the abort() in audio_bug() to avoid this class of guest-triggered
aborts.
Volker Rümelin (2):
Revert "audio: Log context for audio bug"
audio: remove abort() in audio_bug()
audio/audio.c | 24 ++++++++++++------------
audio/audio_template.h | 27 +++++++++++++++------------
2 files changed, 27 insertions(+), 24 deletions(-)
--
2.35.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] Revert "audio: Log context for audio bug"
2022-09-17 13:15 [PATCH 0/2] audio: prevent a class of guest-triggered aborts Volker Rümelin
@ 2022-09-17 13:16 ` Volker Rümelin
2022-09-17 13:16 ` [PATCH 2/2] audio: remove abort() in audio_bug() Volker Rümelin
1 sibling, 0 replies; 3+ messages in thread
From: Volker Rümelin @ 2022-09-17 13:16 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Philippe Mathieu-Daudé
This reverts commit 8e30d39bade3010387177ca23dbc2244352ed4a3.
Revert commit 8e30d39bad "audio: Log context for audio bug"
to make error propagation work again.
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
audio/audio.c | 25 +++++++++++++------------
audio/audio_template.h | 27 +++++++++++++++------------
2 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 76b8735b44..545ded6674 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -117,6 +117,7 @@ int audio_bug (const char *funcname, int cond)
AUD_log (NULL, "I am sorry\n");
}
AUD_log (NULL, "Context:\n");
+ abort();
}
return cond;
@@ -137,7 +138,7 @@ static inline int audio_bits_to_index (int bits)
default:
audio_bug ("bits_to_index", 1);
AUD_log (NULL, "invalid bits %d\n", bits);
- abort();
+ return 0;
}
}
@@ -155,7 +156,7 @@ void *audio_calloc (const char *funcname, int nmemb, size_t size)
AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n",
funcname);
AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len);
- abort();
+ return NULL;
}
return g_malloc0 (len);
@@ -542,7 +543,7 @@ static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
size_t live = hw->total_samples_captured - audio_pcm_hw_find_min_in (hw);
if (audio_bug(__func__, live > hw->conv_buf->size)) {
dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size);
- abort();
+ return 0;
}
return live;
}
@@ -580,7 +581,7 @@ static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size)
}
if (audio_bug(__func__, live > hw->conv_buf->size)) {
dolog("live_in=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size);
- abort();
+ return 0;
}
rpos = audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size);
@@ -655,7 +656,7 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live)
if (audio_bug(__func__, live > hw->mix_buf->size)) {
dolog("live=%zu hw->mix_buf->size=%zu\n", live, hw->mix_buf->size);
- abort();
+ return 0;
}
return live;
}
@@ -705,7 +706,7 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size)
live = sw->total_hw_samples_mixed;
if (audio_bug(__func__, live > hwsamples)) {
dolog("live=%zu hw->mix_buf->size=%zu\n", live, hwsamples);
- abort();
+ return 0;
}
if (live == hwsamples) {
@@ -997,7 +998,7 @@ static size_t audio_get_avail (SWVoiceIn *sw)
if (audio_bug(__func__, live > sw->hw->conv_buf->size)) {
dolog("live=%zu sw->hw->conv_buf->size=%zu\n", live,
sw->hw->conv_buf->size);
- abort();
+ return 0;
}
ldebug (
@@ -1027,7 +1028,7 @@ static size_t audio_get_free(SWVoiceOut *sw)
if (audio_bug(__func__, live > sw->hw->mix_buf->size)) {
dolog("live=%zu sw->hw->mix_buf->size=%zu\n", live,
sw->hw->mix_buf->size);
- abort();
+ return 0;
}
dead = sw->hw->mix_buf->size - live;
@@ -1169,7 +1170,7 @@ static void audio_run_out (AudioState *s)
if (audio_bug(__func__, live > hw->mix_buf->size)) {
dolog("live=%zu hw->mix_buf->size=%zu\n", live, hw->mix_buf->size);
- abort();
+ continue;
}
if (hw->pending_disable && !nb_live) {
@@ -1202,7 +1203,7 @@ static void audio_run_out (AudioState *s)
if (audio_bug(__func__, hw->mix_buf->pos >= hw->mix_buf->size)) {
dolog("hw->mix_buf->pos=%zu hw->mix_buf->size=%zu played=%zu\n",
hw->mix_buf->pos, hw->mix_buf->size, played);
- abort();
+ hw->mix_buf->pos = 0;
}
#ifdef DEBUG_OUT
@@ -1222,7 +1223,7 @@ static void audio_run_out (AudioState *s)
if (audio_bug(__func__, played > sw->total_hw_samples_mixed)) {
dolog("played=%zu sw->total_hw_samples_mixed=%zu\n",
played, sw->total_hw_samples_mixed);
- abort();
+ played = sw->total_hw_samples_mixed;
}
sw->total_hw_samples_mixed -= played;
@@ -1345,7 +1346,7 @@ static void audio_run_capture (AudioState *s)
if (audio_bug(__func__, captured > sw->total_hw_samples_mixed)) {
dolog("captured=%zu sw->total_hw_samples_mixed=%zu\n",
captured, sw->total_hw_samples_mixed);
- abort();
+ captured = sw->total_hw_samples_mixed;
}
sw->total_hw_samples_mixed -= captured;
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 7192b19e73..d2d348638b 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -59,13 +59,12 @@ static void glue(audio_init_nb_voices_, TYPE)(AudioState *s,
if (audio_bug(__func__, !voice_size && max_voices)) {
dolog ("drv=`%s' voice_size=0 max_voices=%d\n",
drv->name, max_voices);
- abort();
+ glue (s->nb_hw_voices_, TYPE) = 0;
}
if (audio_bug(__func__, voice_size && !max_voices)) {
dolog ("drv=`%s' voice_size=%d max_voices=0\n",
drv->name, voice_size);
- abort();
}
}
@@ -82,7 +81,6 @@ static void glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
size_t samples = hw->samples;
if (audio_bug(__func__, samples == 0)) {
dolog("Attempted to allocate empty buffer\n");
- abort();
}
HWBUF = g_malloc0(sizeof(STSampleBuffer) + sizeof(st_sample) * samples);
@@ -254,12 +252,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
if (audio_bug(__func__, !drv)) {
dolog ("No host audio driver\n");
- abort();
+ return NULL;
}
if (audio_bug(__func__, !drv->pcm_ops)) {
dolog ("Host audio driver without pcm_ops\n");
- abort();
+ return NULL;
}
hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE));
@@ -277,13 +275,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
QLIST_INIT (&hw->cap_head);
#endif
if (glue (hw->pcm_ops->init_, TYPE) (hw, as, s->drv_opaque)) {
- g_free(hw);
- return NULL;
+ goto err0;
}
if (audio_bug(__func__, hw->samples <= 0)) {
dolog("hw->samples=%zd\n", hw->samples);
- abort();
+ goto err1;
}
if (hw->info.is_float) {
@@ -312,6 +309,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
audio_attach_capture (hw);
#endif
return hw;
+
+ err1:
+ glue (hw->pcm_ops->fini_, TYPE) (hw);
+ err0:
+ g_free (hw);
+ return NULL;
}
AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
@@ -432,7 +435,7 @@ void glue (AUD_close_, TYPE) (QEMUSoundCard *card, SW *sw)
if (sw) {
if (audio_bug(__func__, !card)) {
dolog ("card=%p\n", card);
- abort();
+ return;
}
glue (audio_close_, TYPE) (sw);
@@ -454,7 +457,7 @@ SW *glue (AUD_open_, TYPE) (
if (audio_bug(__func__, !card || !name || !callback_fn || !as)) {
dolog ("card=%p name=%p callback_fn=%p as=%p\n",
card, name, callback_fn, as);
- abort();
+ goto fail;
}
s = card->state;
@@ -465,12 +468,12 @@ SW *glue (AUD_open_, TYPE) (
if (audio_bug(__func__, audio_validate_settings(as))) {
audio_print_settings (as);
- abort();
+ goto fail;
}
if (audio_bug(__func__, !s->drv)) {
dolog ("Can not open `%s' (no host audio driver)\n", name);
- abort();
+ goto fail;
}
if (sw && audio_pcm_info_eq (&sw->info, as)) {
--
2.35.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] audio: remove abort() in audio_bug()
2022-09-17 13:15 [PATCH 0/2] audio: prevent a class of guest-triggered aborts Volker Rümelin
2022-09-17 13:16 ` [PATCH 1/2] Revert "audio: Log context for audio bug" Volker Rümelin
@ 2022-09-17 13:16 ` Volker Rümelin
1 sibling, 0 replies; 3+ messages in thread
From: Volker Rümelin @ 2022-09-17 13:16 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Philippe Mathieu-Daudé
Commit ab32b78cd1 "audio: Simplify audio_bug() removing old code"
introduced abort() in audio_bug() for regular builds.
audio_bug() was never meant to abort QEMU for the following
reasons.
- There's code in audio_bug() that expects audio_bug() gets
called more than once with error condition true. The variable
'shown' is only 0 on first error.
- All call sites test the return code of audio_bug(), print
an error context message and handle the errror.
- The abort() in audio_bug() enables a class of guest-triggered
aborts similar to the Launchpad Bug #1910603 at
https://bugs.launchpad.net/bugs/1910603.
Fixes: ab32b78cd1 "audio: Simplify audio_bug() removing old code"
Buglink: https://bugs.launchpad.net/bugs/1910603
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
audio/audio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/audio/audio.c b/audio/audio.c
index 545ded6674..01c0cd8202 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -117,7 +117,6 @@ int audio_bug (const char *funcname, int cond)
AUD_log (NULL, "I am sorry\n");
}
AUD_log (NULL, "Context:\n");
- abort();
}
return cond;
--
2.35.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-17 13:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-17 13:15 [PATCH 0/2] audio: prevent a class of guest-triggered aborts Volker Rümelin
2022-09-17 13:16 ` [PATCH 1/2] Revert "audio: Log context for audio bug" Volker Rümelin
2022-09-17 13:16 ` [PATCH 2/2] audio: remove abort() in audio_bug() Volker Rümelin
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.