From 0996612557dfa153cba838c12c6e38bc595f327c Mon Sep 17 00:00:00 2001 From: Martijn van Beurden Date: Wed, 27 Mar 2024 13:56:24 +0100 Subject: [PATCH] Improve failure handling when multithreading In case of encoder failure within a thread, asserts where false and a locked mutex would be destroyed. This fix leaves in a race condition: encoder->protected_->state is written without a lock. Credit: Oss-Fuzz Issue: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62650 --- src/libFLAC/stream_encoder.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libFLAC/stream_encoder.c b/src/libFLAC/stream_encoder.c index e8503a6407..b81c8d5328 100644 --- a/src/libFLAC/stream_encoder.c +++ b/src/libFLAC/stream_encoder.c @@ -3413,9 +3413,10 @@ FLAC__bool process_frame_(FLAC__StreamEncoder *encoder, FLAC__bool is_last_block #ifdef HAVE_PTHREAD uint32_t i; #endif - FLAC__ASSERT(encoder->protected_->state == FLAC__STREAM_ENCODER_OK); - if(encoder->protected_->num_threads < 2 || is_last_block) { + + FLAC__ASSERT(encoder->protected_->state == FLAC__STREAM_ENCODER_OK); + /* * Accumulate raw signal to the MD5 signature */ @@ -3535,10 +3536,13 @@ FLAC__bool process_frame_(FLAC__StreamEncoder *encoder, FLAC__bool is_last_block } } /* Task is finished, write bitbuffer */ - if(!encoder->private_->threadtask[encoder->private_->next_thread]->returnvalue) + if(!encoder->private_->threadtask[encoder->private_->next_thread]->returnvalue) { + pthread_mutex_unlock(&encoder->private_->threadtask[encoder->private_->next_thread]->mutex_this_task); return false; + } if(!write_bitbuffer_(encoder, encoder->private_->threadtask[encoder->private_->next_thread], encoder->protected_->blocksize, is_last_block)) { /* the above function sets the state for us in case of an error */ + pthread_mutex_unlock(&encoder->private_->threadtask[encoder->private_->next_thread]->mutex_this_task); return false; } pthread_mutex_unlock(&encoder->private_->threadtask[encoder->private_->next_thread]->mutex_this_task); @@ -3696,7 +3700,7 @@ FLAC__bool process_frame_thread_inner_(FLAC__StreamEncoder * encoder, FLAC__Stre /* * CRC-16 the whole thing */ - FLAC__ASSERT(FLAC__bitwriter_is_byte_aligned(task->frame)); + FLAC__ASSERT(!ok || FLAC__bitwriter_is_byte_aligned(task->frame)); if( ok && (