Message ID | 20210826132953.91955-1-cyeaa@connect.ust.hk |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] libavdevice: Fix a potential deadlock issue on the lock ctx->frame_lock in function get_audio_config | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
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@connect.ust.hk > Bug tracker link: https://trac.ffmpeg.org/ticket/9386\#ticket > > Signed-off-by: Chengfeng Ye <cyeaa@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,
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); 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); } }
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. Depends-on: series-0001 (libavdevice: Fix a potential deadlock issue on the lock ctx->frame_lock in function get_audio_config) Cc: cyeaa@connect.ust.hk Bug tracker link: https://trac.ffmpeg.org/ticket/9386\#ticket Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> --- libavdevice/avfoundation.m | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)