keyboard: fix qemu load empty keymap

Commit Message

Wang Xin Nov. 3, 2016, 5:56 a.m. UTC

qemu_find_file do not check file is a directory or just a file.
If qemu start with "-k ''", qemu_find_file get a empty string
as keymap file name, then, qemu treat the keymap path as keymap
file, it makes vnc keyboard input unusable.

Signed-off-by: Wang Xin <[e-mail]>

Comments

Michael Tokarev Nov. 3, 2016, 6:30 a.m. UTC | #1

03.11.2016 08:56, Wang Xin wrote:
> qemu_find_file do not check file is a directory or just a file.
> If qemu start with "-k ''", qemu_find_file get a empty string
> as keymap file name, then, qemu treat the keymap path as keymap
> file, it makes vnc keyboard input unusable.

Do we really care?  "Garbage in, garbage out" I'd say :)

Thanks,

/mjt

> diff --git a/vl.c b/vl.c
> index ebd47af..2ec3832 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2264,6 +2264,7 @@ char *qemu_find_file(int type, const char *name)
>      int i;
>      const char *subdir;
>      char *buf;
> +    struct stat file_stat;
>  
>      /* Try the name as a straight path first */
>      if (access(name, R_OK) == 0) {
> @@ -2284,7 +2285,13 @@ char *qemu_find_file(int type, const char *name)
>  
>      for (i = 0; i < data_dir_idx; i++) {
>          buf = g_strdup_printf("%s/%s%s", data_dir[i], subdir, name);
> -        if (access(buf, R_OK) == 0) {
> +        if (stat(buf, &file_stat) < 0) {
> +            error_report("can not get file '%s' stat: %s\n", buf,
> +                         strerror(errno));
> +            g_free(buf);
> +            return NULL;
> +        }
> +        if (!S_ISDIR(file_stat.st_mode) && access(buf, R_OK) == 0) {
>              trace_load_file(name, buf);
>              return buf;
>          }
>

Markus Armbruster Nov. 3, 2016, 7:09 a.m. UTC | #2

Michael Tokarev <[e-mail]> writes:

> 03.11.2016 08:56, Wang Xin wrote:
>> qemu_find_file do not check file is a directory or just a file.
>> If qemu start with "-k ''", qemu_find_file get a empty string
>> as keymap file name, then, qemu treat the keymap path as keymap
>> file, it makes vnc keyboard input unusable.
>
> Do we really care?  "Garbage in, garbage out" I'd say :)

Moreover, are you really, really sure qemu_find_file() is never used to
find a directory?

Outside the scope of the patch: qemu_find_file() is basically a bad
idea.  By the time the file name it computes gets used, the file it
found may be gone, or have become a directory (TOCTTOU).

The proper fix for -k '' breaking the keyboard would be to make -k
reject garbage input.  Have a look at parse_keyboard_layout() and weep.

Patch

diff --git a/vl.c b/vl.c
index ebd47af..2ec3832 100644
--- a/vl.c
+++ b/vl.c
@@ -2264,6 +2264,7 @@  char *qemu_find_file(int type, const char *name)
     int i;
     const char *subdir;
     char *buf;
+    struct stat file_stat;
 
     /* Try the name as a straight path first */
     if (access(name, R_OK) == 0) {
@@ -2284,7 +2285,13 @@  char *qemu_find_file(int type, const char *name)
 
     for (i = 0; i < data_dir_idx; i++) {
         buf = g_strdup_printf("%s/%s%s", data_dir[i], subdir, name);
-        if (access(buf, R_OK) == 0) {
+        if (stat(buf, &file_stat) < 0) {
+            error_report("can not get file '%s' stat: %s\n", buf,
+                         strerror(errno));
+            g_free(buf);
+            return NULL;
+        }
+        if (!S_ISDIR(file_stat.st_mode) && access(buf, R_OK) == 0) {
             trace_load_file(name, buf);
             return buf;
         }