Bug 2272045

Summary: virt-sparsify segmentation fault with advanced machine readable flag
Product: [Community] Virtualization Tools Reporter: Ben Brown <ben+rhbug>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED UPSTREAM QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: ben+rhbug, mhicks, ptoscano
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2024-03-28 12:55:06 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Initialise bar->fp as NULL none

Description Ben Brown 2024-03-28 12:32:09 UTC
Description of problem:

Using `virt-sparsify` with the advanced machine readable flag of `--machine-readable=file:filename` triggers a segmentation fault.

Backtrace without debug symbols pointed to a bug in `progress_bar_free`.

#0  0x00007ffff79c4ec8 in fclose@@GLIBC_2.2.5 () from /nix/store/1zy01hjzwvvia6h9dq5xar88v77fgh9x-glibc-2.38-44/lib/libc.so.6
#1  0x000000000053ad52 in progress_bar_free ()

Running through valgrind showed that there was an uninitialised value.

==545114== Conditional jump or move depends on uninitialised value(s)
==545114==    at 0x53AD4B: progress_bar_free (in /nix/store/0lw96m0h5yjsbxgqa7638v5l0lnkbw2z-guestfs-tools-1.50.1/bin/virt-sparsify)
...
==545114==  Uninitialised value was created by a heap allocation
==545114==    at 0x484276B: malloc (in /nix/store/4yj7p7b95fjbr9dp6cnnqdnq476622w1-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==545114==    by 0x53ADF1: progress_bar_init (in /nix/store/0lw96m0h5yjsbxgqa7638v5l0lnkbw2z-guestfs-tools-1.50.1/bin/virt-sparsify)

Inspection of the `progress_bar_init()` function revealed that `bar->fp` is left uninitialised when the `PROGRESS_BAR_MACHINE_READABLE` flag is set, which results in the call to `fclose()` in `progress_bar_free()` triggering a segmentation fault.

Version-Release number of selected component (if applicable):

virt-sparsify 1.50.1


How reproducible:

Always.

Steps to Reproduce:
1. Download a cloud image, say https://cloud.debian.org/images/cloud/bullseye/20240104-1616/debian-11-genericcloud-amd64-20240104-1616.qcow2
2. Run `virt-sparsify --compress --machine-readable=file:foo debian-11-genericcloud-amd64-20240104-1616.qcow2 compressed.qcow2`

Actual results:

Disk is seemingly compressed, but a segmentation fault occurs.

Expected results:

Command should succeed without seg fault.

Additional info:

Comment 1 Ben Brown 2024-03-28 12:36:58 UTC
Created attachment 2023993 [details]
Initialise bar->fp as NULL

This is likely enough to fix the issue, though an appropriate test case ought to be implemented as well.

Comment 2 Richard W.M. Jones 2024-03-28 12:55:06 UTC
Thanks, fixed upstream in https://github.com/libguestfs/libguestfs-common/commit/0330ebe40cb645df311bab25888c5ca6cc179efe

Doesn't really need a regression test as it's a simple bug.

Fix will be included in the next version of guestfs-tools.