Fix fuzzing-discovered issues in filesystem drivers #76

stokescat · 2025-07-23T11:42:01Z

Fix undefined behavior (UB) in cluster number construction
Replaced ASSERT() with explicit error handling
Fix memory leak in FatIFileAccess error handling
Added EFI_FILE_PROTOCOL Flush() dummy implementation

Description

This patch series addresses critical issues discovered during fuzzing testing of filesystem drivers:

Undefined behavior fix:

Added explicit casts and ensured proper bit-width handling

Robust error handling:

Replaced ASSERT() with explicit error checking

Resource managment:

Fixed async task object leak in the FAT driver

UEFI specification compliance:

Implemented missing Flush() function in the ext4 driver
Added minimal dummy implementation to satisfy protocol requirements
Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- All changes are backward-compatible bug fixes and spec compliance improvements
Impacts security?
- Security - Does this PR have a direct security impact?
- Fixes memory leak and UB
- Replaces ASSERT conditions
- Implements required spec function
Includes tests?
- Tests - Does this PR include any explicit test code?
- Validation performed through fuzzing harness

How This Was Tested

All fixes were discovered and validated through custom filesystem fuzzing harness.

Integration Instructions

N/A - Changes are self-contained and require no special integration steps.

An UB was found during fuzz testing. When constructing a cluster number from 16-bit components, a left shift by 16 bits caused an implicit conversion to an 'int' type, leading to UB due to overflow. The fix ensures proper handling of integer promotions by: 1. Using explicit cast to unsigned type before shift operation 2. Ensuring sufficient bit-width for the operation Reference: ISO/IEC 9899:2018 (C17 standard), Section 6.3.1.1 Draft available: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf Signed-off-by: Pavel Naberezhnev <[email protected]>

MikhailKrichanov

Include in PR only single resulting commit for the feature with full description from b1d3b53, i.e. unite b1d3b53 and a46bf5a.

During fuzz testing, we encountered an ASSERT() condition being triggered. Changes: - Replaced ASSERT() with explicit condition check - Returns EFI_DEVICE_ERROR on failure case Signed-off-by: Pavel Naberezhnev <[email protected]>

A memory leak was discovered during fuzz testing: - The async task object (Task) created in FatIFileAccess was not being freed when FatGrowEof() returned an error Changes: - Moved the "Done" label to precede the "Task" check in if-then branch Signed-off-by: Pavel Naberezhnev <[email protected]>

Ext4Pkg/Ext4Dxe/Ext4Dxe.h

Ext4Pkg/Ext4Dxe/File.c

Ext4Pkg/Ext4Dxe/Partition.c

The ext4 driver implements EFI_FILE_PROTOCOL_REVISION but was missing the required Flush() function. According to UEFI Specification v2.10, Section 13.5 (File Protocol), all protocol functions must be implemented. Changes: - Adds minimal Flush() function implementation Signed-off-by: Pavel Naberezhnev <[email protected]>

MikhailKrichanov reviewed Jul 23, 2025

View reviewed changes

Pavel Naberezhnev and others added 2 commits July 23, 2025 18:44

Replaced ASSERT() with explicit error handling

41ecc8a

During fuzz testing, we encountered an ASSERT() condition being triggered. Changes: - Replaced ASSERT() with explicit condition check - Returns EFI_DEVICE_ERROR on failure case Signed-off-by: Pavel Naberezhnev <[email protected]>

stokescat force-pushed the fix/fs-06-07-2025 branch from 026be0d to ce72698 Compare July 23, 2025 15:51