render api: fix use-after-free

render api needs to wait for vo to be destroyed before frees the context.
The purpose of kill_cb is to wake up render api after vo is destroyed,
but uninit did that before kill_cb, so kill_cb tries using the freed
memory. Remove kill_cb to fix the issue as uninit is able to do the
work.
This commit is contained in:
wnoun 2019-05-26 16:58:25 +08:00 committed by wm4
parent db09d77e46
commit 35da5a4d8e
3 changed files with 6 additions and 33 deletions

View File

@ -1817,16 +1817,9 @@ int64_t mpv_get_time_us(mpv_handle *ctx)
#include "video/out/libmpv.h"
struct kill_ctx {
struct MPContext *mpctx;
void (*fin)(void *ctx);
void *fin_ctx;
};
static void do_kill(void *ptr)
{
struct kill_ctx *k = ptr;
struct MPContext *mpctx = k->mpctx;
struct MPContext *mpctx = ptr;
struct track *track = mpctx->vo_chain ? mpctx->vo_chain->track : NULL;
uninit_video_out(mpctx);
@ -1834,22 +1827,13 @@ static void do_kill(void *ptr)
mpctx->error_playing = MPV_ERROR_VO_INIT_FAILED;
error_on_track(mpctx, track);
}
k->fin(k->fin_ctx);
}
// Used by vo_libmpv to (a)synchronously uninitialize video.
void kill_video_async(struct mp_client_api *client_api, void (*fin)(void *ctx),
void *fin_ctx)
void kill_video_async(struct mp_client_api *client_api)
{
struct MPContext *mpctx = client_api->mpctx;
struct kill_ctx *k = talloc_ptrtype(NULL, k);
*k = (struct kill_ctx){
.mpctx = mpctx,
.fin = fin,
.fin_ctx = fin_ctx,
};
mp_dispatch_enqueue_autofree(mpctx->dispatch, do_kill, k);
mp_dispatch_enqueue(mpctx->dispatch, do_kill, mpctx);
}
// Used by vo_libmpv to set the current render context.

View File

@ -49,8 +49,7 @@ bool mp_set_main_render_context(struct mp_client_api *client_api,
struct mpv_render_context *ctx, bool active);
struct mpv_render_context *
mp_client_api_acquire_render_context(struct mp_client_api *ca);
void kill_video_async(struct mp_client_api *client_api, void (*fin)(void *ctx),
void *fin_ctx);
void kill_video_async(struct mp_client_api *client_api);
bool mp_streamcb_lookup(struct mpv_global *g, const char *protocol,
void **out_user_data, mpv_stream_cb_open_ro_fn *out_fn);

View File

@ -245,16 +245,6 @@ void mp_render_context_set_control_callback(mpv_render_context *ctx,
pthread_mutex_unlock(&ctx->control_lock);
}
static void kill_cb(void *ptr)
{
struct mpv_render_context *ctx = ptr;
pthread_mutex_lock(&ctx->update_lock);
ctx->had_kill_update = true;
pthread_cond_broadcast(&ctx->update_cond);
pthread_mutex_unlock(&ctx->update_lock);
}
void mpv_render_context_free(mpv_render_context *ctx)
{
if (!ctx)
@ -269,7 +259,7 @@ void mpv_render_context_free(mpv_render_context *ctx)
// context. The above removal guarantees it can't come back (so ctx->vo
// can't change to non-NULL).
if (atomic_load(&ctx->in_use)) {
kill_video_async(ctx->client_api, kill_cb, ctx);
kill_video_async(ctx->client_api);
while (atomic_load(&ctx->in_use)) {
// As long as the video decoders are not destroyed, they can still
@ -279,7 +269,7 @@ void mpv_render_context_free(mpv_render_context *ctx)
if (ctx->dispatch)
mp_dispatch_queue_process(ctx->dispatch, 0);
// Wait for kill_cb() or update() calls.
// Wait for update() calls.
pthread_mutex_lock(&ctx->update_lock);
if (!ctx->had_kill_update)
pthread_cond_wait(&ctx->update_cond, &ctx->update_lock);