keymaps: detect recursive keyboard layout file

When the parse_keyboard_layout() find a "include " line in the keyboard layout file, it will call parse_keyboard_layout() to perform a recursive parse. If the keyboard layout is malformed by adding a line include itself, this can cause an infinite parse. Thus cause qemu a segv. This patch avoid this.

Quoted 5 lines · Li Qiang

When the parse_keyboard_layout() find a "include " line in the keyboard layout file, it will call parse_keyboard_layout() to perform a recursive parse. If the keyboard layout is malformed by adding a line include itself, this can cause an infinite parse. Thus cause qemu a segv. This patch avoid this.

Hmm. Most keymap files are generated by qemu-keymap these days and do not use includes in the first place. Three are left over: nl-be, sl, sv.

Looking at them it seems like nl-be is not functional, it just includes "common" and doesn't define any mappings. For sl and sv I have no clue what keymap they represent.

So I'd suggest to just remove support for "include", drop the nl-be map, fix the sl and sv maps that they don't need "include" any more. Either just replace the "include" statement with the content of the "common" file. Or, if someone has a clue what keyboard layout these keymaps are for, add rules to the Makefile and update them using then using qemu-keymap.

cheers,
Gerd

When the parse_keyboard_layout() find a "include " line in the keyboard layout file, it will call parse_keyboard_layout() to perform a recursive parse. If the keyboard layout is malformed by adding a line include itself, this can cause an infinite parse. Thus cause qemu a segv. This patch avoid this.

Signed-off-by Li Qiang <[email protected]>

---
 ui/keymaps.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

ui/keymaps.c +7 -0

index 085889b555..564893a9f3 100644
@@ -38,6 +38,8 @@ struct kbd_layout_t {
     GHashTable *hash;
 };
 
+GList *keyboard_files;
+
 static int get_keysym(const name2keysym_t *table,
                       const char *name)
 {

@@ -80,6 +82,11 @@ static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
     trace_keymap_add(keysym, keycode, line);
 }
 
+static gint compare_string(gconstpointer a, gconstpointer b)
+{
+    return g_strcmp0(a, b);
+}
+
 static int parse_keyboard_layout(kbd_layout_t *k,
                                  const name2keysym_t *table,
                                  const char *language, Error **errp)

@@ -94,12 +101,18 @@ static int parse_keyboard_layout(kbd_layout_t *k,
     filename = qemu_find_file(QEMU_FILE_TYPE_KEYMAP, language);
     trace_keymap_parse(filename);

ui/keymaps.c +4 -0

+        g_free(filename);
+        return -1;
+    }
+    keyboard_files = g_list_append(keyboard_files, filename);
     for(;;) {
         if (fgets(line, 1024, f) == NULL) {
             break;

ui/keymaps.c static void add_keysym(char *line, int keysym, int

Quoted 25 lines · Li Qiang

When the parse_keyboard_layout() find a "include " line in the keyboard layout file, it will call parse_keyboard_layout() to perform a recursive parse. If the keyboard layout is malformed by adding a line include itself, this can cause an infinite parse. Thus cause qemu a segv. This patch avoid this.

Signed-off-by Li Qiang <[email protected]>

---
 ui/keymaps.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

ui/keymaps.c +2 -0

index 085889b555..564893a9f3 100644
@@ -38,6 +38,8 @@ struct kbd_layout_t {
     GHashTable *hash;
 };
 
+GList *keyboard_files;
+
 static int get_keysym(const name2keysym_t *table,
                       const char *name)
 {

@@ -80,6 +82,11 @@ static void add_keysym(char *line, int keysym, int

keycode, kbd_layout_t *k)

Quoted 14 lines

ui/keymaps.c +5 -0

     trace_keymap_add(keysym, keycode, line);
 }
 
+static gint compare_string(gconstpointer a, gconstpointer b)
+{
+    return g_strcmp0(a, b);
+}
+
 static int parse_keyboard_layout(kbd_layout_t *k,
                                  const name2keysym_t *table,
                                  const char *language, Error **errp)

@@ -94,12 +101,18 @@ static int parse_keyboard_layout(kbd_layout_t *k,
     filename = qemu_find_file(QEMU_FILE_TYPE_KEYMAP, language);
     trace_keymap_parse(filename);

ui/keymaps.c static int parse_keyboard_layout(kbd_layout_t *k, · line 111

Quoted 10 lines

ui/keymaps.c +3 -1

     f = filename ? fopen(filename, "r") : NULL;
-    g_free(filename);
     if (!f) {
+        g_free(filename);
         error_setg(errp, "could not read keymap file: '%s'", language);
         return -1;
     }
 
+    if (g_list_find_custom(keyboard_files, filename, compare_string)) {
+        error_setg(errp, "find recursive keyboard layout: %s'",

filename);

Suggest something like "Inclusion loop for %s".

Quoted 11 lines

ui/keymaps.c +4 -0

+        g_free(filename);
+        return -1;
+    }
+    keyboard_files = g_list_append(keyboard_files, filename);
     for(;;) {
         if (fgets(line, 1024, f) == NULL) {
             break;

@@ -168,6 +181,8 @@ static int parse_keyboard_layout(kbd_layout_t *k,
     ret = 0;
 out:
     fclose(f);

ui/keymaps.c static int parse_keyboard_layout(kbd_layout_t *k, · line 187

Quoted 4 lines

ui/keymaps.c +2 -0

+    keyboard_files = g_list_remove(keyboard_files, filename);
+    g_free(filename);
     return ret;
 }

There's no real need to make @keyboard_files global. I'd make it local to init_keyboard_layout(), and pass it as argument to parse_keyboard_layout().

Matter of taste. Gerd is the maintainer, not
me.