summaryrefslogtreecommitdiffstats
path: root/video
diff options
context:
space:
mode:
authorSven Kroeger <svenni.kroeger@gmail.com>2020-03-04 12:48:40 +0100
committerwm4 <1387750+wm4@users.noreply.github.com>2020-03-05 18:12:57 +0100
commitfc8c1fcfb2b66d3a05f6d94f8dfffac34207900c (patch)
tree54d881b56897287744863742c9998a7dd5932769 /video
parent8b24510388c7af223ecdfd8ebe242be315424eaf (diff)
downloadmpv-fc8c1fcfb2b66d3a05f6d94f8dfffac34207900c.tar.bz2
mpv-fc8c1fcfb2b66d3a05f6d94f8dfffac34207900c.tar.xz
drm_prime: double free bug
This commit fixes a bug where handle for a framebuffer gets double freed. It seems to happen that the same prime fd gets two framebuffers. As the prime fd is the same the resulting prime handle is also the same. This means one handle but 2 framebuffers and can lead to the following chain: 1. The first framebuffer gets deleted the handle gets also freed via the ioctl. 2. In startup phase not all 4 dumb buffers for overlay drawing are set up. It can happen that the last dumb buffer gets the handle we freed above. 3. The second framebuffer gets freed and the handle will be freed again resulting that the 4's dumb buffer handle is not backed by a buffer. 4. Drm prime continues to assign handles to its prime fds an will lead to have this handle which was just freed to reassign again but to an prime buffer. 5.Now the overlay should be drawn into dumb buffer 4 which still has the same handle but is backed by the wrong buffer. This leads to two different behaviors: - MPV crashes as the drm prime buffers size als calculated by the decoder output format. The overlay output format differs and it takes more space. SO the size check in kernel fails. - MPV is continuing play. This happens when the decoders allocates a bigger buffer than needed for the overlay. For example overlay is Full HD and decoder output is 4k. This leads to the behavior das the overlay wil be drawn into the wrong buffer as its a drm prime buffer and results in a flicker every fourth step.
Diffstat (limited to 'video')
-rw-r--r--video/out/drm_prime.c80
-rw-r--r--video/out/drm_prime.h16
-rw-r--r--video/out/opengl/hwdec_drmprime_drm.c12
3 files changed, 93 insertions, 15 deletions
diff --git a/video/out/drm_prime.c b/video/out/drm_prime.c
index 8eccb529a0..d29837e53d 100644
--- a/video/out/drm_prime.c
+++ b/video/out/drm_prime.c
@@ -24,8 +24,10 @@
#include "drm_common.h"
#include "drm_prime.h"
-int drm_prime_create_framebuffer(struct mp_log *log, int fd, AVDRMFrameDescriptor *descriptor, int width, int height,
- struct drm_prime_framebuffer *framebuffer)
+int drm_prime_create_framebuffer(struct mp_log *log, int fd,
+ AVDRMFrameDescriptor *descriptor, int width,
+ int height, struct drm_prime_framebuffer *framebuffer,
+ struct drm_prime_handle_refs *handle_refs)
{
AVDRMLayerDescriptor *layer = NULL;
uint32_t pitches[4], offsets[4], handles[4];
@@ -36,12 +38,14 @@ int drm_prime_create_framebuffer(struct mp_log *log, int fd, AVDRMFrameDescripto
*framebuffer = (struct drm_prime_framebuffer){0};
for (int object = 0; object < descriptor->nb_objects; object++) {
- ret = drmPrimeFDToHandle(fd, descriptor->objects[object].fd, &framebuffer->gem_handles[object]);
+ ret = drmPrimeFDToHandle(fd, descriptor->objects[object].fd,
+ &framebuffer->gem_handles[object]);
if (ret < 0) {
- mp_err(log, "Failed to retrieve the Prime Handle from handle %d (%d).\n", object, descriptor->objects[object].fd);
+ mp_err(log, "Failed to retrieve the Prime Handle from handle %d (%d).\n",
+ object, descriptor->objects[object].fd);
goto fail;
}
- if(object == 0) {
+ if (object == 0) {
modifiers[object] = descriptor->objects[object].format_modifier;
}
}
@@ -64,12 +68,17 @@ int drm_prime_create_framebuffer(struct mp_log *log, int fd, AVDRMFrameDescripto
}
ret = drmModeAddFB2WithModifiers(fd, width, height, layer->format,
- handles, pitches, offsets, modifiers, &framebuffer->fb_id, DRM_MODE_FB_MODIFIERS);
+ handles, pitches, offsets,
+ modifiers, &framebuffer->fb_id,
+ DRM_MODE_FB_MODIFIERS);
if (ret < 0) {
mp_err(log, "Failed to create framebuffer on layer %d.\n", 0);
goto fail;
}
- }
+ for (int plane = 0; plane < AV_DRM_MAX_PLANES; plane++) {
+ drm_prime_add_handle_ref(handle_refs, framebuffer->gem_handles[plane]);
+ }
+ }
return 0;
@@ -78,15 +87,66 @@ fail:
return -1;
}
-void drm_prime_destroy_framebuffer(struct mp_log *log, int fd, struct drm_prime_framebuffer *framebuffer)
+void drm_prime_destroy_framebuffer(struct mp_log *log, int fd,
+ struct drm_prime_framebuffer *framebuffer,
+ struct drm_prime_handle_refs *handle_refs)
{
if (framebuffer->fb_id)
drmModeRmFB(fd, framebuffer->fb_id);
for (int i = 0; i < AV_DRM_MAX_PLANES; i++) {
- if (framebuffer->gem_handles[i])
- drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &framebuffer->gem_handles[i]);
+ if (framebuffer->gem_handles[i]) {
+ drm_prime_remove_handle_ref(handle_refs,
+ framebuffer->gem_handles[i]);
+ if (!drm_prime_get_handle_ref_count(handle_refs,
+ framebuffer->gem_handles[i])) {
+ drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &framebuffer->gem_handles[i]);
+ }
+ }
}
memset(framebuffer, 0, sizeof(*framebuffer));
}
+
+void drm_prime_init_handle_ref_count(void *talloc_parent,
+ struct drm_prime_handle_refs *handle_refs)
+{
+ handle_refs->handle_ref_count = talloc_zero(talloc_parent, uint32_t);
+ handle_refs->size = 1;
+ handle_refs->ctx = talloc_parent;
+}
+
+void drm_prime_add_handle_ref(struct drm_prime_handle_refs *handle_refs,
+ uint32_t handle)
+{
+ if (handle) {
+ if (handle > handle_refs->size) {
+ handle_refs->size = handle;
+ MP_TARRAY_GROW(handle_refs->ctx, handle_refs->handle_ref_count,
+ handle_refs->size);
+ }
+ handle_refs->handle_ref_count[handle - 1]++;
+ }
+}
+
+void drm_prime_remove_handle_ref(struct drm_prime_handle_refs *handle_refs,
+ uint32_t handle)
+{
+ if (handle) {
+ if (handle <= handle_refs->size &&
+ handle_refs->handle_ref_count[handle - 1])
+ {
+ handle_refs->handle_ref_count[handle - 1]--;
+ }
+ }
+}
+
+uint32_t drm_prime_get_handle_ref_count(struct drm_prime_handle_refs *handle_refs,
+ uint32_t handle)
+{
+ if (handle) {
+ if (handle <= handle_refs->size)
+ return handle_refs->handle_ref_count[handle - 1];
+ }
+ return 0;
+}
diff --git a/video/out/drm_prime.h b/video/out/drm_prime.h
index 0653fdbebf..69acba6061 100644
--- a/video/out/drm_prime.h
+++ b/video/out/drm_prime.h
@@ -27,7 +27,19 @@ struct drm_prime_framebuffer {
uint32_t gem_handles[AV_DRM_MAX_PLANES];
};
+struct drm_prime_handle_refs {
+ uint32_t *handle_ref_count;
+ size_t size;
+ void *ctx;
+};
+
int drm_prime_create_framebuffer(struct mp_log *log, int fd, AVDRMFrameDescriptor *descriptor, int width, int height,
- struct drm_prime_framebuffer *framebuffers);
-void drm_prime_destroy_framebuffer(struct mp_log *log, int fd, struct drm_prime_framebuffer *framebuffers);
+ struct drm_prime_framebuffer *framebuffers,
+ struct drm_prime_handle_refs *handle_refs);
+void drm_prime_destroy_framebuffer(struct mp_log *log, int fd, struct drm_prime_framebuffer *framebuffers,
+ struct drm_prime_handle_refs *handle_refs);
+void drm_prime_init_handle_ref_count(void *talloc_parent, struct drm_prime_handle_refs *handle_refs);
+void drm_prime_add_handle_ref(struct drm_prime_handle_refs *handle_refs, uint32_t handle);
+void drm_prime_remove_handle_ref(struct drm_prime_handle_refs *handle_refs, uint32_t handle);
+uint32_t drm_prime_get_handle_ref_count(struct drm_prime_handle_refs *handle_refs, uint32_t handle);
#endif // DRM_PRIME_H
diff --git a/video/out/opengl/hwdec_drmprime_drm.c b/video/out/opengl/hwdec_drmprime_drm.c
index bc9f548019..66d3497807 100644
--- a/video/out/opengl/hwdec_drmprime_drm.c
+++ b/video/out/opengl/hwdec_drmprime_drm.c
@@ -55,6 +55,8 @@ struct priv {
struct mp_rect src, dst;
int display_w, display_h;
+
+ struct drm_prime_handle_refs handle_refs;
};
static void set_current_frame(struct ra_hwdec *hw, struct drm_frame *frame)
@@ -68,7 +70,7 @@ static void set_current_frame(struct ra_hwdec *hw, struct drm_frame *frame)
// is not being displayed when we release it.
if (p->ctx) {
- drm_prime_destroy_framebuffer(p->log, p->ctx->fd, &p->old_frame.fb);
+ drm_prime_destroy_framebuffer(p->log, p->ctx->fd, &p->old_frame.fb, &p->handle_refs);
}
mp_image_setrefp(&p->old_frame.image, p->last_frame.image);
@@ -183,7 +185,7 @@ static int overlay_frame(struct ra_hwdec *hw, struct mp_image *hw_image,
int dstw = MP_ALIGN_UP(p->dst.x1 - p->dst.x0, 2);
int dsth = MP_ALIGN_UP(p->dst.y1 - p->dst.y0, 2);
- if (drm_prime_create_framebuffer(p->log, p->ctx->fd, desc, srcw, srch, &next_frame.fb)) {
+ if (drm_prime_create_framebuffer(p->log, p->ctx->fd, desc, srcw, srch, &next_frame.fb, &p->handle_refs)) {
ret = -1;
goto fail;
}
@@ -222,7 +224,7 @@ static int overlay_frame(struct ra_hwdec *hw, struct mp_image *hw_image,
return 0;
fail:
- drm_prime_destroy_framebuffer(p->log, p->ctx->fd, &next_frame.fb);
+ drm_prime_destroy_framebuffer(p->log, p->ctx->fd, &next_frame.fb, &p->handle_refs);
return ret;
}
@@ -288,6 +290,10 @@ static int init(struct ra_hwdec *hw)
goto err;
}
+ if (has_prime) {
+ drm_prime_init_handle_ref_count(p, &p->handle_refs);
+ }
+
disable_video_plane(hw);
p->hwctx = (struct mp_hwdec_ctx) {