Bug 636061

Summary: [abrt] guestfish-1.2.11-1.fc12: malloc_consolidate: Process /usr/bin/guestfish was killed by signal 11 (SIGSEGV)
Product: [Fedora] Fedora Reporter: Karel Klíč <kklic>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 12CC: mbooth, rjones, rvokal, virt-maint
Target Milestone: ---   
Target Release: ---   
Hardware: i686   
OS: Linux   
Whiteboard: abrt_hash:d3d70ee929f45eb7e472d8473b3dfeab4cd9f064
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-10-25 07:50:01 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
File: backtrace none

Description Karel Klíč 2010-09-21 09:07:44 EDT
abrt version: 1.1.14
architecture: i686
Attached file: backtrace
cmdline: guestfish -a fedora-13-i386-mojebanka.img
component: libguestfs
crash_function: malloc_consolidate
executable: /usr/bin/guestfish
kernel: 2.6.32.21-168.fc12.i686.PAE
package: guestfish-1.2.11-1.fc12
rating: 4
reason: Process /usr/bin/guestfish was killed by signal 11 (SIGSEGV)
release: Fedora release 12 (Constantine)
time: 1285071768
uid: 502

comment
-----
Sometimes libguestfs returns an error message instead of crashing:
><fs> upload ~/  /home/karel/notes/finance/certifikat_kb.p12
libguestfs: error: check_for_daemon_cancellation_or_eof: read 0x58 from daemon, expected 0xffffeeee

Sometimes glibc catches the memory corruption:
><fs> upload ~/  /home/karel/notes/finance/certifikat_kb.p12
*** glibc detected *** guestfish: malloc(): memory corruption: 0x0896f068 ***


The problem is 100% reproducible.

How to reproduce
-----
Run `guestfish fedora-13.img` (qcow2 format)
><fs> run
><fs> mount /dev/VolGroup/lv_root /
><fs> upload ~/certifikat_kb.p12  /home/karel/notes/finance/certifikat_kb.p12
libguestfs: error: open: /home/karel/certifikat_kb.p12: No such file or directory
><fs> upload ~/  /home/karel/notes/finance/certifikat_kb.p12
Segmentation fault (core dumped)
Comment 1 Karel Klíč 2010-09-21 09:07:47 EDT
Created attachment 448689 [details]
File: backtrace
Comment 2 Richard W.M. Jones 2010-09-21 13:35:31 EDT
Karel, I can't seem to reproduce this.  With the
same version of guestfish on F12 (but x86-64 instead
of i386) I get:

><fs> upload ~/ /foo
libguestfs: error: read: /home/rjones/: Is a directory

And the same thing happens with the latest libguestfs too.

I'm trying to think what could be different about our
systems.  So can you try this test out?  Just copy and
paste the lines below into a shell -- you don't need root.
It will create a 100MB file called /tmp/test1.img which
you can remove after the end of the test.

guestfish <<EOF
alloc /tmp/test1.img 100M
run
part-disk /dev/sda mbr
mkfs ext2 /dev/sda1
mount /dev/sda1 /
upload ~/ /foo
EOF
echo $?
Comment 3 Karel Klíč 2010-10-25 07:33:10 EDT
I tried the test and this is the output:

*** glibc detected *** guestfish: malloc(): memory corruption: 0x08a93128 ***


When I run it under gdb:
(gdb) run
Starting program: /usr/bin/guestfish 

Welcome to guestfish, the libguestfs filesystem interactive shell for
editing virtual machine filesystems.

Type: 'help' for help with commands
      'quit' to quit the shell

><fs> alloc /tmp/test1.img 100M
><fs> run
Detaching after fork from child process 3039.
Detaching after fork from child process 3040.
Detaching after fork from child process 3041.
Detaching after fork from child process 3043.
Detaching after fork from child process 3044.

><fs> 
><fs> part-disk /dev/sda mbr
><fs> mkfs ext2 /dev/sda1
><fs> mount /dev/sda1 /
><fs> upload ~/ /foo
libguestfs: error: open: /home/karel/: No such file or directory
><fs> upload ~/ /foo
libguestfs: error: read: /home/karel/: Is a directory
><fs> upload ~/ /foo
*** glibc detected *** /usr/bin/guestfish: malloc(): memory corruption: 0x080a5068 ***

I tracked it to the expand_home function, where a buffer is allocated, but the allocation does not take the terminating \0 into account.


And here is a fix: 

diff -up libguestfs-1.2.11/fish/tilde.c.fixed libguestfs-1.2.11/fish/tilde.c
--- libguestfs-1.2.11/fish/tilde.c.fixed        2010-10-25 13:08:52.042769107 +0200
+++ libguestfs-1.2.11/fish/tilde.c      2010-10-25 13:08:58.346768251 +0200
@@ -85,7 +85,7 @@ expand_home (const char *append)
   home = getenv ("HOME");
   if (!home) home = "~";
 
-  len = strlen (home) + (append ? strlen (append) : 0);
+  len = strlen (home) + (append ? strlen (append) : 0) + 1;
   str = malloc (len);
   if (str == NULL) {
     perror ("malloc");


I tested the fix and it works.
Comment 4 Richard W.M. Jones 2010-10-25 07:50:01 EDT
Yes that is indeed a rather nasty off-by-one
allocation bug.

I have pushed your fix upstream:

http://git.annexia.org/?p=libguestfs.git;a=commitdiff;h=1d5ec10659d67020c5b709e4df4c49bc0d59d58c

Thanks for tracking this bug down and your contribution
to libguestfs.