diff mbox

[FFmpeg-devel,v3] avdevice/xcbgrab: check return values of xcb query functions

Message ID 20190919154230.19017-1-barsnick@gmx.net
State New
Headers show

Commit Message

Moritz Barsnick Sept. 19, 2019, 3:42 p.m. UTC 
xcb_query_pointer_reply() and xcb_get_geometry_reply() can return NULL
if e.g. the X server closes or the connection is lost. This needs to
be checked in order to cleanly exit, because the returned pointers are
dereferenced later.

Furthermore, their return values need to be free()d, also in error
code paths.

Signed-off-by: Moritz Barsnick <barsnick@gmx.net>
---
To reproduce:
Terminal 1:
$ Xvfb :1 -nolisten tcp -screen 0 800x600x24
Terminal 2:
$ ffmpeg -f x11grab -i :1 -f null -
or rather
$ gdb -ex r --args ffmpeg_g -f x11grab -i :1 -f null -
Then terminate Xvfb while ffmpeg is running.

 libavdevice/xcbgrab.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

--
2.20.1

Comments

Moritz Barsnick Sept. 19, 2019, 3:52 p.m. UTC | #1 
On Thu, Sep 19, 2019 at 17:42:30 +0200, Moritz Barsnick wrote:
> +            av_log(c, AV_LOG_ERROR, "Failed get xcb geometry\n");

D'uh, missing a "to " in there. Will fix, or pusher please do so.
Moritz
diff mbox

Patch

diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index b7e689343e..3fb3c56285 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -326,8 +326,10 @@  static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
         return;

     cursor = xcb_xfixes_get_cursor_image_cursor_image(ci);
-    if (!cursor)
+    if (!cursor) {
+        free(ci);
         return;
+    }

     cx = ci->x - ci->xhot;
     cy = ci->y - ci->yhot;
@@ -404,7 +406,16 @@  static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
         pc  = xcb_query_pointer(c->conn, c->screen->root);
         gc  = xcb_get_geometry(c->conn, c->screen->root);
         p   = xcb_query_pointer_reply(c->conn, pc, NULL);
+        if (!p) {
+            av_log(c, AV_LOG_ERROR, "Failed to query xcb pointer\n");
+            return AVERROR(EIO);
+        }
         geo = xcb_get_geometry_reply(c->conn, gc, NULL);
+        if (!geo) {
+            av_log(c, AV_LOG_ERROR, "Failed get xcb geometry\n");
+            free(p);
+            return AVERROR(EIO);
+        }
     }

     if (c->follow_mouse && p->same_screen)
@@ -537,6 +548,10 @@  static int create_stream(AVFormatContext *s)

     gc  = xcb_get_geometry(c->conn, c->screen->root);
     geo = xcb_get_geometry_reply(c->conn, gc, NULL);
+    if (!geo) {
+        av_log(c, AV_LOG_ERROR, "Failed to get xcb geometry\n");
+        return AVERROR(EIO);
+    }

     if (c->x + c->width > geo->width ||
         c->y + c->height > geo->height) {