Date: Thu, 20 Feb 2014 12:22:41 +0000 From: Robert Millan <rmh@freebsd.org> To: Alex Kozlov <spam@rm-rf.kiev.ua> Cc: freebsd-x11@freebsd.org Subject: Re: [PATCH] Fix double-free conditions in X devd backend Message-ID: <5305F391.9030202@freebsd.org> In-Reply-To: <20140220071848.GA1541@ravenloft.kiev.ua> References: <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> <52EFA6D3.3000309@freebsd.org> <52FD558B.2070704@freebsd.org> <20140220071848.GA1541@ravenloft.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5305F391.9030202>