mirror of
https://github.com/HarbourMasters/Shipwright
synced 2026-08-21 06:48:39 -04:00
fix(audio): bound audio-heap cache tables (SIGSEGV with large custom music packs) (#6932)
Root cause of three identical field crashes (audio thread, opcode fetch
through a pointer with its low 32 bits overwritten, seconds after scene
transitions): AudioHeap_AllocPermanent writes permanentCache[index] with
index = permanentPool.count and no bound against the 32-entry array. In
SoH every soundfont sync-load is forced permanent, and custom sequences
whose SEQ.xml says CachePolicy="Temporary" ALSO allocate permanently
(the factory stores the LUS enum where CACHE_TEMPORARY == 0, while
AudioLoad_SyncLoad's switch reads 0 with the ROM convention
'permanent'). A pack with ~60 streamed customs plus vanilla fonts pushes
count past 32 within a session, after which each allocation sprays a
{ptr, size, tableType/id} triplet at 24-byte stride through
gAudioContext - entry[135]'s ptr field lands exactly on
seqPlayers[0].scriptState.pc and entry[156] on seqPlayers[1]'s (both
verified against the crash-dump registers).
- permanentCache raised 32 -> 512 (12 KB) and AllocPermanent refuses
allocations past the array instead of corrupting memory.
- Same unbounded-index disease fixed in the three sibling writers:
AllocCached's persistent path (16-entry array; CACHE_EITHER degrades
to temporary, hard persistent requests fail cleanly),
AllocPersistentSampleCacheEntry, AllocTemporarySampleCacheEntry.
- seqLoadStatus malloc sized for the full id space (sequenceMapSize +
0xF) matching sequenceMap; custom ids above sequenceMapSize previously
overflowed the allocation by up to 15 bytes.
Upstream SoH bugs, not branch-introduced - this branch's many-track
packs merely made the overflow reachable in normal play. Standalone
upstreamable fix.
This commit is contained in:
@@ -915,7 +915,8 @@ typedef struct {
|
|||||||
/* 0x2B30 */ AudioCache fontCache;
|
/* 0x2B30 */ AudioCache fontCache;
|
||||||
/* 0x2C40 */ AudioCache sampleBankCache;
|
/* 0x2C40 */ AudioCache sampleBankCache;
|
||||||
/* 0x2D50 */ AudioAllocPool permanentPool;
|
/* 0x2D50 */ AudioAllocPool permanentPool;
|
||||||
/* 0x2D60 */ AudioCacheEntry permanentCache[32];
|
// SOH [Bugfix] 32 -> 512: large custom-music packs overflowed this (see AudioHeap_AllocPermanent).
|
||||||
|
/* 0x2D60 */ AudioCacheEntry permanentCache[512];
|
||||||
/* 0x2EE0 */ AudioSampleCache persistentSampleCache;
|
/* 0x2EE0 */ AudioSampleCache persistentSampleCache;
|
||||||
/* 0x3174 */ AudioSampleCache temporarySampleCache;
|
/* 0x3174 */ AudioSampleCache temporarySampleCache;
|
||||||
/* 0x3408 */ AudioPoolSplit4 sessionPoolSplit;
|
/* 0x3408 */ AudioPoolSplit4 sessionPoolSplit;
|
||||||
|
|||||||
@@ -534,6 +534,14 @@ void* AudioHeap_AllocCached(s32 tableType, ptrdiff_t size, s32 cache, s32 id) {
|
|||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SOH [Bugfix] Bound entries[] (see AudioHeap_AllocPermanent); CACHE_EITHER falls back to temporary.
|
||||||
|
if (loadedPool->persistent.numEntries >= ARRAY_COUNT(loadedPool->persistent.entries)) {
|
||||||
|
if (cache == CACHE_EITHER) {
|
||||||
|
return AudioHeap_AllocCached(tableType, size, CACHE_TEMPORARY, id);
|
||||||
|
}
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
mem = AudioHeap_Alloc(&loadedPool->persistent.pool, size);
|
mem = AudioHeap_Alloc(&loadedPool->persistent.pool, size);
|
||||||
loadedPool->persistent.entries[loadedPool->persistent.numEntries].ptr = mem;
|
loadedPool->persistent.entries[loadedPool->persistent.numEntries].ptr = mem;
|
||||||
|
|
||||||
@@ -1011,6 +1019,12 @@ void* AudioHeap_AllocPermanent(s32 tableType, s32 id, size_t size) {
|
|||||||
|
|
||||||
index = gAudioContext.permanentPool.count;
|
index = gAudioContext.permanentPool.count;
|
||||||
|
|
||||||
|
// SOH [Bugfix] Bound permanentCache: large custom-music packs overflowed it and corrupted
|
||||||
|
// gAudioContext (crashed the audio thread). Refuse rather than corrupt memory; callers handle NULL.
|
||||||
|
if (index >= ARRAY_COUNT(gAudioContext.permanentCache)) {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
ret = AudioHeap_Alloc(&gAudioContext.permanentPool, size);
|
ret = AudioHeap_Alloc(&gAudioContext.permanentPool, size);
|
||||||
gAudioContext.permanentCache[index].ptr = ret;
|
gAudioContext.permanentCache[index].ptr = ret;
|
||||||
if (ret == NULL) {
|
if (ret == NULL) {
|
||||||
@@ -1134,6 +1148,10 @@ SampleCacheEntry* AudioHeap_AllocTemporarySampleCacheEntry(size_t size) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (index == -1) {
|
if (index == -1) {
|
||||||
|
// SOH [Bugfix] Bound entries[] (see AudioHeap_AllocPermanent); callers treat NULL as uncached.
|
||||||
|
if (pool->size >= ARRAY_COUNT(pool->entries)) {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
index = pool->size++;
|
index = pool->size++;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1210,6 +1228,10 @@ SampleCacheEntry* AudioHeap_AllocPersistentSampleCacheEntry(size_t size) {
|
|||||||
void* mem;
|
void* mem;
|
||||||
|
|
||||||
pool = &gAudioContext.persistentSampleCache;
|
pool = &gAudioContext.persistentSampleCache;
|
||||||
|
// SOH [Bugfix] Bound entries[] (see AudioHeap_AllocPermanent).
|
||||||
|
if (pool->size >= ARRAY_COUNT(pool->entries)) {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
mem = AudioHeap_Alloc(&pool->pool, size);
|
mem = AudioHeap_Alloc(&pool->pool, size);
|
||||||
if (mem == NULL) {
|
if (mem == NULL) {
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|||||||
@@ -1350,8 +1350,9 @@ void AudioLoad_Init(void* heap, size_t heapSize) {
|
|||||||
// calloc: unassigned slots stay NULL for the guard in AudioLoad_SyncInitSeqPlayerInternal().
|
// calloc: unassigned slots stay NULL for the guard in AudioLoad_SyncInitSeqPlayerInternal().
|
||||||
sequenceMap = calloc(sequenceMapSize + 0xF, sizeof(char*));
|
sequenceMap = calloc(sequenceMapSize + 0xF, sizeof(char*));
|
||||||
|
|
||||||
gAudioContext.seqLoadStatus = malloc(sequenceMapSize);
|
// SOH [Bugfix] Size to match sequenceMap (+ 0xF); custom ids can exceed sequenceMapSize.
|
||||||
memset(gAudioContext.seqLoadStatus, 5, sequenceMapSize);
|
gAudioContext.seqLoadStatus = malloc(sequenceMapSize + 0xF);
|
||||||
|
memset(gAudioContext.seqLoadStatus, 5, sequenceMapSize + 0xF);
|
||||||
for (size_t i = 0; i < seqListSize; i++) {
|
for (size_t i = 0; i < seqListSize; i++) {
|
||||||
SequenceData sDat = ResourceMgr_LoadSeqByName(seqList[i]);
|
SequenceData sDat = ResourceMgr_LoadSeqByName(seqList[i]);
|
||||||
sequenceMap[sDat.seqNumber] = strdup(seqList[i]);
|
sequenceMap[sDat.seqNumber] = strdup(seqList[i]);
|
||||||
|
|||||||
Reference in New Issue
Block a user