| Message ID | 20200710191327.GB16321@sunshine.barsnick.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v4] avdevice/xcbgrab: check return values of xcb query functions | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Fri, 10. Jul 21:13, Moritz Barsnick wrote: > Since xcbgrab is getting some attention recently... > > Fixes a segfault, as reported in #7312. > > 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. > > Cheers, > Moritz > From 3bbf40dd08bb67e993cca97880aec032644fd02b Mon Sep 17 00:00:00 2001 > From: Moritz Barsnick <barsnick@gmx.net> > Date: Fri, 10 Jul 2020 13:26:55 +0200 > Subject: [PATCH] avdevice/xcbgrab: check return values of xcb query functions > > Fixes #7312, segmentation fault on close of X11 server > > 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> > --- > libavdevice/xcbgrab.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c > index 6f6b2dbf15..be4e0d14f9 100644 > --- a/libavdevice/xcbgrab.c > +++ b/libavdevice/xcbgrab.c > @@ -346,8 +346,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; > @@ -425,7 +427,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_EXTERNAL; > + } > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > + if (!geo) { > + av_log(c, AV_LOG_ERROR, "Failed to get xcb geometry\n"); The rest of the av_log calls use AVFormatContext*. You may want to update to be consistent. > + free(p); > + return AVERROR_EXTERNAL; > + } > } Thanks,
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c index 6f6b2dbf15..be4e0d14f9 100644 --- a/libavdevice/xcbgrab.c +++ b/libavdevice/xcbgrab.c @@ -346,8 +346,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; @@ -425,7 +427,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_EXTERNAL; + } geo = xcb_get_geometry_reply(c->conn, gc, NULL); + if (!geo) { + av_log(c, AV_LOG_ERROR, "Failed to get xcb geometry\n"); + free(p); + return AVERROR_EXTERNAL; + } } if (c->follow_mouse && p->same_screen)