zdoom/fluidsynth
0
0
Fork 0
mirror of https://github.com/ZDoom/fluidsynth.git synced 2024-11-30 07:51:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix use-after-free in fluid_player_stop()

Browse source 
Previously, sample timers were deleted in fluid_player_stop() which caused a use-after-free when at the same time the sample timers were advanced by the synthesizer thread. This was incorrectly addressed in 5d3f727547 . Deleting sample timers is now done in delete_fluid_player(). A broken application could still crash if it does not respect the order of object creation though. At least now, this issue is properly documented.
This commit is contained in:
derselbst 2019-08-15 15:54:00 +02:00
parent f70a6321c5
commit df893bbfa4
4 changed files with 68 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/drivers/fluid_adriver.c
View file

@ -253,8 +253,14 @@ find_fluid_audio_driver(fluid_settings_t *settings)
* @param synth Synthesizer instance for which the audio driver is created for.
* @return The new audio driver instance.
*
* Creates a new audio driver for a given 'synth' instance with a defined set
* of configuration 'settings'.
* Creates a new audio driver for a given \p synth instance with a defined set
* of configuration \p settings.
*
* @note As soon as an audio driver is created, the \p synth starts rendering audio.
* This means that all necessary sound-setup should be completed after this point,
* thus of all object types in use (synth, midi player, sequencer, etc.) the audio
* driver should always be the last one to be created and the first one to be deleted!
* Also refer to the order of object creation in the code examples.
*/
fluid_audio_driver_t *
new_fluid_audio_driver(fluid_settings_t *settings, fluid_synth_t *synth)
@ -286,9 +292,15 @@ new_fluid_audio_driver(fluid_settings_t *settings, fluid_synth_t *synth)
*
* Like new_fluid_audio_driver() but allows for custom audio processing before
* audio is sent to audio driver. It is the responsibility of the callback
* 'func' to render the audio into the buffers.
* \p func to render the audio into the buffers.
*
* NOTE: Not as efficient as new_fluid_audio_driver().
* @note Not as efficient as new_fluid_audio_driver().
*
* @note As soon as an audio driver is created, the \p synth starts rendering audio.
* This means that all necessary sound-setup should be completed after this point,
* thus of all object types in use (synth, midi player, sequencer, etc.) the audio
* driver should always be the last one to be created and the first one to be deleted!
* Also refer to the order of object creation in the code examples.
*/
fluid_audio_driver_t *
new_fluid_audio_driver2(fluid_settings_t *settings, fluid_audio_func_t func, void *data)

96
src/midi/fluid_midi.c
View file

