diff mbox

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

Message ID 20180626154225.8455-1-barsnick@gmx.net
State Superseded
Headers show

Commit Message

Moritz Barsnick June 26, 2018, 3:42 p.m. UTC 
xcb_query_pointer_reply() and xcb_get_geometry_reply() return NULL
when the display drops. This needs to be checked in order to cleanly
exit, because the returned pointers are dereferenced later.

Signed-off-by: Moritz Barsnick <barsnick@gmx.net>
---
 libavdevice/xcbgrab.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Carl Eugen Hoyos June 26, 2018, 9:01 p.m. UTC | #1 
2018-06-26 17:42 GMT+02:00, Moritz Barsnick <barsnick@gmx.net>:

> @@ -425,8 +435,9 @@ static int xcbgrab_read_packet
>          xcbgrab_draw_mouse(s, pkt, p, geo);
>  #endif
>
> -    free(p);
> -    free(geo);
> +fail:
> +    av_free(p);
> +    av_free(geo);

I suspect this is incorrect, if it is correct, it should be a separate patch.

Thank you, Carl Eugen
Moritz Barsnick June 27, 2018, 7:15 a.m. UTC | #2 
On Tue, Jun 26, 2018 at 23:01:16 +0200, Carl Eugen Hoyos wrote:
> 2018-06-26 17:42 GMT+02:00, Moritz Barsnick <barsnick@gmx.net>:
> > +fail:
> > +    av_free(p);
> > +    av_free(geo);
> 
> I suspect this is incorrect, if it is correct, it should be a separate patch.

I agree. There were two misconceptions of mine in there, and a further
buglet. Patch drop, V2 to follow.

Thanks,
Moritz
diff mbox

Patch

diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index 6d142abd4f..609445918b 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -404,7 +404,17 @@  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, "Could not get xcb pointer\n");
+            ret = AVERROR(EIO);
+            goto fail;
+        }
         geo = xcb_get_geometry_reply(c->conn, gc, NULL);
+        if (!geo) {
+            av_log(c, AV_LOG_ERROR, "Could not get xcb geometry\n");
+            ret = AVERROR_EOF;
+            goto fail;
+        }
     }
 
     if (c->follow_mouse && p->same_screen)
@@ -425,8 +435,9 @@  static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
         xcbgrab_draw_mouse(s, pkt, p, geo);
 #endif
 
-    free(p);
-    free(geo);
+fail:
+    av_free(p);
+    av_free(geo);
 
     return ret;
 }
@@ -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, "Could not get xcb geometry\n");
+        return AVERROR(EIO);
+    }
 
     if (c->x + c->width > geo->width ||
         c->y + c->height > geo->height) {