Browse files

inspection: Fix double-free when certain guest files are empty.

The following commit:

  commit 5a3da36
  Author: Richard W.M. Jones <>
  Date:   Thu Jan 24 17:07:38 2013 +0000

      inspect: Use CLEANUP_* macros in inspection code.

can cause a double-free along an error path when certain guest files
are empty where we expected those files to contain at least one line.

This causes virt-inspector to crash when run on these guests.

The following is a test case which demonstrates the crash.
`f20rawhidex64' is a Fedora guest, but with small adjustments to the
test you could use any Linux guest for this test.

  $ qemu-img create -f qcow2 -b f20rawhidex64 /tmp/test.qcow2
  Formatting '/tmp/test.qcow2', fmt=qcow2 size=21474836480 backing_file='f20rawhidex64' encryption=off cluster_size=65536 lazy_refcounts=off
  $ guestfish -i -a /tmp/test.qcow2 -- rm /etc/redhat-release : touch /etc/redhat-release
  $ virt-inspector /tmp/test.qcow2
  *** glibc detected *** virt-inspector: double free or corruption (fasttop): 0x00007f18bc9925a0 ***
  ======= Backtrace: =========

This is a denial of service, but not likely to be exploitable.

(Found by Coverity)
  • Loading branch information...
1 parent 1e7f2b2 commit fa6a76050d82894365dfe32916903ef7fee3ffcd @rwmjones rwmjones committed May 28, 2013
Showing with 3 additions and 1 deletion.
  1. +3 −1 src/inspect-fs.c
@@ -544,7 +544,7 @@ guestfs___check_package_management (guestfs_h *g, struct inspect_fs *fs)
char *
guestfs___first_line_of_file (guestfs_h *g, const char *filename)
- CLEANUP_FREE char **lines = NULL; /* sic: not CLEANUP_FREE_STRING_LIST */
+ char **lines = NULL; /* sic: not CLEANUP_FREE_STRING_LIST */
bonzini Jun 26, 2013

Wouldn't a simpler fix be just to stop calling guestfs___free_string_list? Since lines[0] == NULL, all it does is free(lines).

rwmjones Jun 27, 2013 Collaborator

Yeah, that would be another way to fix this, probably simpler.

int64_t size;
char *ret;
@@ -573,6 +573,8 @@ guestfs___first_line_of_file (guestfs_h *g, const char *filename)
ret = lines[0]; /* caller frees */
+ free (lines);
return ret;

0 comments on commit fa6a760

Please sign in to comment.