Fix ADPCM sample buffer overread in audio synthesis (#6364)
The sampleDataStartPad and aligned variables existed solely to satisfy the N64 RSP DMA requirement that source addresses be 16-byte aligned. On PC, aLoadBuffer is a plain memcpy with no such constraint. The alignment dance caused aLoadBuffer to read up to 15 bytes before sampleData and up to 8+ bytes past the end of the sample buffer. On platforms with strict allocator guard pages (e.g. OpenBSD), this triggers a SIGSEGV. A second issue remains after removing the alignment dance: nFramesToDecode is derived from sample counts (loopEnd), but size is not always a multiple of frameSize. loopEnd and size are derived independently during encoding and can disagree on the final partial frame, leaving nFramesToDecode * frameSize exceeding the remaining bytes in the buffer. Remove sampleDataStartPad and aligned entirely. Clamp the load to min(nFramesToDecode * frameSize, audioFontSample->size - sampleDataOffset). The ADPCM decoder operates on DMEM, so a partial last frame in DMEM produces at most a negligible artifact at sound termination.
This commit is contained in:
@@ -96,11 +96,11 @@ void aClearBufferImpl(uint16_t addr, int nbytes) {
|
||||
|
||||
void aLoadBufferImpl(const void* source_addr, uint16_t dest_addr, uint16_t nbytes) {
|
||||
#if __SANITIZE_ADDRESS__
|
||||
for (size_t i = 0; i < ROUND_DOWN_16(nbytes); i++) {
|
||||
for (size_t i = 0; i < nbytes; i++) {
|
||||
BUF_U8(dest_addr)[i] = ((const unsigned char*)source_addr)[i];
|
||||
}
|
||||
#else
|
||||
memcpy(BUF_U8(dest_addr), source_addr, ROUND_DOWN_16(nbytes));
|
||||
memcpy(BUF_U8(dest_addr), source_addr, nbytes);
|
||||
#endif
|
||||
}
|
||||
|
||||
|
||||
@@ -699,7 +699,6 @@ Acmd* AudioSynth_ProcessNote(s32 noteIndex, NoteSubEu* noteSubEu, NoteSynthesisS
|
||||
u8* sampleData;
|
||||
s32 nParts;
|
||||
s32 curPart;
|
||||
intptr_t sampleDataStartPad;
|
||||
s32 side;
|
||||
s32 resampledTempLen;
|
||||
u16 noteSamplesDmemAddrBeforeResampling;
|
||||
@@ -713,7 +712,6 @@ Acmd* AudioSynth_ProcessNote(s32 noteIndex, NoteSubEu* noteSubEu, NoteSynthesisS
|
||||
s16* filter;
|
||||
s32 bookOffset;
|
||||
s32 finished;
|
||||
s32 aligned;
|
||||
s16 addr;
|
||||
u16 unused;
|
||||
|
||||
@@ -896,14 +894,17 @@ Acmd* AudioSynth_ProcessNote(s32 noteIndex, NoteSubEu* noteSubEu, NoteSynthesisS
|
||||
return cmd;
|
||||
}
|
||||
|
||||
sampleDataStartPad = (uintptr_t)sampleData & 0xF;
|
||||
aligned = ALIGN16((nFramesToDecode * frameSize) + 16);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - aligned;
|
||||
|
||||
aLoadBuffer(cmd++, sampleData - sampleDataStartPad, addr, aligned);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - ALIGN16(nFramesToDecode * frameSize);
|
||||
u32 bytesToLoad = nFramesToDecode * frameSize;
|
||||
u32 bytesAvail = (u32)sampleDataOffset < audioFontSample->size
|
||||
? audioFontSample->size - (u32)sampleDataOffset
|
||||
: 0;
|
||||
if (bytesToLoad > bytesAvail) {
|
||||
bytesToLoad = bytesAvail;
|
||||
}
|
||||
aLoadBuffer(cmd++, sampleData, addr, bytesToLoad);
|
||||
} else {
|
||||
nSamplesToDecode = 0;
|
||||
sampleDataStartPad = 0;
|
||||
}
|
||||
|
||||
if (synthState->restart) {
|
||||
@@ -920,24 +921,18 @@ Acmd* AudioSynth_ProcessNote(s32 noteIndex, NoteSubEu* noteSubEu, NoteSynthesisS
|
||||
}
|
||||
switch (audioFontSample->codec) {
|
||||
case CODEC_ADPCM:
|
||||
aligned = ALIGN16((nFramesToDecode * frameSize) + 0x10);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - aligned;
|
||||
aSetBuffer(cmd++, 0, addr + sampleDataStartPad, DMEM_UNCOMPRESSED_NOTE + phi_s4,
|
||||
nSamplesToDecode * 2);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - ALIGN16(nFramesToDecode * frameSize);
|
||||
aSetBuffer(cmd++, 0, addr, DMEM_UNCOMPRESSED_NOTE + phi_s4, nSamplesToDecode * 2);
|
||||
aADPCMdec(cmd++, flags, synthState->synthesisBuffers->adpcmdecState);
|
||||
break;
|
||||
case CODEC_SMALL_ADPCM:
|
||||
aligned = ALIGN16((nFramesToDecode * frameSize) + 0x10);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - aligned;
|
||||
aSetBuffer(cmd++, 0, addr + sampleDataStartPad, DMEM_UNCOMPRESSED_NOTE + phi_s4,
|
||||
nSamplesToDecode * 2);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - ALIGN16(nFramesToDecode * frameSize);
|
||||
aSetBuffer(cmd++, 0, addr, DMEM_UNCOMPRESSED_NOTE + phi_s4, nSamplesToDecode * 2);
|
||||
aADPCMdec(cmd++, flags | 4, synthState->synthesisBuffers->adpcmdecState);
|
||||
break;
|
||||
case CODEC_S8:
|
||||
aligned = ALIGN16((nFramesToDecode * frameSize) + 0x10);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - aligned;
|
||||
AudioSynth_SetBuffer(cmd++, 0, addr + sampleDataStartPad, DMEM_UNCOMPRESSED_NOTE + phi_s4,
|
||||
nSamplesToDecode * 2);
|
||||
addr = DMEM_COMPRESSED_ADPCM_DATA - ALIGN16(nFramesToDecode * frameSize);
|
||||
AudioSynth_SetBuffer(cmd++, 0, addr, DMEM_UNCOMPRESSED_NOTE + phi_s4, nSamplesToDecode * 2);
|
||||
AudioSynth_S8Dec(cmd++, flags, synthState->synthesisBuffers->adpcmdecState);
|
||||
break;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user