[PATCH] xfs: don't ASSERT on corrupt ftype

[Eric Sandeen]

On 9/8/14 8:05 AM, Dave Chinner wrote:
> On Sun, Sep 07, 2014 at 08:06:35PM -0500, Eric Sandeen wrote:
>> xfs_dir3_data_get_ftype() and xfs_dir2_sf_check() get
>> the file type off disk, but ASSERT if it's invalid:
>>
>> 	ASSERT(type < XFS_DIR3_FT_MAX);
>>
>> This might be cut & paste from the "put" functions,
>> which should be checking that they've not been passed
>> bad values, but we shouldn't ASSERT on bad values
>> read from disk.
>
> No, they weren't cut-n-paste from the put functions. They were
> actually designed for a metadata block where bad values would not be
> written to disk, and corrupted disk blocks would be detected by CRC
> validation failures. So on debug kernels it's quite appropriate to
> assert fail on a "should never, ever happen" condition.

hohum, ok.

> Then the v4 ftype feature bit was rammed in but none of the code got
> changed to reflect that the values in the ftype fields are not CRC
> protected on v4 filesystems....
>
>> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
>> index 5079e05..ea89250 100644
>> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
>> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
>> @@ -635,7 +635,6 @@ xfs_dir2_sf_check(
>>   		offset =
>>   			xfs_dir2_sf_get_offset(sfep) +
>>   			dp->d_ops->data_entsize(sfep->namelen);
>> -		ASSERT(dp->d_ops->sf_get_ftype(sfep) < XFS_DIR3_FT_MAX);
>>   	}
>>   	ASSERT(i8count == sfp->i8count);
>>   	ASSERT((char *)sfep - (char *)sfp == dp->i_d.di_size);
>
> That's a debug only function validating that the shortform directory
> is internally consistent. And as the comment says:
>
> /*
>   * Check consistency of shortform directory, assert if bad.
>   */
>
> So that assert should remain because it's checking the in-memory
> state immediately before, during and after modifications are made to
> the directory, not when it has just been read of disk....
>
> Cheers,
>
> Dave.
>