From b6bf97e2f199ae9c947e3b192b690c1d6257517b Mon Sep 17 00:00:00 2001 From: Paul Schwabauer Date: Sat, 21 Mar 2026 19:34:18 +0100 Subject: [PATCH] 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. --- soh/soh/mixer.c | 4 ++-- soh/src/code/audio_synthesis.c | 35 +++++++++++++++------------------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/soh/soh/mixer.c b/soh/soh/mixer.c index 1646d1a07..508ae401d 100644 --- a/soh/soh/mixer.c +++ b/soh/soh/mixer.c @@ -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 } diff --git a/soh/src/code/audio_synthesis.c b/soh/src/code/audio_synthesis.c index abd657e0d..d283d4cfb 100644 --- a/soh/src/code/audio_synthesis.c +++ b/soh/src/code/audio_synthesis.c @@ -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; }