[Qemu-devel] [PATCH] monitor: Fix file_completion() to check for stat() failure

[Qemu-devel] [PATCH] monitor: Fix file_completion() to check for stat() failure

[Markus Armbruster]

stat() can fail for a file name just read with readdir().  Easiest way
to trigger is a dangling symbolic link --- look ma, no race!  When it
fails, file_completion() uses sb.st_mode uninitialized.  If the
directory bit happens to be set, it appends a "/" to the completed
name.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 5ea35de..1be222e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4207,9 +4207,9 @@ static void file_completion(const char *input)
             /* stat the file to find out if it's a directory.
              * In that case add a slash to speed up typing long paths
              */
-            stat(file, &sb);
-            if(S_ISDIR(sb.st_mode))
+            if (stat(file, &sb) == 0 && S_ISDIR(sb.st_mode)) {
                 pstrcat(file, sizeof(file), "/");
+            }
             readline_add_completion(cur_mon->rs, file);
         }
     }
-- 
1.7.6.4

Re: [Qemu-devel] [Qemu-trivial] [PATCH] monitor: Fix file_completion() to check for stat() failure

[Stefan Hajnoczi]

On Wed, Nov 16, 2011 at 03:43:47PM +0100, Markus Armbruster wrote:
> stat() can fail for a file name just read with readdir().  Easiest way
> to trigger is a dangling symbolic link --- look ma, no race!  When it
> fails, file_completion() uses sb.st_mode uninitialized.  If the
> directory bit happens to be set, it appends a "/" to the completed
> name.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  monitor.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

I have already sent a pull request including this patch for QEMU 1.0.

Stefan