search

0x09 / hfsfuse

FUSE driver for HFS+ filesystems
Other
77 stars 13 forks source link

Allow reading empty files. #13

Closed pteromys closed 4 years ago

pteromys commented 4 years ago

Symptom

cat: '/mnt/usb/empty.txt': Operation not permitted

Cause

The outputs of hfslib_get_file_extents() cannot distinguish between errors and an empty list of extents—in both cases, it sets *out_extents = NULL and returns 0 for the number of extents. Naturally, hfslib_readd_with_extents() returns -1 in this error case.

Fix

In hfsfuse_read(), don't call hfslib_readd_with_extents() if the file size is zero. This requires recording, in struct hf_file, either the file size or a boolean flag that the file is empty.

I went with a boolean flag in a uint8_t to be nicer to memory-constrained users, and to make it clear that we don't actually use the full file size information. But I didn't really think about performance at all.

0x09 commented 4 years ago

Thanks, looks great. I was curious if it made sense to just return EOFs any time the extents were empty, which AFAIK outside of this scenario could only occur as a result of corruption, so I decided to check out how the Apple driver handles this and other mismatches between logical/extent size by tweaking a test image.

Looking at the fork structure with extents embedded there are essentially 3 values in play: the reported logical size, the total number of blocks worth of extents, and the block count per extent.

struct hfs_fork_t {
    uint64_t logical_size;
    uint32_t clump_size;
    uint32_t total_blocks;
    struct {
        uint32_t start_block;
        uint32_t block_count;
    } extents[8];
};

The driver takes total_blocks quite seriously -- any time this (x the block size) is less than the logical_size open itself will fail with EINVAL. As long as total_blocks is greater than or equal to the logical size then open will succeed and it's not used further. The sum of the block_counts isn't taken into consideration there but it is used for reading. Both read and stat will actually report/give the minimum of the logical_size and the block_count (x the block size).

Point being that referencing the logical size like you're doing here for is definitely more correct according to the official driver than my idea.

n.b. It's not particularly important that we handle random fork data corruption identically to the official driver (although I may try to tweak some things knowing that.)

pteromys commented 4 years ago

Huh, good to know. Thanks for digging into that! All I was going off of was a suspicion that silently swallowing the symptoms of corruption would be wrong.