From owner-freebsd-x11@FreeBSD.ORG Thu Feb 20 07:18:52 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 69FF2F09; Thu, 20 Feb 2014 07:18:52 +0000 (UTC) Received: from ravenloft.kiev.ua (ravenloft.kiev.ua [94.244.131.95]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 38A6D1B3D; Thu, 20 Feb 2014 07:18:51 +0000 (UTC) Date: Thu, 20 Feb 2014 09:18:48 +0200 From: Alex Kozlov To: Robert Millan Subject: Re: [PATCH] Fix double-free conditions in X devd backend Message-ID: <20140220071848.GA1541@ravenloft.kiev.ua> References: <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> <52EFA6D3.3000309@freebsd.org> <52FD558B.2070704@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52FD558B.2070704@freebsd.org> Cc: freebsd-x11@freebsd.org 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, 20 Feb 2014 07:18:52 -0000 On Thu, Feb 13, 2014 at 11:30:19PM +0000, Robert Millan wrote: > 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: > >>> 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! > > isdigit() is more correct in this case (the input is not locale-dependant), > > and also more portable since it is provided on systems with Glibc (e.g. > > Debian GNU/kFreeBSD). I think these patches are fine. >> This patch removes uhid from the hw_types[] list. According to the >> uhid driver description, this driver is only a fallback for devices >> not supported by any other driver. >> >> On my system, the USB keyboard shows up as an uhid device in addition >> to /dev/ukbd0, but the previous devd code misidentified it as a mouse. This is a little more controversial. I've keyboard like this too, so IMHO it's ok to remove uhid device for now. -- Alex