Patchwork monitor: Fix file_completion() to check for stat() failure

login
register
mail settings
Submitter Markus Armbruster
Date Nov. 16, 2011, 2:43 p.m.
Message ID <1321454627-19403-1-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/125993/
State New
Headers show

Comments

Markus Armbruster - Nov. 16, 2011, 2:43 p.m.
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(-)
Stefan Hajnoczi - Nov. 18, 2011, 11:24 a.m.
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

Patch

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);
         }
     }