evdev problems

[Magnus Vigerlöf]

On Wednesday 21 March 2007 00:05, Daniel Stone wrote:
> On Tue, Mar 20, 2007 at 11:53:09PM +0100, Magnus Vigerlöf wrote:
> > Btw, how many are actually working on input-related stuff at the moment?
>
> Just myself and Peter, really.

Ok, I was hoping there would be more, but I guess the best way to increase the 
numbers would be to help out where I can..

> > I'm asking as I've been looking through the code for a few days to see
> > what changes are needed in the wacom-driver to make it hotpluggable. So
> > far I've seen memory-leaks, security holes, old limitations (and maybe
> > some other, smaller things) in the server code, but not very much
> > activity on the list addressing this. Maybe people are working, but I
> > don't see it..
>
> Patches for leaks and pointing out limitations would be welcome; please
> send xorg_security at x.org an email about the security holes.

Hmm.. I should have written 'hole'.. It has been pointed out in the 
wiki/discussions here, so it's really not new; How can the path that are sent 
into the server with the hotplug request (typically device-paths) be ensured 
that they are really something allowed and should be readable by the Xserver 
for the user?

The function configAddDevice [1] allocate a list of InputOption's that is 
never freed. And in NewInputDeviceRequest [2] there will be a leak if there 
are multiple device, name, or identifier options in the request. Not serious 
ones, but I'll get to them in time.
But the most interesting one I found was in the option list functions [3]. The 
function addNewOption2 [4] can replace the values of an existing option in 
the list, but it doesn't free the old values. But there are some values that 
are not allocated on the heap that will be put into these lists as well, so 
those places must be updated before the free is added here.

I think I'll start with the last one unless there are plans on replacing them 
with some other structure..

What's best, to send the patches to the list or register a bug? Other options?

[1] +94 config/config.c
[2] +310 hw/xfree86/common/xf86Xinput.c
[3] hw/xfree86/parser/Flags.c hw/xfree86/common/xf86Option.c
[4] +196 hw/xfree86/parser/Flags.c

I'm also concerned about which memory actually gets freed when the 
remove-request arrives for a hot-plugged device. But I haven't got that far 
yet as the wacom driver is no saint in that part either.

The 'old limits' part is the 20-device limit (inputInfo.numDevices) which is 
increased for every call to xf86ActivateDevice [5,6] but never decreased. 
When it get past 20 FatalError is called even if there's maybe only 4 devices 
in the list... Would it be desireable to have an ever increasing counter or 
is it possible to reuse the ids as soon as they are freed?

[5] +141 hw/xfree86/common/xf86Xinput.c
[6] +83 dix/devices.c

> > I'm interested in making hotplugging work. I'm most interested in the
> > wacom driver, but if getting that driver to work means I have to work on
> > more generic parts I won't shy away from it. My 'only' problem is that
> > I've only looked at the source for a limited time and haven't lurked here
> > long enough to have the big picture, so I'd appreciate a few pointers
> > helping me understand what should happend with this part in the long
> > term.
>
> If you have any questions, please ask, and we'll fill you in to the best
> of our ability.

Ok, first ones :)

There's a proposal on the wiki [7] which outlines the functions that could be 
used for what purpose for input devices. Last edit is Sept '05, so one wonder 
is this is a good reference to use when updating the current device drivers?

Also, the 'PreInit' function [8] is currently mandatory, but the 'UnInit' 
function is never even called (even if it's defined). In a static defined 
config I guess it doesn't really matter, but I like having balanced 
create/destroy calls.. So should we eliminate PreInit or start using UnInit 
(if defined)?

Cheers
  Magnus

[7] http://xorg.freedesktop.org/wiki/XOrgInputDriverSpec
[8] +90 hw/xfree86/common/xf86Xinput.h