Hi folks,
With this patch, having tested more, I find another way to trigger the
assertion.
I found it just now such that I did a quick investigation and reported
it to you. I
hope this would prevent merging this patch.
Brief analysis:
This existing patch limits s->freq in dma_cmd8 before continue_dma8 followed
by AUD_open_out. It's good to prevent the flow by this path. However, we can
directly call continue_dma8 via command 0xd4 but there is no limit check there.
To trigger this assertion, we can manipulate s->freq in the following way.
1. dsp_write, offset=0xc, val=0x41
Because s->needed_bytes = 0, command() is called.
```
case 0x41:
s->freq = -1;
s->time_const = -1;
s->needed_bytes = 2; // look at here
...
s->cmd = cmd; // 0x41, and here
```
2. dsp_write, offset=0xc, val=0x14
Because s->needed_bytes = 2, complete() is called.
```
s->in2_data[s->in_index++] = 0x14; // remembere this
s->needed_bytes = 0
```
Because s->cmd = 0x41, s->freq will be reset.
```
case 0x41:
case 0x42:
s->freq = dsp_get_hilo (s);
// return s->in2_data[--s->in_index]
// s->freq will be 0x14, there is no check ...
```
3. dsp_write, offset=0xc, val=0xd4
Call continue_dma8 directly then we can trigger this assertion because
s->freq is too small.
Maybe we can fix this flaw by adding s->freq check after s->freq =
dsp_get_hilo (s) in the second step?
Best,
Qiang
On Thu, Jun 17, 2021 at 5:56 PM Gerd Hoffmann <kra...@redhat.com> wrote:
>
> On Wed, Jun 16, 2021 at 12:43:49PM +0200, Philippe Mathieu-Daudé wrote:
> > While the SB16 seems to work up to 48000 Hz, the "Sound Blaster Series
> > Hardware Programming Guide" limit the sampling range from 4000 Hz to
> > 44100 Hz (Section 3-9, 3-10: Digitized Sound I/O Programming, tables
> > 3-2 and 3-3).
> >
> > Later, section 6-15 (DSP Commands) is more specific regarding the 41h /
> > 42h registers (Set digitized sound output sampling rate):
> >
> > Valid sampling rates range from 5000 to 45000 Hz inclusive.
> >
> > There is no comment regarding error handling if the register is filled
> > with an out-of-range value. (See also section 3-28 "8-bit or 16-bit
> > Auto-initialize Transfer"). Assume limits are enforced in hardware.
> >
> > This fixes triggering an assertion in audio_calloc():
> >
> > #1 abort
> > #2 audio_bug audio/audio.c:119:9
> > #3 audio_calloc audio/audio.c:154:9
> > #4 audio_pcm_sw_alloc_resources_out audio/audio_template.h:116:15
> > #5 audio_pcm_sw_init_out audio/audio_template.h:175:11
> > #6 audio_pcm_create_voice_pair_out audio/audio_template.h:410:9
> > #7 AUD_open_out audio/audio_template.h:503:14
> > #8 continue_dma8 hw/audio/sb16.c:216:20
> > #9 dma_cmd8 hw/audio/sb16.c:276:5
> > #10 command hw/audio/sb16.c:0
> > #11 dsp_write hw/audio/sb16.c:949:13
> > #12 portio_write softmmu/ioport.c:205:13
> > #13 memory_region_write_accessor softmmu/memory.c:491:5
> > #14 access_with_adjusted_size softmmu/memory.c:552:18
> > #15 memory_region_dispatch_write softmmu/memory.c:0:13
> > #16 flatview_write_continue softmmu/physmem.c:2759:23
> > #17 flatview_write softmmu/physmem.c:2799:14
> > #18 address_space_write softmmu/physmem.c:2891:18
> > #19 cpu_outw softmmu/ioport.c:70:5
> >
> > [*] http://www.baudline.com/solutions/full_duplex/sb16_pci/index.html
> >
> > Fixes: 85571bc7415 ("audio merge (malc)")
> > Buglink: https://bugs.launchpad.net/bugs/1910603
> > OSS-Fuzz Report: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29174
> > Tested-by: Qiang Liu <cyruscy...@gmail.com>
> > Reviewed-by: Qiang Liu <cyruscy...@gmail.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> Added to audio queue.
>
> thanks,
> Gerd
>
