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.

Sign up for GitHub

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

Jump to bottom

If XKB_DEFAULT_LAYOUT doesn't exist the program segfaults #29

Open
01micko opened this issue Nov 11, 2024 · 13 comments · May be fixed by #30
Open

If XKB_DEFAULT_LAYOUT doesn't exist the program segfaults #29

01micko opened this issue Nov 11, 2024 · 13 comments · May be fixed by #30

Comments

@01micko
Copy link
Contributor

01micko commented Nov 11, 2024

context

We are in early stages of testing labwc in VMs over at bunsenlabs and I have a package for labwc-tweaks-gtk that is installable via apt because I have a tiny repo setup with just a few things that debian don't provide. Things aren't quite up to date but in an early netinstall installation only some very basics were provided in the ~/.config/labwc/environment/ and XKB_DEFAULT_LAYOUT isn't one of them!

current state of play

If env vars for XCURSOR_THEME and XCURSOR_SIZE aren't there it doesn't matter (values obtained from gsettings). However if XKB_DEFAULT_LAYOUT isn't there the program stops with a segfault in update. There is a super simple "fix"; next point.

super simple fix

--- ../labwc-tweaks-gtk/update.c	2024-10-20 08:00:32.751587188 +1000
+++ update.c	2024-11-11 19:35:10.330534674 +1000
@@ -20,6 +20,9 @@
 static const char
 *first_field(const char *s, char delim)
 {
+	if(!s){
+		return "us";
+	}
 	char *p = strchr(s, delim);
 	if (p) {
 		*p = '\0';

why?

Well a segfault looks crappy and at least this stops it with a legit entry that works. There are probably a thousand better ways with a quite a few more lines of code but I think the proposed fix is a small price to pay; most non-English speakers would likely want to change the layout anyway and if they hit Update because they did the mouse or the corner radius beforehand they will just throw their PC out the window, and it's our fault!
01micko added a commit to 01micko/labwc-tweaks-gtk that referenced this issue Nov 11, 2024
@01micko
update.c: work around segfault
61edc15 
fixes labwc#29
@01micko 01micko linked a pull request Nov 11, 2024 that will close this issue
Open
@01micko
Copy link
Contributor Author

01micko commented Nov 11, 2024

Sorry, I should cc @johnraff because he discovered the bug.

@Consolatis
Copy link
Member

Well a segfault looks crappy

Can confirm.

@01micko
Copy link
Contributor Author

01micko commented Nov 12, 2024

Ok, #30 is a bit simplistic and dare I say frivolous. (though it wasn't meant to be)

I suppose we should check the environment first; labwc may have been started as a session (not exactly sure how that works).
Second, check if /etc/default/keyboard exists and parse that.
Third fallback so the program doesn't segfault.

@Consolatis
Copy link
Member

Consolatis commented Nov 12, 2024 •
edited
Loading

At least on my (Debian based) system /etc/default/keyboard actually uses the correct layout (e.g. non us). I have no idea what part of Debian modified it, maybe during installation as it asks for the keyboard layout. So trying to use that file should at least improve things for Debian users (and I assume for derivatives like Ubuntu as well).

There is also localectl which may help on systemd based systems (no clue where that one gets its information from though).
It's man page mentions /etc/vconsole.conf.

Absolutely agree on some hardcoded fallback as last choice.

@01micko
Copy link
Contributor Author

01micko commented Nov 12, 2024

Actually that's all nonsense. All we need to do is test that (COMBO_TEXT(state->widgets.keyboard_layout) isn't NULL and only send it to first_field() if it has a value.

if (COMBO_TEXT(state->widgets.keyboard_layout)) {
	environment_set("XKB_DEFAULT_LAYOUT", first_field(COMBO_TEXT(state->widgets.keyboard_layout), ' '));
}

We're not messing with anything that way, no need of a fallback. The program will carry on quite happily.

@johnraff
Copy link

On a Debian Trixie VM with XKB_DEFAULT_LAYOUT=jp in the environment, labwc-tweaks-gtk still segfaults when the user attempts to change the gtk theme, so there's more to the story.

@johnraff
Copy link

check if /etc/default/keyboard exists and parse that.

FWIW the current proposed BunsenLabs Wayland setup runs this at login:

# Export keyboard layout from /etc/default/keyboard
if [ "$BL_SESSION" = 'wayland' ]; then
    . /etc/default/keyboard
    [ -n "$XKBMODEL" ] && export XKB_DEFAULT_MODEL=$XKBMODEL
    [ -n "$XKBLAYOUT" ] && export XKB_DEFAULT_LAYOUT=$XKBLAYOUT
    [ -n "$XKBVARIANT" ] && export XKB_DEFAULT_VARIANT=$XKBVARIANT
    [ -n "$XKBOPTIONS" ] && export XKB_DEFAULT_OPTIONS=$XKBOPTIONS
    [ -n "$XKBRULES" ] && export XKB_DEFAULT_RULES=$XKBRULES
fi

@01micko
Copy link
Contributor Author

01micko commented Nov 12, 2024

On a Debian Trixie VM with XKB_DEFAULT_LAYOUT=jp in the environment, labwc-tweaks-gtk still segfaults when the user attempts to change the gtk theme, so there's more to the story.

Is that just exported to the environment or in ~/.config/labwc/environment file?

@johanmalm
Copy link
Contributor

On a Debian Trixie VM with XKB_DEFAULT_LAYOUT=jp in the environment, labwc-tweaks-gtk still segfaults when the user attempts to change the gtk theme, so there's more to the story.

Could you run with ASAN and post a stacktrace?

meson setup build
meson configure -Db_sanitize=address,undefined build
meson compile -C build
./build/labwc-tweaks-gtk 2>log.txt

@johanmalm
Copy link
Contributor

@01micko Suggest a more generic approach. How about something like:

diff --git a/environment.c b/environment.c
index 94e1130..7e10d1f 100644
--- a/environment.c
+++ b/environment.c
@@ -86,6 +86,13 @@ environment_get(char *buffer, size_t size, const char *key)
 void
 environment_set(const char *key, const char *value)
 {
+	if (!key || !*key) {
+		return;
+	}
+	if (!value || !*value) {
+		return;
+	}
+
 	/* set cursor for labwc  - should cover 'replace' or 'append' */
 	char xcur[4096] = {0};
 	strcpy(xcur, key);
diff --git a/update.c b/update.c
index 56d3f62..8300dd0 100644
--- a/update.c
+++ b/update.c
@@ -20,6 +20,9 @@ spawn_sync(char const *command)
 static const char
 *first_field(const char *s, char delim)
 {
+	if (!s) {
+		return NULL;
+	}
 	char *p = strchr(s, delim);
 	if (p) {
 		*p = '\0';

For completeness, if XKB_DEFAULT_LAYOUT is not set or empty, labwc will default to us anyway.

@01micko
Copy link
Contributor Author

01micko commented Nov 12, 2024

Here's a stacktrace on bare metal from ASAN. This is with XKB_DEFAULT_LAYOUT removed from~/.config/labwc/environment

trace 

mick@dellhome:~/Github/labwc/labwc-tweaks-gtkc$ build/labwc-tweaks-gtk
../update.c:23:12: runtime error: null pointer passed as argument 1, which is declared to never be null
AddressSanitizer:DEADLYSIGNAL
=================================================================
==67467==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fef4e57101f bp 0x7ffe90cdb240 sp 0x7ffe90cda9d8 T0)
==67467==The signal is caused by a READ memory access.
==67467==Hint: address points to the zero page.
    #0 0x7fef4e57101f in __strchr_avx2 ../sysdeps/x86_64/multiarch/strchr-avx2.S:67
    #1 0x7fef4fa7e0ff in strchr ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:709
    #2 0x55f9f1e6db28 in first_field ../update.c:23
    #3 0x55f9f1e6e717 in update ../update.c:74
    #4 0x7fef5016cbc8  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x16bc8) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #5 0x7fef501828f7  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x2c8f7) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #6 0x7fef50188665 in g_signal_emit_valist (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32665) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #7 0x7fef50188722 in g_signal_emit (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32722) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #8 0x7fef4f2e1e0e  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0xe1e0e) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #9 0x7fef5016cbc8  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x16bc8) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #10 0x7fef501828f7  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x2c8f7) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #11 0x7fef50188665 in g_signal_emit_valist (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32665) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #12 0x7fef50188722 in g_signal_emit (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32722) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #13 0x7fef4f2e0133  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0xe0133) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #14 0x7fef4f29e05d  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x9e05d) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #15 0x7fef5016cbc8  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x16bc8) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #16 0x7fef501828f7  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x2c8f7) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #17 0x7fef50188665 in g_signal_emit_valist (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32665) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #18 0x7fef50188722 in g_signal_emit (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32722) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #19 0x7fef4f3bc33d  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x1bc33d) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #20 0x7fef5016fdd0 in g_cclosure_marshal_VOID__BOXEDv (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x19dd0) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #21 0x7fef5016cbc8  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x16bc8) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #22 0x7fef501828f7  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x2c8f7) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #23 0x7fef50188665 in g_signal_emit_valist (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32665) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #24 0x7fef50188722 in g_signal_emit (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32722) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #25 0x7fef4f3b940a  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x1b940a) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #26 0x7fef4f3baa12  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x1baa12) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #27 0x7fef4f3bdc29  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x1bdc29) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #28 0x7fef4f382500 in gtk_event_controller_handle_event (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x182500) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #29 0x7fef4f5536cc  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x3536cc) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #30 0x7fef4f297db9  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x97db9) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #31 0x7fef5016cbc8  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x16bc8) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #32 0x7fef50181b72  (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x2bb72) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #33 0x7fef50188665 in g_signal_emit_valist (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32665) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #34 0x7fef50188722 in g_signal_emit (/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x32722) (BuildId: f964ade0885c530b26dae48f2f013ca6cb364bf6)
    #35 0x7fef4f55514b  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x35514b) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #36 0x7fef4f405dad  (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x205dad) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #37 0x7fef4f407b75 in gtk_main_do_event (/lib/x86_64-linux-gnu/libgtk-3.so.0+0x207b75) (BuildId: 193497d3ee906e04b310d52a58789757c17ebcc1)
    #38 0x7fef4e33ace8  (/lib/x86_64-linux-gnu/libgdk-3.so.0+0x3dce8) (BuildId: 5bbb01267b170bb27c6ff112fd518004f01bd1f3)
    #39 0x7fef4e36ef95  (/lib/x86_64-linux-gnu/libgdk-3.so.0+0x71f95) (BuildId: 5bbb01267b170bb27c6ff112fd518004f01bd1f3)
    #40 0x7fef4ef177de  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5a7de) (BuildId: f88be3a6bcb9df2a9d9eb549104662ed879d653d)
    #41 0x7fef4ef19a16  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5ca16) (BuildId: f88be3a6bcb9df2a9d9eb549104662ed879d653d)
    #42 0x7fef4ef1a17f in g_main_context_iteration (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5d17f) (BuildId: f88be3a6bcb9df2a9d9eb549104662ed879d653d)
    #43 0x7fef4f0f4444 in g_application_run (/lib/x86_64-linux-gnu/libgio-2.0.so.0+0xe9444) (BuildId: d0853cc0bcc57c13a5713dc63af3abf9deaf5257)
    #44 0x55f9f1e63244 in main ../main.c:82
    #45 0x7fef4e433d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #46 0x7fef4e433e24 in __libc_start_main_impl ../csu/libc-start.c:360
    #47 0x55f9f1e62a10 in _start (/home/mick/Github/labwc/labwc-tweaks-gtkc/build/labwc-tweaks-gtk+0x14a10) (BuildId: 0a34ebb27ba61fcd4d83f155911d4d4af49ccd38)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ../sysdeps/x86_64/multiarch/strchr-avx2.S:67 in __strchr_avx2
==67467==ABORTING

@01micko
Copy link
Contributor Author

01micko commented Nov 12, 2024

@johanmalm I like that fix. It addresses the actual bug.

update.c:23:12: runtime error: null pointer passed as argument 1, which is declared to never be null

Future proofs first_field() in case we use it for something else down the track.

01micko added a commit to 01micko/labwc-tweaks-gtk that referenced this issue Nov 12, 2024
@01micko
update.c: work around segfault
77f8e3d 
fixes labwc#29
@johnraff
Copy link

On a Debian Trixie VM with XKB_DEFAULT_LAYOUT=jp in the environment, labwc-tweaks-gtk still segfaults when the user attempts to change the gtk theme, so there's more to the story.

Is that just exported to the environment or in ~/.config/labwc/environment file?

It's in the environment, exported by the startup scripting, not in any labwc-specific file, but the export is done before starting labwc. It sets my keyboard layout to jp, so I guess labwc is seeing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

update.c: work around segfault 01micko/labwc-tweaks-gtk
4 participants
@01micko @johnraff @johanmalm @Consolatis