* [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h
@ 2021-06-24 2:44 Qiang Liu
2021-06-24 9:29 ` Philippe Mathieu-Daudé
2021-06-24 9:38 ` Stefan Hajnoczi
0 siblings, 2 replies; 3+ messages in thread
From: Qiang Liu @ 2021-06-24 2:44 UTC (permalink / raw)
Cc: Laurent Vivier, Thomas Huth, Qiang Liu, f4bug,
open list:All patches CC here, Alexander Bulekov, Bandan Das,
Gerd Hoffmann, Stefan Hajnoczi, Paolo Bonzini
The I/O sampling rate range is enforced to 5000 to 45000HZ according to
commit a2cd86a9. Setting I/O sampling rate with command 41h/42h, a guest
user can break this assumption and trigger an assertion in audio_calloc
via command 0xd4. This patch restricts the I/O sampling rate range for
command 41h/42h.
Fixes: 85571bc7415 ("audio merge (malc)")
Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
---
hw/audio/sb16.c | 31 +++++++++++++++++++------------
tests/qtest/fuzz-sb16-test.c | 17 +++++++++++++++++
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 5cf121f..60f1f75 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -229,6 +229,23 @@ static void continue_dma8 (SB16State *s)
control (s, 1);
}
+static inline int restrict_sampling_rate(int freq)
+{
+ if (freq < SAMPLE_RATE_MIN) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "sampling range too low: %d, increasing to %u\n",
+ freq, SAMPLE_RATE_MIN);
+ return SAMPLE_RATE_MIN;
+ } else if (freq > SAMPLE_RATE_MAX) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "sampling range too high: %d, decreasing to %u\n",
+ freq, SAMPLE_RATE_MAX);
+ return SAMPLE_RATE_MAX;
+ } else {
+ return freq;
+ }
+}
+
static void dma_cmd8 (SB16State *s, int mask, int dma_len)
{
s->fmt = AUDIO_FORMAT_U8;
@@ -244,17 +261,7 @@ static void dma_cmd8 (SB16State *s, int mask, int dma_len)
int tmp = (256 - s->time_const);
s->freq = (1000000 + (tmp / 2)) / tmp;
}
- if (s->freq < SAMPLE_RATE_MIN) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "sampling range too low: %d, increasing to %u\n",
- s->freq, SAMPLE_RATE_MIN);
- s->freq = SAMPLE_RATE_MIN;
- } else if (s->freq > SAMPLE_RATE_MAX) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "sampling range too high: %d, decreasing to %u\n",
- s->freq, SAMPLE_RATE_MAX);
- s->freq = SAMPLE_RATE_MAX;
- }
+ s->freq = restrict_sampling_rate(s->freq);
if (dma_len != -1) {
s->block_size = dma_len << s->fmt_stereo;
@@ -768,7 +775,7 @@ static void complete (SB16State *s)
* and FT2 sets output freq with this (go figure). Compare:
* http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate
*/
- s->freq = dsp_get_hilo (s);
+ s->freq = restrict_sampling_rate(dsp_get_hilo(s));
ldebug ("set freq %d\n", s->freq);
break;
diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c
index 51030cd..f47a8bc 100644
--- a/tests/qtest/fuzz-sb16-test.c
+++ b/tests/qtest/fuzz-sb16-test.c
@@ -37,6 +37,22 @@ static void test_fuzz_sb16_0x91(void)
qtest_quit(s);
}
+/*
+ * This used to trigger the assert in audio_calloc
+ * through command 0xd4
+ */
+static void test_fuzz_sb16_0xd4(void)
+{
+ QTestState *s = qtest_init("-M pc -display none "
+ "-device sb16,audiodev=none "
+ "-audiodev id=none,driver=none");
+ qtest_outb(s, 0x22c, 0x41);
+ qtest_outb(s, 0x22c, 0x00);
+ qtest_outb(s, 0x22c, 0x14);
+ qtest_outb(s, 0x22c, 0xd4);
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
const char *arch = qtest_get_arch();
@@ -46,6 +62,7 @@ int main(int argc, char **argv)
if (strcmp(arch, "i386") == 0) {
qtest_add_func("fuzz/test_fuzz_sb16/1c", test_fuzz_sb16_0x1c);
qtest_add_func("fuzz/test_fuzz_sb16/91", test_fuzz_sb16_0x91);
+ qtest_add_func("fuzz/test_fuzz_sb16/d4", test_fuzz_sb16_0xd4);
}
return g_test_run();
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h
2021-06-24 2:44 [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h Qiang Liu
@ 2021-06-24 9:29 ` Philippe Mathieu-Daudé
2021-06-24 9:38 ` Stefan Hajnoczi
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-24 9:29 UTC (permalink / raw)
To: Qiang Liu
Cc: Laurent Vivier, Thomas Huth, open list:All patches CC here,
Alexander Bulekov, Bandan Das, Gerd Hoffmann, Stefan Hajnoczi,
Paolo Bonzini
On 6/24/21 4:44 AM, Qiang Liu wrote:
> The I/O sampling rate range is enforced to 5000 to 45000HZ according to
> commit a2cd86a9. Setting I/O sampling rate with command 41h/42h, a guest
> user can break this assumption and trigger an assertion in audio_calloc
> via command 0xd4. This patch restricts the I/O sampling rate range for
> command 41h/42h.
>
> Fixes: 85571bc7415 ("audio merge (malc)")
> Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
> ---
> hw/audio/sb16.c | 31 +++++++++++++++++++------------
> tests/qtest/fuzz-sb16-test.c | 17 +++++++++++++++++
> 2 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
> index 5cf121f..60f1f75 100644
> --- a/hw/audio/sb16.c
> +++ b/hw/audio/sb16.c
> @@ -229,6 +229,23 @@ static void continue_dma8 (SB16State *s)
> control (s, 1);
> }
>
> +static inline int restrict_sampling_rate(int freq)
No need to 'inline', 1/ this is not performance critical code
path, and 2/ we expect the compiler to be clever enough to figure
that by itself.
> +{
> + if (freq < SAMPLE_RATE_MIN) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sampling range too low: %d, increasing to %u\n",
> + freq, SAMPLE_RATE_MIN);
> + return SAMPLE_RATE_MIN;
> + } else if (freq > SAMPLE_RATE_MAX) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sampling range too high: %d, decreasing to %u\n",
> + freq, SAMPLE_RATE_MAX);
> + return SAMPLE_RATE_MAX;
> + } else {
> + return freq;
> + }
> +}
> +
> static void dma_cmd8 (SB16State *s, int mask, int dma_len)
> {
> s->fmt = AUDIO_FORMAT_U8;
> @@ -244,17 +261,7 @@ static void dma_cmd8 (SB16State *s, int mask, int dma_len)
> int tmp = (256 - s->time_const);
> s->freq = (1000000 + (tmp / 2)) / tmp;
> }
> - if (s->freq < SAMPLE_RATE_MIN) {
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "sampling range too low: %d, increasing to %u\n",
> - s->freq, SAMPLE_RATE_MIN);
> - s->freq = SAMPLE_RATE_MIN;
> - } else if (s->freq > SAMPLE_RATE_MAX) {
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "sampling range too high: %d, decreasing to %u\n",
> - s->freq, SAMPLE_RATE_MAX);
> - s->freq = SAMPLE_RATE_MAX;
> - }
> + s->freq = restrict_sampling_rate(s->freq);
>
> if (dma_len != -1) {
> s->block_size = dma_len << s->fmt_stereo;
> @@ -768,7 +775,7 @@ static void complete (SB16State *s)
> * and FT2 sets output freq with this (go figure). Compare:
> * http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate
> */
> - s->freq = dsp_get_hilo (s);
> + s->freq = restrict_sampling_rate(dsp_get_hilo(s));
It seems the best we can do here...
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ldebug ("set freq %d\n", s->freq);
> break;
Thanks for the fix,
Phil.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h
2021-06-24 2:44 [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h Qiang Liu
2021-06-24 9:29 ` Philippe Mathieu-Daudé
@ 2021-06-24 9:38 ` Stefan Hajnoczi
1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2021-06-24 9:38 UTC (permalink / raw)
To: Qiang Liu
Cc: Laurent Vivier, Thomas Huth, f4bug, open list:All patches CC here,
Alexander Bulekov, Bandan Das, Gerd Hoffmann, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 722 bytes --]
On Thu, Jun 24, 2021 at 10:44:47AM +0800, Qiang Liu wrote:
> The I/O sampling rate range is enforced to 5000 to 45000HZ according to
> commit a2cd86a9. Setting I/O sampling rate with command 41h/42h, a guest
> user can break this assumption and trigger an assertion in audio_calloc
> via command 0xd4. This patch restricts the I/O sampling rate range for
> command 41h/42h.
>
> Fixes: 85571bc7415 ("audio merge (malc)")
> Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
> ---
> hw/audio/sb16.c | 31 +++++++++++++++++++------------
> tests/qtest/fuzz-sb16-test.c | 17 +++++++++++++++++
> 2 files changed, 36 insertions(+), 12 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-28 13:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-24 2:44 [PATCH v2] hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h Qiang Liu
2021-06-24 9:29 ` Philippe Mathieu-Daudé
2021-06-24 9:38 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).