Opened 5 years ago
Closed 5 years ago
#8229 closed defect (invalid)
A potential Use-After-Free bug in the source file libavfilter/vf_hwmap.c
Reported by: | wurongxin | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | undetermined |
Version: | git-master | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
Summary of the bug:
How to reproduce:
% ffmpeg -i input ... output ffmpeg version built on ...
Patches should be submitted to the ffmpeg-devel mailing list and not this bug tracker.
In the source file https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/vf_hwmap.c, at Line 114, the call to "av_hwframe_ctx_create_derived" would free device->buffer, and device->buffer would be used later at Line 259. This lead to a use-after-free bug.
53. static int hwmap_config_output(AVFilterLink *outlink) 54. { … 114. err = av_hwframe_ctx_create_derived(&ctx->hwframes_ref, 115. outlink->format, 116. device, 117. inlink->hw_frames_ctx, 118. ctx->mode); 119. if (err < 0) { 120. av_log(avctx, AV_LOG_ERROR, "Failed to create derived " 121. "frames context: %d.\n", err); 122. goto fail; 123. } … 257. fail: 258. if (device_is_derived) 259. av_buffer_unref(&device); 260. av_buffer_unref(&ctx->hwframes_ref); 261. return err;
To see how "av_hwframe_ctx_create_derived" would free device->buffer, please see the following code snippet in the source file libavutil/hwcontext.c. At Line 828, the copy of the variable derived_device_ctx will be created and assigned to the variable dst_ref. Since this copy is a shallow copy, dst_ref->buffer is actually the same memory address as derived_device_ctx->buffer. At Line 870, dst_ref->buffer can be freed when calling to the function av_buffer_unref.
798. int av_hwframe_ctx_create_derived(AVBufferRef **derived_frame_ctx, 799. enum AVPixelFormat format, 800. AVBufferRef *derived_device_ctx, 801. AVBufferRef *source_frame_ctx, 802. int flags) 803. { … 828. dst_ref = av_hwframe_ctx_alloc(derived_device_ctx); … 867. fail: 868. if (dst) 869. av_buffer_unref(&dst->internal->source_frames); 870. av_buffer_unref(&dst_ref); 871. return ret;
The code snippet about the function av_buffer_unref is shown as follows.
void av_buffer_unref(AVBufferRef **buf) { if (!buf || !*buf) return; buffer_replace(buf, NULL); } static void buffer_replace(AVBufferRef **dst, AVBufferRef **src) { AVBuffer *b; b = (*dst)->buffer; if (src) { **dst = **src; av_freep(src); } else av_freep(dst); if (atomic_fetch_add_explicit(&b->refcount, -1, memory_order_acq_rel) == 1) { b->free(b->opaque, b->data); av_freep(&b); } }
Change History (2)
comment:1 by , 5 years ago
Priority: | normal → critical |
---|
comment:2 by , 5 years ago
Component: | avfilter → undetermined |
---|---|
Priority: | critical → normal |
Resolution: | → invalid |
Status: | new → closed |
No.
a) If the call to av_hwframe_ctx_alloc() fails, the reference counter of the underlying AVBuffer of derived_device_ctx is the same as before the call and dst_ref is NULL, so that av_buffer_unref(&dst_ref) is basically a no-op.
b) If the call to av_hwframe_ctx_alloc() succeeds, the reference counter to derived_device_ctx has been incremented by 1. Should we goto fail in av_hwframe_ctx_create_derived() lateron, dst_ref will be unreferenced; given that the reference counter of the underlying AVBuffer (which is different from the underlying AVBuffer of derived_device_ctx) is certain to be 1 at this point, this will trigger freeing of the underlying buffer via hwframe_ctx_free(). This in turn will unreference ((AVHWFramesContext*) dst_ref->buffer)->device_ref and therefore decrement the reference counter of the underlying buffer of derived_device_ctx, but it will not free this buffer; it will just undo the earlier increment.