@ -48,7 +48,7 @@ static fluid_midi_event_t *fluid_track_next_event(fluid_track_t *track);
static int fluid_track_get_duration(fluid_track_t *track);
static int fluid_track_reset(fluid_track_t *track);
static int fluid_track_send_events(fluid_track_t *track,
static void fluid_track_send_events(fluid_track_t *track,
fluid_synth_t *synth,
fluid_player_t *player,
unsigned int ticks);
@ -1552,13 +1552,12 @@ fluid_track_reset(fluid_track_t *track)
/*
* fluid_track_send_events
*/
int
void
fluid_track_send_events(fluid_track_t *track,
fluid_synth_t *synth,
fluid_player_t *player,
unsigned int ticks)
{
int status = FLUID_OK;
fluid_midi_event_t *event;
int seeking = player->seek_ticks >= 0;
@ -1579,7 +1578,7 @@ fluid_track_send_events(fluid_track_t *track,
if(event == NULL)
{
return status;
return;
}
/* printf("track=%02d\tticks=%05u\ttrack=%05u\tdtime=%05u\tnext=%05u\n", */
@ -1591,7 +1590,7 @@ fluid_track_send_events(fluid_track_t *track,
if(track->ticks + event->dtime > ticks)
{
return status;
return;
}
track->ticks += event->dtime;
@ -1619,8 +1618,6 @@ fluid_track_send_events(fluid_track_t *track,
fluid_track_next_event(track);
}
return status;
}
/******************************************************
@ -1678,6 +1675,26 @@ new_fluid_player(fluid_synth_t *synth)
fluid_player_set_playback_callback(player, fluid_synth_handle_midi_event, synth);
player->use_system_timer = fluid_settings_str_equal(synth->settings,
"player.timing-source", "system");
if(player->use_system_timer)
{
player->system_timer = new_fluid_timer((int) player->deltatime,
fluid_player_callback, player, TRUE, FALSE, TRUE);
if(player->system_timer == NULL)
{
goto err;
}
}
else
{
player->sample_timer = new_fluid_sample_timer(player->synth,
fluid_player_callback, player);
if(player->sample_timer == NULL)
{
goto err;
}
}
fluid_settings_getint(synth->settings, "player.reset-synth", &i);
fluid_player_handle_reset_synth(player, NULL, i);
@ -1686,11 +1703,16 @@ new_fluid_player(fluid_synth_t *synth)
fluid_player_handle_reset_synth, player);
return player;
err:
delete_fluid_player(player);
return NULL;
}
/**
* Delete a MIDI player instance.
* @param player MIDI player instance
* @warning Do not call while the \p synth renders audio, i.e. an audio driver is running or any other synthesizer thread calls fluid_synth_process() or fluid_synth_nwrite_float() or fluid_synth_write_*() !
*/
void
delete_fluid_player(fluid_player_t *player)
@ -1703,6 +1725,9 @@ delete_fluid_player(fluid_player_t *player)
fluid_player_stop(player);
fluid_player_reset(player);
delete_fluid_timer(player->system_timer);
delete_fluid_sample_timer(player->synth, player->sample_timer);
while(player->playlist != NULL)
{
q = player->playlist->next;
@ -2030,6 +2055,11 @@ fluid_player_callback(void *data, unsigned int msec)
loadnextfile = player->currentfile == NULL ? 1 : 0;
if(player->status == FLUID_PLAYER_READY)
{
fluid_synth_all_notes_off(synth, -1);
return 1;
}
do
{
if(loadnextfile)
@ -2058,12 +2088,7 @@ fluid_player_callback(void *data, unsigned int msec)
if(!fluid_track_eot(player->track[i]))
{
status = FLUID_PLAYER_PLAYING;
if(fluid_track_send_events(player->track[i], synth, player,
player->cur_ticks) != FLUID_OK)
{
/* */
}
fluid_track_send_events(player->track[i], synth, player, player->cur_ticks);
}
}
@ -2098,42 +2123,21 @@ fluid_player_callback(void *data, unsigned int msec)
int
fluid_player_play(fluid_player_t *player)
{
if(player->status == FLUID_PLAYER_PLAYING)
if(player->status == FLUID_PLAYER_PLAYING ||
player->playlist == NULL)
{
return FLUID_OK;
}
if(player->playlist == NULL)
if(!player->use_system_timer)
{
return FLUID_OK;
fluid_sample_timer_reset(player->synth, player->sample_timer);
}
player->status = FLUID_PLAYER_PLAYING;
if(player->use_system_timer)
{
player->system_timer = new_fluid_timer((int) player->deltatime,
fluid_player_callback, (void *) player, TRUE, FALSE, TRUE);
if(player->system_timer == NULL)
{
return FLUID_FAILED;
}
}
else
{
player->sample_timer = new_fluid_sample_timer(player->synth,
fluid_player_callback, (void *) player);
if(player->sample_timer == NULL)
{
return FLUID_FAILED;
}
}
return FLUID_OK;
}
/**
* Stops a MIDI player.
* @param player MIDI player instance
@ -2142,19 +2146,7 @@ fluid_player_play(fluid_player_t *player)
int
fluid_player_stop(fluid_player_t *player)
{
if(player->system_timer != NULL)
{
delete_fluid_timer(player->system_timer);
}
if(player->sample_timer != NULL)
{
delete_fluid_sample_timer(player->synth, player->sample_timer);
}
player->status = FLUID_PLAYER_DONE;
player->sample_timer = NULL;
player->system_timer = NULL;
player->status = FLUID_PLAYER_READY;
return FLUID_OK;
}
@ -2254,7 +2246,7 @@ fluid_player_join(fluid_player_t *player)
else if(player->sample_timer)
{
/* Busy-wait loop, since there's no thread to wait for... */
while(player->status != FLUID_PLAYER_DONE)
while(player->status == FLUID_PLAYER_PLAYING)
{
fluid_msleep(10);
}

13
src/synth/fluid_synth.c
View file

@ -494,16 +494,13 @@ struct _fluid_sample_timer_t
*/
static void fluid_sample_timer_process(fluid_synth_t *synth)
{
fluid_sample_timer_t *st, *stnext;
fluid_sample_timer_t *st;
long msec;
int cont;
unsigned int ticks = fluid_synth_get_ticks(synth);
for(st = synth->sample_timers; st; st = stnext)
for(st = synth->sample_timers; st; st = st->next)
{
/* st may be freed in the callback below. cache it's successor now to avoid use after free */
stnext = st->next;
if(st->isfinished)
{
continue;
@ -529,7 +526,7 @@ fluid_sample_timer_t *new_fluid_sample_timer(fluid_synth_t *synth, fluid_timer_c
return NULL;
}
result->starttick = fluid_synth_get_ticks(synth);
fluid_sample_timer_reset(synth, result);
result->isfinished = 0;
result->data = data;
result->callback = callback;
@ -559,6 +556,10 @@ void delete_fluid_sample_timer(fluid_synth_t *synth, fluid_sample_timer_t *timer
}
}
void fluid_sample_timer_reset(fluid_synth_t *synth, fluid_sample_timer_t *timer)
{
timer->starttick = fluid_synth_get_ticks(synth);
}
/***************************************************************
*

2
src/synth/fluid_synth.h
View file

@ -205,7 +205,7 @@ int fluid_synth_set_chorus_full(fluid_synth_t *synth, int set, int nr, double le
fluid_sample_timer_t *new_fluid_sample_timer(fluid_synth_t *synth, fluid_timer_callback_t callback, void *data);
void delete_fluid_sample_timer(fluid_synth_t *synth, fluid_sample_timer_t *timer);
void fluid_sample_timer_reset(fluid_synth_t *synth, fluid_sample_timer_t *timer);
void fluid_synth_process_event_queue(fluid_synth_t *synth);