hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h

Commit Message

Qiang Liu June 23, 2021, 12:24 a.m. UTC

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 <[e-mail]>
---
 hw/audio/sb16.c              | 31 +++++++++++++++++++------------
 tests/qtest/fuzz-sb16-test.c | 17 +++++++++++++++++
 2 files changed, 36 insertions(+), 12 deletions(-)

--
2.7.4

Comments

Gerd Hoffmann June 23, 2021, 12:20 p.m. UTC | #1

Hi,

Applying: hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h
error: corrupt patch at line 35
error: could not build fake ancestor

> @@ -244,17 +261,7 @@ static void dma_cmd8 (SB16State *s, int mask, int
> dma_len)

This looks like your mailer mangled the patch.
Can you resent using 'git send-email'?

thanks,
  Gerd

Qiang Liu June 24, 2021, 2:11 a.m. UTC | #2

Hi

> This looks like your mailer mangled the patch.
> Can you resent using 'git send-email`?
Yes, I can. I finally figure it out how to config my Gmail. I should
send a new email
and this thread is supposed to be closed, right?

Best
Qiang

Gerd Hoffmann June 24, 2021, 8:42 a.m. UTC | #3

On Thu, Jun 24, 2021 at 10:11:35AM +0800, Qiang Liu wrote:
> Hi
> 
> > This looks like your mailer mangled the patch.
> > Can you resent using 'git send-email`?
> Yes, I can. I finally figure it out how to config my Gmail. I should
> send a new email
> and this thread is supposed to be closed, right?

Yes, sending as new message (or thread for a series) is correct.
Applying v3 worked, queued up now.

thanks,
  Gerd

Patch

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();