[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] keymaps: detect recursive keyboard layout file
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] keymaps: detect recursive keyboard layout file |
Date: |
Thu, 15 Nov 2018 14:29:23 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Li Qiang <address@hidden> writes:
> 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 <address@hidden>
> ---
> ui/keymaps.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/ui/keymaps.c b/ui/keymaps.c
> index 085889b555..564893a9f3 100644
> --- a/ui/keymaps.c
> +++ b/ui/keymaps.c
> @@ -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);
> 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".
> + 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);
> + 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.