Re: [RFC PATCH 0/8] Listening to keyboard in background, RawInput and DInput​

[Rémi Bernon]

On 9/11/19 5:56 PM, Derek Lesho wrote:
> On 9/11/19 9:30 AM, Rémi Bernon wrote:
> 
>> Hi all,
>>
>> I'm sending out this as follow-up of my previous e-mail [1] about dinput
>> and background input processing, hopefully to get some comments and to
>> discuss about the idea and the implementation.
>>
>> The patch series also depends on some additional dinput8 patches I
>> already sent to the mailing list, to implement dinput8 devices on top of
>> raw input, but I think they can be independent.
>>
>> There's a few points I believe that may be of concern, the main one
>> being a privacy issue. Allowing Wine applications to listen to input
>> while in background would allow keyloggers to work in Wine. One could
>> argue though that this implementation does not use any priviledged
>> method for that, and that Xlib allows any application to listen to raw
>> input events in background. I guess there may be some ways to get an
>> exclusive device access, to prevent other apps to get the events but
>> I didn't investigate this part.
>>
>> Also, this is currently only used for Wine dinput8 implementation, so
>> we could also add some internal mechanism to enforce only Wine dinput8
>> to be able to access these events.
>>
>> [1] https://www.winehq.org/pipermail/wine-devel/2019-August/149941.html
>>
>> -- 
>> Rémi Bernon <rbernon at codeweavers.com>
>>
>> Derek Lesho (1):
>>    user32: Add support for RIDEV_NOLEGACY flag.
>>
>> Rémi Bernon (7):
>>    server: Add send_hardware_message flags to hint raw input translation.
>>    user32: Add __wine_send_input flags to hint raw input translation.
>>    server: Implement raw mouse/keyboard input message broadcast.
>>    user32: Add support for RIDEV_INPUTSINK flag.
>>    winex11.drv: Listen to XInput2 master devices only.
>>    winex11.drv: Listen to RawMotion and RawButton* events in the desktop
>>      thread.
>>    winex11.drv: Listen to RawKey* events in the desktop thread.
>>
>>   dlls/user32/input.c             |   4 +-
>>   dlls/user32/rawinput.c          |   9 ++-
>>   dlls/user32/tests/rawinput.c    |   2 -
>>   dlls/user32/user32.spec         |   2 +-
>>   dlls/wineandroid.drv/keyboard.c |   2 +-
>>   dlls/wineandroid.drv/window.c   |   4 +-
>>   dlls/winemac.drv/ime.c          |   4 +-
>>   dlls/winemac.drv/keyboard.c     |   2 +-
>>   dlls/winemac.drv/mouse.c        |   2 +-
>>   dlls/winex11.drv/event.c        |  42 +---------
>>   dlls/winex11.drv/keyboard.c     |  42 +++++++++-
>>   dlls/winex11.drv/mouse.c        | 131 +++++++++++++++++++-------------
>>   dlls/winex11.drv/x11drv.h       |   7 +-
>>   dlls/winex11.drv/x11drv_main.c  |   5 ++
>>   include/winuser.h               |   2 +-
>>   server/protocol.def             |   3 +
>>   server/queue.c                  | 106 +++++++++++++++++++++-----
>>   17 files changed, 236 insertions(+), 133 deletions(-)
>>
>> -- 
>> 2.23.0
>>
>>
> Nice work, this patch-set looks much tidier than mine.  I have a few 
> concerns:
> 
> 1) This patch-set breaks XWayland support, you can see how I worked 
> around this in commit 6709734 on my rawinput tree on github.
> <https://github.com/Guy1524/wine/commit/670973417d576dec16c610f2fde25350a9638dca> 
> 

I think this could go in a separate "absolute raw input support" patch 
somehow - although maybe environment-specific hacks aren't very welcome 
- and it doesn't necessarily require these patch series either.

> 2) Without support for absolute raw-input, there's a regression in OSU 
> on touch tablets.  To fix this we need to add support for absolute 
> raw-input.  openglfreak on the VKx discord already did a bunch of 
> testing for how this behaves on windows, and I've incorporated it into 
> commit 2fade59 of my rawinput tree on github.

I think that unless you add absolute RawMotion valuators support the 
events from the tablets will be discarded, and the input will come from 
the MotionNotify event codepath which should be unmodified. As far as I 
can tell, osu! is still working fine with these patches.

Absolute RawMotion events could be useful, but it's not currently 
implemented and I think it can be implemented separately.

> 3) Without DeviceChanged being used, there may be some weird 
> interactions with the ckb-next driver for corsair mice.  (I am not able 
> to reproduce this, you'll probably want https://github.com/sur3 to test 
> this patch-set)

Yes, I went into issues when dropping these as well, I believe they are 
still subscribed to, in these patches, or they should be.
-- 
Rémi Bernon <rbernon at codeweavers.com>