[FFmpeg-devel] [PATCH 2/2] libavdevice: Fix a potential deadlock issue on the lock ctx->frame_lock in function avf_read_packet

[Nicolas George]

Chengfeng Ye (12021-08-26):
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] libavdevice: Fix a potential deadlock
> issue on the lock ctx->frame_lock in function avf_read_packet

"libavdevice/avfoundation"

> The problem here is that the lock ctx->frame_lock will become an unreleased lock if the program returns at line 1067, line 1071, line 1098, line 1104, line 1108, line 1112, line 1131, line 1142 and line 1162.

Please wrap the lines below 72 characters.

> 
> Depends-on: series-0001 (libavdevice: Fix a potential deadlock issue on the lock ctx->frame_lock in function get_audio_config)
> Cc: cyeaa at connect.ust.hk
> Bug tracker link: https://trac.ffmpeg.org/ticket/9386\#ticket
> 
> Signed-off-by: Chengfeng Ye <cyeaa at connect.ust.hk>
> ---
>  libavdevice/avfoundation.m | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
> index 0953a8b11e..a461cf7d3b 100644
> --- a/libavdevice/avfoundation.m
> +++ b/libavdevice/avfoundation.m
> @@ -1067,10 +1067,12 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt)
>              } else if (block_buffer != nil) {
>                  length = (int)CMBlockBufferGetDataLength(block_buffer);
>              } else  {

> +				unlock_frames(ctx);

Tabs are not allowed.

>                  return AVERROR(EINVAL);
>              }
>  
>              if (av_new_packet(pkt, length) < 0) {
> +				unlock_frames(ctx);
>                  return AVERROR(EIO);
>              }
>  
> @@ -1097,21 +1099,26 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt)
>              CFRelease(ctx->current_frame);
>              ctx->current_frame = nil;
>  
> -            if (status < 0)
> +            if (status < 0) {
> +				unlock_frames(ctx);
>                  return status;
> +			}
>          } 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) {
> +				unlock_frames(ctx);
>                  return AVERROR(EIO);
>              }
>  
>              if (ctx->audio_non_interleaved && block_buffer_size > ctx->audio_buffer_size) {
> +				unlock_frames(ctx);
>                  return AVERROR_BUFFER_TOO_SMALL;
>              }
>  
>              if (av_new_packet(pkt, block_buffer_size) < 0) {
> +				unlock_frames(ctx);
>                  return AVERROR(EIO);
>              }
>  
> @@ -1131,6 +1138,7 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt)
>  
>                  OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, ctx->audio_buffer);
>                  if (ret != kCMBlockBufferNoErr) {
> +					unlock_frames(ctx);
>                      return AVERROR(EIO);
>                  }
>  
> @@ -1140,9 +1148,12 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt)
>                  #define INTERLEAVE_OUTPUT(bps)                                         \
>                  {                                                                      \
>                      int##bps##_t **src;                                                \
> -                    int##bps##_t *dest;                                                \
> +                    int##bps##_t *dest;												   \
>                      src = av_malloc(ctx->audio_channels * sizeof(int##bps##_t*));      \
> -                    if (!src) return AVERROR(EIO);                                     \
> +                    if (!src) {														   \
> +						unlock_frames(ctx);                                            \
> +						return AVERROR(EIO);										   \
> +					}																   \
>                      for (c = 0; c < ctx->audio_channels; c++) {                        \
>                          src[c] = ((int##bps##_t*)ctx->audio_buffer) + c * num_samples; \
>                      }                                                                  \
> @@ -1162,6 +1173,7 @@ static int avf_read_packet(AVFormatContext *s, AVPacket *pkt)
>              } else {
>                  OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, pkt->data);
>                  if (ret != kCMBlockBufferNoErr) {
> +					unlock_frames(ctx);
>                      return AVERROR(EIO);
>                  }
>              }

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210826/d2f99175/attachment.sig>