Opened 3 years ago

Closed 3 years ago

Last modified 20 months ago

#9385 closed defect (fixed)

Potential error(e.g., resource leak, deadlock) due to the unreleased lock in function get_audio_config in FFmpeg/libavdevice/avfoundation.m

Reported by: cyeaa Owned by:
Priority: minor Component: avdevice
Version: git-master Keywords: avfoundation
Cc: cyeaa Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Hi,developers,thank you for your checking.The lock ctx->frame_lock may be not released correctly if the method get_audio_config returns at three error handling branches(line 697, line 735, line744). The relevant code is listed below and the problematic place is commented.

source code link:
https://github.com/FFmpeg/FFmpeg/blob/master/libavdevice/avfoundation.m#L686

FFmpeg/libavdevice/avfoundation.m:

  static void lock_frames(AVFContext* ctx)
  {
        pthread_mutex_lock(&ctx->frame_lock);
  }

  static void unlock_frames(AVFContext* ctx)
  {
        pthread_mutex_unlock(&ctx->frame_lock);
  }
  
  static int get_audio_config(AVFormatContext *s)
  {
        ...;
        
    lock_frames(ctx); 

    if (!basic_desc) {
        av_log(s, AV_LOG_ERROR, "audio format not available\n");
        return 1; //line 697,lock leak: ctx->frame_lock is not released before return
    }

    ...;
    else {
        av_log(s, AV_LOG_ERROR, "audio format is not supported\n");
        return 1; //line 735, lock leak: ctx->frame_lock is not released before return
    }

    if (ctx->audio_non_interleaved) {
        CMBlockBufferRef block_buffer = CMSampleBufferGetDataBuffer(ctx->current_audio_frame);
        ctx->audio_buffer_size        = CMBlockBufferGetDataLength(block_buffer);
        ctx->audio_buffer             = av_malloc(ctx->audio_buffer_size);
        if (!ctx->audio_buffer) {
            av_log(s, AV_LOG_ERROR, "error allocating audio buffer\n");
            return 1; //line 744, lock leak: ctx->frame_lock is not released before return
        }
    }

    CFRelease(ctx->current_audio_frame);
    ctx->current_audio_frame = nil;

    unlock_frames(ctx);

    return 0;
  }

Fixing suggestion:
add unlock_frames(ctx); at all the above three branches.

Change History (2)

comment:1 by Thilo Borgmann, 3 years ago

Resolution: fixed
Status: newclosed

comment:2 by Carl Eugen Hoyos, 20 months ago

Keywords: avfoundation added
Note: See TracTickets for help on using tickets.