From owner-freebsd-x11@FreeBSD.ORG Thu Feb 20 12:22:49 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 3FC2FB7F for ; Thu, 20 Feb 2014 12:22:49 +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 0294519C3 for ; Thu, 20 Feb 2014 12:22:49 +0000 (UTC) Received: from localhost ([::1]) by master.debian.org with esmtp (Exim 4.80) (envelope-from ) id 1WGSeH-000830-Uy; Thu, 20 Feb 2014 12:22:42 +0000 Message-ID: <5305F391.9030202@freebsd.org> Date: Thu, 20 Feb 2014 12:22:41 +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: Alex Kozlov 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> <52FD558B.2070704@freebsd.org> <20140220071848.GA1541@ravenloft.kiev.ua> In-Reply-To: <20140220071848.GA1541@ravenloft.kiev.ua> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 12:22:49 -0000 On 20/02/2014 07:18, Alex Kozlov wrote: > 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. Thanks. Please note, I've sent them to xorg-devel too: http://lists.x.org/archives/xorg-devel/2014-February/040633.html >>> 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 figured as much, so I left this part out for now ;-) > I've keyboard like this too, so IMHO > it's ok to remove uhid device for now. I'd remove it, but AFAICS it's harmless to keep it, so I won't argue either way about it. -- Robert Millan