[PATCH master] fs: ext4: fix bogus behavior on failure to read ext4 block

[Ahmad Fatoum]

On 25.03.21 14:04, Sascha Hauer wrote:
> On Wed, Mar 24, 2021 at 01:19:28PM +0100, Ahmad Fatoum wrote:
>> The conversion of blknr from a signed 32-bit to an unsigned 64-type resulted
>> in the check for error to never return true. Fix this.
>>
>> Affected configuration would behave incorrectly when served with invalid
>> blocks. Instead of aborting and having the filesystem bubble up an error
>> code, it would return invalid data. As there is no ext4 write support,
>> this wouldn't lead to ext4 data corruption however.
>>
>> Reported-by: Bastian Krause <bst at pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
>> ---
>>  fs/ext4/ext4fs.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
>> index 54349aad3f3f..344d423fd9c4 100644
>> --- a/fs/ext4/ext4fs.c
>> +++ b/fs/ext4/ext4fs.c
>> @@ -74,11 +74,11 @@ loff_t ext4fs_read_file(struct ext2fs_node *node, loff_t pos,
>>  		loff_t blockend = blocksize;
>>  		loff_t skipfirst = 0;
>>  
>> -		blknr = read_allocated_block(node, i);
>> -		if (blknr < 0)
>> -			return blknr;
>> +		ret = read_allocated_block(node, i);
>> +		if (ret < 0)
>> +			return ret;
> 
> ret is a 32bit type. What about drives > 2T?

the first if-clause has a 64-bit type coerced to signed long, but the
rest will only return up to 32-bit, so this needs a bit more investigation.

The fix is correct regardless IMO, so it should go into master.
Expanding the type can go into next then.

Cheers,
Ahmad

> 
> Sascha
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |