From owner-freebsd-x11@FreeBSD.ORG Thu Feb 13 23:30:22 2014 Return-Path: Delivered-To: freebsd-x11@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AEA59B21 for ; Thu, 13 Feb 2014 23:30:22 +0000 (UTC) Received: from master.debian.org (master.debian.org [IPv6:2001:41b8:202:deb:216:36ff:fe40:4001]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 6F9081110 for ; Thu, 13 Feb 2014 23:30:22 +0000 (UTC) Received: from localhost ([::1]) by master.debian.org with esmtp (Exim 4.80) (envelope-from ) id 1WE5jY-0003wn-96; Thu, 13 Feb 2014 23:30:20 +0000 Message-ID: <52FD558B.2070704@freebsd.org> Date: Thu, 13 Feb 2014 23:30:19 +0000 From: Robert Millan User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: freebsd-x11@freebsd.org Subject: Re: [PATCH] Fix double-free conditions in X devd backend References: <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> <52EFA6D3.3000309@freebsd.org> In-Reply-To: <52EFA6D3.3000309@freebsd.org> X-Enigmail-Version: 1.6 Content-Type: multipart/mixed; boundary="------------020908000508030709070907" Cc: Alex Kozlov X-BeenThere: freebsd-x11@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: X11 on FreeBSD -- maintaining and support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Feb 2014 23:30:22 -0000 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--