Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keypresses leak to the console with 2.0.5 (didn't happen with 2.0.4) #2287

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.5
Reported for operating system, platform: Linux, ARM

Comments on the original bug report:

On 2016-10-25 09:52:30 +0000, Manuel Alfayate Corchete wrote:

On the Raspberry Pi platform, where SDL2 runs in console mode (using dispmanx + GLES renderer), keypresses during the game/app runtime now appear on the console. They did not appear with stable 2.0.4.
It's a critical issue because, for example, pressing the up arrow + enter inside the game/app runs commands that the user ran before launching that game/app, which could have all kind of dangerours consequences on the system.

On 2016-10-25 16:42:49 +0000, Sam Lantinga wrote:

I don't have access to the Raspberry Pi platform. Can you identify which commit caused the regression?

On 2016-10-26 09:08:16 +0000, Manuel Alfayate Corchete wrote:

I have just took a look at http://hg.libsdl.org/SDL/shortlog, and it's HUGE.
Building every "candidate" revision is impossible.
However, if you tell me what I should look for, I would be able to do a reasonabe number of builds.
I am willing to send you a Raspberry Pi, too. SDL2 is too important on the platform.

On 2016-10-26 19:12:12 +0000, Philipp Wiesemann wrote:

The patch for bug 3157 may have affected the keyboard handling.

On 2016-10-27 09:40:25 +0000, Manuel Alfayate Corchete wrote:

(In reply to Philipp Wiesemann from comment # 3)

The patch for bug 3157 may have affected the keyboard handling.

Can you indicate the revision numbers before and after the suspicious patch was merged? I am not used to this hg and I can't find out. An indication on how to fund out would be great, too.

On 2016-10-27 21:38:12 +0000, Philipp Wiesemann wrote:

The revision before the patch was applied is "eeb3847d81a6" and the revision after the patch was applied is "2b008531d078".

On 2016-10-27 21:38:39 +0000, Sam Lantinga wrote:

You can see the revision numbers here:
https://hg.libsdl.org/SDL/rev/0f79dd727ed6

On 2016-10-29 11:42:20 +0000, Manuel Alfayate Corchete wrote:

@sam Lantiga: I have built both eeb3847d81a6 and 2b008531d078 revisions, and indeed key leakage appears JUST after 2b008531d078.
So that's the commit where it appears. Thanks Phillipp for your suggestion as it was spot-on.

On 2016-10-31 02:21:44 +0000, Sam Lantinga wrote:

tvc, can you look at this regression caused by your patch?

Thanks!

On 2016-10-31 03:13:04 +0000, tvc wrote:

Created attachment 2595
enable keyboard muting

This was a complete oversight by me, clearly I didn't do proper testing. Sorry all.

I have attached a patch that I believe will fix this bug, however I'm not able to test this personally for a couple of days. Manuel, are you able to test this in the meantime?

On 2016-10-31 14:20:08 +0000, Manuel Alfayate Corchete wrote:

me@tvc.id.au

This patch doesn't work: keycodes are still leaking to the console and now the console is corrupted after exit: keypresses send wrong codes.

On 2017-01-07 10:06:19 +0000, tvc wrote:

Created attachment 2662
remove tty search and enable keyboard muting

I believe this patch should fix it, instead of looping through all the tty's and seemingly selecting the wrong one and corrupting the console I've just made SDL open /dev/tty which is the console attached to the current process anyway.

I know I took a fair amount of time on this, however, Manuel are you still able to test this to make sure it works on your system?

On 2017-01-07 14:53:37 +0000, Manuel Alfayate Corchete wrote:

@tvc, it will be a pleasure to test this patch. SDL2 on the Pi is a fundamental part of it's operating system IMHO...
However, I have tried to apply this patch with no success:

pi@raspberrypi:~/src/SDL-2.0.5-10739 $ patch -p1 < patch.diff
patching file src/core/linux/SDL_evdev.c
Hunk # 2 succeeded at 134 with fuzz 2.
Hunk # 3 FAILED at 152.
Hunk # 4 FAILED at 164.
Hunk # 5 succeeded at 236 (offset 4 lines).
Hunk # 6 succeeded at 267 (offset 4 lines).
2 out of 6 hunks FAILED -- saving rejects to file src/core/linux/SDL_evdev.c.rej

Do I need an specific SDL2 mercurial commit or something?

On 2017-01-07 15:01:01 +0000, tvc wrote:

Sorry I should have specified, it's a patch for the latest mercurial commit.

On 2017-01-07 17:04:11 +0000, Sam Lantinga wrote:

You can grab the latest Mercurial drop from here:
http://www.libsdl.org/tmp/SDL-2.0.zip

On 2017-01-07 17:48:38 +0000, Manuel Alfayate Corchete wrote:

@tvc: This works perfectly well now. No more keypress leaks. As good as 2.0.4 was. So I'd say it's FIXED. Thanks a lot! :)

On 2017-01-07 18:13:44 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/fe9c7d01a093

On 2017-01-08 09:31:24 +0000, Sam Lantinga wrote:

tvc, your patch broke SDL on the Steam Link, where the application can run without a tty. I fixed that, but I looked more closely at what the code is doing, and I had some questions for you.

On 2017-01-08 09:43:27 +0000, Sam Lantinga wrote:

It seems like console_fd is being used for two different purposes.

It mutes the keyboard so keystrokes only generate evdev events and don't leak through to the console. In this case I think we do want the active console, whatever that is. If the application is running from somebody logged in via ssh, and it opens an evdev keyboard, we want that keyboard input to go to the SDL application and not generate key events on the shell that is logged in at the console.

Separately from that, we use the console file descriptor to do translation of key presses into characters. In this case we don't care what tty we get, we just need one that we can query the kernel translation tables from. In bug 3545 someone notes that they're running into the type < 0xf0 case, and I think that's really what we want. We want to put the tty into VC_UNICODE mode, query all the tables into userspace, reset and close the tty, and then translate those unicode values into UTF-8. That still doesn't get us dead key handling and diacritics and so forth, as you noted, but it gets basic US key translation. I haven't dug into the Linux keyboard driver to see how it handles dead keys, but I see posts from people showing that it does work, and we can probably emulate what the keyboard driver does.

Remember, SDL programs don't necessarily run from the console, and there's no reason why they can't open keyboards and mice and do the right thing in that case. :)

On 2017-01-08 10:02:39 +0000, tvc wrote:

Sorry about the Steam Link break, I didn't realise that corner case.

Well we could open /dev/console which is the first VT, switch it to K_UNICODE and read the tables. Ideally this should be done before we open /dev/tty (the current VT) for muting, as if our current VT is the first VT we wouldn't want to unmute it. I'm currently looking at the kernel sources to see how the keyboard driver handles it all, so I should have a working patch soon.

On 2017-01-08 10:11:40 +0000, tvc wrote:

(In reply to tvc from comment # 19)

Sorry about the Steam Link break, I didn't realise that corner case.

Well we could open /dev/console which is the first VT, switch it to
K_UNICODE and read the tables. Ideally this should be done before we open
/dev/tty (the current VT) for muting, as if our current VT is the first VT
we wouldn't want to unmute it. I'm currently looking at the kernel sources
to see how the keyboard driver handles it all, so I should have a working
patch soon.

Scratch that, I was wrong, /dev/tty0 is current VT, /dev/tty1 is first VT, and /dev/tty is the VT the process was started in.

On 2017-01-09 04:02:10 +0000, Sam Lantinga wrote:

After discussion this is the correct fix.
It doesn't cover someone running the application from ssh, but in that case you can only access the virtual console if you're running as root.

On 2017-01-13 03:38:57 +0000, Jools Wills wrote:

Should https://hg.libsdl.org/SDL/rev/fe9c7d01a093 also fix this issue if applied to SDL 2.0.5 or does it rely on some additional patches ?

Running Emulation Station on Raspberry Pi directly on the framebuffer (on /dev/tty1)

I applied https://hg.libsdl.org/SDL/rev/fe9c7d01a093 to sdl 2.0.5 and the problem is still happening. Characters ending up on the console and the terminal being left in an unusable state on exit.

The latest SDL2 code does work however - so I assume I need some additional changes ?

I also applied https://hg.libsdl.org/SDL/rev/5ede976d922b but that didn't help either.

On 2017-01-13 04:29:25 +0000, Jools Wills wrote:

Unusable console on exit was fixed by cherry-picking https://hg.libsdl.org/SDL/rev/6cfccf993c5d

Also needed https://hg.libsdl.org/SDL/rev/6cfccf993c5d or else the keyboard didn't work in some parts of Emulation Station.

All seems good now. Looking forward to 2.0.6. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant