Fix critical memory-safety and concurrency bugs found in code review

- handleController: bounds-check NRPN/CC controller ID before indexing
  last_midiCC_info[] (NRPN IDs can reach 16383, array is sized 1024)
- JaySynthMidiCC::add/remove: also reject negative controller IDs coming
  from patch/bank file data, not just out-of-range-high ones
- bankImportXml/loadPatchFromFile: null-check XML elements after
  getFirstChildElement()/XmlDocument::parse() before dereferencing, so
  malformed/truncated patch or bank files no longer crash on load
- loadBankFromFile/loadPatchFromFile: validate the loaded file is large
  enough for the fxBank/fxProgram header, and that the embedded chunk
  size fits within the file, before reading through the raw pointer
- processBlock/JaySynth::renderNextBlock: host blocks larger than
  SYNTH_MAX_BUFSIZE no longer overflow the fixed-size stack work buffers
  or deadlock the render worker threads; oversized blocks are now
  rendered in multiple SYNTH_MAX_BUFSIZE-sized passes (pending MIDI
  events are correctly held over between passes)
- JaySynth constructor: guard against createInputStream() returning null
  when the wavetable file can't be opened
- JaySynth::setParameter/setControl/setPerVoiceControl/setParam/
  ClearControls: take the synth's lock, matching renderNextBlock and the
  note/controller handlers, to close a data race between parameter
  changes (UI/host automation) and audio-thread rendering

Verified with a full rebuild (make) producing JaySynth.so with no new
warnings or errors.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ
This commit is contained in:
2026-07-27 17:22:46 +02:00
co-authored by Claude Sonnet 5
parent 468b4c36de
commit a65b71ab86
3 changed files with 127 additions and 72 deletions
+35 -6
View File
@@ -53,7 +53,8 @@ JaySynth::JaySynth (int num_voices, String pathToWaves)
{
File wavesFile = File(pathToWaves);
ScopedPointer<FileInputStream>pIn = wavesFile.createInputStream();
pIn->read(synth_common.vco.wt.wave_rawdata, sizeof(synth_common.vco.wt.wave_rawdata));
if (pIn != NULL)
pIn->read(synth_common.vco.wt.wave_rawdata, sizeof(synth_common.vco.wt.wave_rawdata));
}
max_num_voices = num_voices;
@@ -379,6 +380,8 @@ void JaySynth::humanizeVarianceChanged_ENV(void)
//==============================================================================
void JaySynth::ClearControls(void)
{
const ScopedLock sl (lock);
memset(controls, 0, SYNTH_NUM_PARAMS*sizeof(synth_float_t));
}
@@ -428,6 +431,8 @@ synth_float_t JaySynth::getParameter(int paramID)
void JaySynth::setParameter(int paramID, synth_float_t param)
{
const ScopedLock sl (lock);
params[paramID] = param;
setParam(paramID, -1);
@@ -437,6 +442,8 @@ void JaySynth::setParameter(int paramID, synth_float_t param)
void JaySynth::setControl(int paramID, synth_float_t param)
{
const ScopedLock sl (lock);
controls[paramID] = param;
setParam(paramID, -1);
@@ -446,12 +453,16 @@ void JaySynth::setControl(int paramID, synth_float_t param)
void JaySynth::setPerVoiceControl(int voice, int channel, int paramID, synth_float_t param)
{
const ScopedLock sl (lock);
pPer_voice_controls[voice].ctrl[channel][paramID] = param;
setParam(paramID, voice);
}
void JaySynth::setParam(int paramID, int voice)
{
const ScopedLock sl (lock);
int i, j, voice_param_type, voice_min, voice_max;
synth_float_t param;
@@ -898,6 +909,9 @@ void JaySynth::handlePitchWheel (const int midiChannel, const int wheelValue)
void JaySynth::handleController (const midiCC_info_t &midiCC_info)
{
if (midiCC_info.ID < 0 || midiCC_info.ID >= NUM_MIDI_CONTROLLERS)
return;
last_midiCC_info[midiCC_info.ID] = midiCC_info;
listeners.call (&JaySynthListener::synthChanged, SYNTH_CHANGED_MIDICC, (midiCC_info_t*)&midiCC_info);
@@ -990,14 +1004,26 @@ void JaySynth::renderNextBlock (AudioSampleBuffer& outputBuffer,
midiIterator.setNextSamplePosition (startSample);
MidiMessage m (0xf4, 0.0);
bool havePendingEvent = false;
int midiEventPos = 0;
while (numSamples > 0)
{
int midiEventPos;
const bool useEvent = midiIterator.getNextEvent (m, midiEventPos)
if (!havePendingEvent)
havePendingEvent = midiIterator.getNextEvent (m, midiEventPos);
const bool useEvent = havePendingEvent
&& midiEventPos < startSample + numSamples;
const int numThisTime = useEvent ? midiEventPos - startSample
: numSamples;
int numThisTime = useEvent ? midiEventPos - startSample
: numSamples;
// The DSP core's per-block work buffers are fixed at SYNTH_MAX_BUFSIZE
// samples, so render oversized ranges in multiple passes rather than
// overrunning them. The pending midi event (if any) stays queued until
// we actually reach its sample position.
if (numThisTime > SYNTH_MAX_BUFSIZE)
numThisTime = SYNTH_MAX_BUFSIZE;
if (numThisTime > 0)
{
@@ -1035,9 +1061,12 @@ void JaySynth::renderNextBlock (AudioSampleBuffer& outputBuffer,
}
}
}
if (useEvent)
// Only fire the pending event once we've actually rendered up to its
// sample position (numThisTime may have been clamped short of it above).
if (useEvent && numThisTime == midiEventPos - startSample)
{
handleMidiEvent (m);
havePendingEvent = false;
}
startSample += numThisTime;