Date: Thu, 13 Feb 2014 23:30:19 +0000 From: Robert Millan <rmh@freebsd.org> To: freebsd-x11@freebsd.org Cc: Alex Kozlov <spam@rm-rf.kiev.ua> Subject: Re: [PATCH] Fix double-free conditions in X devd backend Message-ID: <52FD558B.2070704@freebsd.org> In-Reply-To: <52EFA6D3.3000309@freebsd.org> References: <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> <52EFA6D3.3000309@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------020908000508030709070907 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 03/02/2014 14:25, Robert Millan wrote: > On 01/02/2014 23:16, Baptiste Daroussin wrote: >> On Sat, Feb 01, 2014 at 01:39:48AM +0100, Robert Millan wrote: >>> >>> Hi Baptiste, >>> >>> Is the devd backend you wrote for X still maintained? If so, I've fixed a >>> few problems (including a 100% reproducible heap corruption!). Shall I send >>> patches your way? >>> >> >> Yes it is please send the patches to the x11@ mailing list CC me . > > Okay, here's the first one which fixes three conditions that could lead to > double-free: > > - xstrdup(path) before passing it to input_option_new() a second time. This > avoids the potential for double-free when the callee deallocates them. > > - Fix another double-free condition: socket_getline() is expected by its caller > to set **out as a pointer to an allocated block whenever it returns a > non-negative value. Therefore do not free() buf when its strlen() is zero. > > - The routine in wakeup_handler() ends with a "free(line)" so the `line' > variable must not be tampered with. This issue is 100% reproducible and > in my system results in an X server crash each time a mouse/keyboard is > plugged/unplugged! > Rediffed against: http://trillian.chruetertee.ch/ports/browser/trunk/x11-servers/xorg-server/files/extra-devd -- Robert Millan --------------020908000508030709070907 Content-Type: text/x-patch; name="double-free.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="double-free.diff" === modified file 'devd.c' --- devd.c 2014-02-13 23:24:33 +0000 +++ devd.c 2014-02-13 23:26:53 +0000 @@ -278,7 +278,7 @@ device_added(char *line) options = input_option_new(options, "device", path); } #else - add_option(&options, "path", path); + add_option(&options, "path", strdup(path)); add_option(&options, "device", path); add_option(&options, "driver", hw_types[i].xdriver); #endif @@ -399,7 +399,7 @@ socket_getline(int fd, char **out) } buf[sz] = '\0'; - if (sz > 0) + if (sz >= 0) *out = buf; else free(buf); @@ -421,10 +421,10 @@ wakeup_handler(pointer data, int err, po switch(*line) { case DEVD_EVENT_ADD: - device_added(++line); + device_added(line+1); break; case DEVD_EVENT_REMOVE: - device_removed(++line); + device_removed(line+1); break; default: break; --------------020908000508030709070907--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52FD558B.2070704>