Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Feb 2014 09:18:48 +0200
From:      Alex Kozlov <spam@rm-rf.kiev.ua>
To:        Robert Millan <rmh@freebsd.org>
Cc:        freebsd-x11@freebsd.org
Subject:   Re: [PATCH] Fix double-free conditions in X devd backend
Message-ID:  <20140220071848.GA1541@ravenloft.kiev.ua>
In-Reply-To: <52FD558B.2070704@freebsd.org>
References:  <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> <52EFA6D3.3000309@freebsd.org> <52FD558B.2070704@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140220071848.GA1541>