Backing layer read in wrong precedence on a long read

[jdkuki]

When reading a qcow with a backing file, the backing file can take precedent when blocks exist in the layer above.

Setup:

Created a backing qcow with data formatted as MBR + ntfs, then a delta disk on top of it with a new file in the NTFS partition. When opening the delta file and setting the backing file, the new files created in the delta disk are not seen.

In this case we read the entire qcow into memory (4MB). Attaching the example files, the qcow map looks like:

jkuki@desktop$ qemu-img map ntfs_test_delta.qcow2
Offset          Length          Mapped to       File
0               0x20000         0x50000         ntfs_test_delta.qcow2
0x20000         0x60000         0x80000         ./ntfs_test.qcow2
0x80000         0x10000         0x70000         ntfs_test_delta.qcow2
0x90000         0x50000         0xf0000         ./ntfs_test.qcow2
0xe0000         0x10000         0xa0000         ntfs_test_delta.qcow2
0xf0000         0x100000        0x150000        ./ntfs_test.qcow2
0x1f0000        0x20000         0x80000         ntfs_test_delta.qcow2
0x210000        0x1e0000        0x270000        ./ntfs_test.qcow2
0x3f0000        0x10000         0x60000         ./ntfs_test.qcow2

The problematic line(s) are here:
https://github.com/libyal/libqcow/blob/main/libqcow/libqcow_file.c#L2693

			read_count = libqcow_file_read_buffer_at_offset(
				      internal_file->parent_file,
				      &( ( (uint8_t *) buffer )[ buffer_offset ] ),
				      buffer_size - buffer_offset,  <- length of remaining 
				      internal_file->current_offset,
				      error );

When we reach offset 0x20000, we pass the rest of read to be read from the parent file rather than breaking back out and calling libqcow_internal_file_get_cluster_block_offset on the immediate offset after the backing block.

This causes an issue where if we create files at the root of the drive in the delta disk, the MFT is actually read from the base disk. Only the blocks which are new in the delta disk get read.

You can test this by trying to read the entire QCOW into memory and then comparing a byte read to a small read at

#include <libqcow.h>
#include <stdlib.h>
#include <string.h>
#define EXIT_FAILURE 1


libqcow_file_t * open_qcow(const char * filename) {

    libqcow_error_t *error = NULL;
    libqcow_file_t *file = NULL;

    if( libqcow_file_initialize(&file, &error) != 1 )
    {
        fprintf(stderr, "Unable to initialize file.\n");
  
        libqcow_error_free(&error);
  
        exit(EXIT_FAILURE);


    }

    if( libqcow_file_open(file, filename, LIBQCOW_OPEN_READ, &error) != 1 )
    {
        fprintf(stderr, "Unable to open file.\n" );
    
        libqcow_file_free(&file, NULL);
        libqcow_error_free(&error);
    
        exit(EXIT_FAILURE);
    }

    return file;
}

int main() {
    const char *filename = "ntfs_test_delta.qcow2";
    const char *parent = "ntfs_test.qcow2";


    libqcow_error_t *error = NULL;

    libqcow_file_t *file = open_qcow(filename);
    libqcow_file_t *parentFile = open_qcow(parent);

    if (libqcow_file_set_parent_file(file, parentFile, &error) != 1 ) {
        fprintf(stderr, "Unable to set parent\n" );
    
        libqcow_file_free(&file, NULL);
        libqcow_file_free(&parentFile, NULL);
        libqcow_error_free(&error);
        exit(EXIT_FAILURE);
    }


    const size_t bsz = 0x85960;
    void *buf = malloc(bsz);
    ssize_t rd = libqcow_file_read_buffer(file, buf, bsz, &error);
    if (rd == -1) {
        fprintf(stderr, "Failed to read\n" );
    
        libqcow_file_free(&file, NULL);
        libqcow_file_free(&parentFile, NULL);
        libqcow_error_free(&error);
        exit(EXIT_FAILURE);
    }


    size_t diffByteOffset = 0x85910;
    char diffByte;
    rd = libqcow_file_read_buffer_at_offset(file, &diffByte, 1, diffByteOffset, &error);
    if (rd == -1) {
        fprintf(stderr, "Failed to read\n" );
    
        libqcow_file_free(&file, NULL);
        libqcow_file_free(&parentFile, NULL);
        libqcow_error_free(&error);
        exit(EXIT_FAILURE);
    }

    if (memcmp(&diffByte, &(*((char*)buf + diffByteOffset)), 1) != 0){
        fprintf(stderr, "Magic byte not equal!\n" );
    }
}

ntfs_test.zip

[joachimmetz]

thx for flagging I'll have a closer look when time permits

[jdkuki]

Thanks @joachimmetz, I took a stab at a fix: #21

Reading on the spec, it seems when reading from a parent layer we shouldn't read past the start of the next cluster block wherever that may be

[joachimmetz]

thx having a look later, when time permits

[jdkuki]

@joachimmetz any thoughts here?

[joachimmetz]

unfortunately this has not been on my top prio list, but I will take a look when time permits