[FFmpeg-trac] #9386(avdevice:new): Potential error(e.g., resource leak, deadlock) due to the unreleased lock in function avf_read_packet in FFmpeg/libavdevice/avfoundation.m

[FFmpeg]

#9386: Potential error(e.g., resource leak, deadlock) due to the unreleased lock
in function avf_read_packet in FFmpeg/libavdevice/avfoundation.m
-------------------------------------+-------------------------------------
             Reporter:  cyeaa        |                     Type:  defect
               Status:  new          |                 Priority:  normal
            Component:  avdevice     |                  Version:  git-
             Keywords:  deadlock,    |  master
  lock leak                          |               Blocked By:
             Blocking:               |  Reproduced by developer:  0
Analyzed by developer:  0            |
-------------------------------------+-------------------------------------
 Hi,developers,thank you for your checking.The lock **ctx->frame_lock** may
 be not released correctly if the method **avf_read_packe** returns at
 eleven error handling branches(line 1067, line 1071, line 1098, line 1104,
 line 1108, line 1112, line 1131, line 1142, line 1162, line 1172 and line
 1174). 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#L1053

 {{{#!div style="font-size: 80%"
 FFmpeg/libavdevice/avfoundation.m:
   {{{#!c++
   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 avf_read_packet(AVFormatContext *s, AVPacket *pkt)
   {
     AVFContext* ctx = (AVFContext*)s->priv_data;

     do {
         CVImageBufferRef image_buffer;
         CMBlockBufferRef block_buffer;
         lock_frames(ctx);

         if (ctx->current_frame != nil) {
             ...;

             if (image_buffer != nil) {
                 length = (int)CVPixelBufferGetDataSize(image_buffer);
             } else if (block_buffer != nil) {
                 length = (int)CMBlockBufferGetDataLength(block_buffer);
             } else  {
                 return AVERROR(EINVAL); //line 1067，lock leak:
 ctx->frame_lock is not released before return
             }

             if (av_new_packet(pkt, length) < 0) {
                 return AVERROR(EIO); //line 1071，lock leak:
 ctx->frame_lock is not released before return
             }

             ...;

             if (status < 0)
                 return status; //line 1098，lock leak: ctx->frame_lock is
 not released before return
         } else if (ctx->current_audio_frame != nil) {
             CMBlockBufferRef block_buffer =
 CMSampleBufferGetDataBuffer(ctx->current_audio_frame);
             int block_buffer_size         =
 CMBlockBufferGetDataLength(block_buffer);

             if (!block_buffer || !block_buffer_size) {
                 return AVERROR(EIO); //line 1104，lock leak:
 ctx->frame_lock is not released before return
             }

             if (ctx->audio_non_interleaved && block_buffer_size >
 ctx->audio_buffer_size) {
                 return AVERROR_BUFFER_TOO_SMALL; //line 1108，lock leak:
 ctx->frame_lock is not released before return
             }

             if (av_new_packet(pkt, block_buffer_size) < 0) {
                 return AVERROR(EIO); //line 1112，lock leak:
 ctx->frame_lock is not released before return
             }

             ...;

             if (ctx->audio_non_interleaved) {
                 int sample, c, shift, num_samples;

                 OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0,
 pkt->size, ctx->audio_buffer);
                 if (ret != kCMBlockBufferNoErr) {
                     return AVERROR(EIO); //line 1131，lock leak:
 ctx->frame_lock is not released before return
                 }

                 num_samples = pkt->size / (ctx->audio_channels *
 (ctx->audio_bits_per_sample >> 3));

                 // transform decoded frame into output format
                 #define INTERLEAVE_OUTPUT(bps)
                 {
                                         ...;
                     if (!src) return AVERROR(EIO); //line 1142，lock leak:
 ctx->frame_lock is not released before return
                                         ...;
                 }

                                 ...;
             } else {
                 OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0,
 pkt->size, pkt->data);
                 if (ret != kCMBlockBufferNoErr) {
                     return AVERROR(EIO); //line 1162，lock leak:
 ctx->frame_lock is not released before return
                 }
             }

             CFRelease(ctx->current_audio_frame);
             ctx->current_audio_frame = nil;
         } else {
             pkt->data = NULL;
             unlock_frames(ctx);
             if (ctx->observed_quit) {
                 return AVERROR_EOF; //line 1172，lock leak:
 ctx->frame_lock is not released before return
             } else {
                 return AVERROR(EAGAIN); //line 1174，lock leak:
 ctx->frame_lock is not released before return
             }
         }

         unlock_frames(ctx);
     } while (!pkt->data);

     return 0;
   }
   }}}
 }}}

 Fixing suggestion:
 add **unlock_frames(ctx);** at all the above branches.
-- 
Ticket URL: <https://trac.ffmpeg.org/ticket/9386>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker