[lvc-project] [PATCH 1/1] dm-vdo: Refactor VDO_ASSERT usage in start_restoring_volume functions

[Matthew Sakai]

On 9/9/25 2:58 PM, Matthew Sakai wrote:
> On 9/8/25 6:24 PM, Ivan Abramov wrote:
>> There's an incorrect VDO_ASSERT macro usage in
>> start_restoring volume_index() and start_restoring_volume_sub_index(),
>> since assert's return value is not used anywhere.
>>
>> Thus, use VDO_ASSERT_LOG_ONLY macro in such cases.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Signed-off-by: Ivan Abramov <i.abramov at mt-integration.ru>
>> ---
>>   drivers/md/dm-vdo/indexer/volume-index.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/md/dm-vdo/indexer/volume-index.c b/drivers/md/dm- 
>> vdo/indexer/volume-index.c
>> index 12f954a0c532..99608f14400a 100644
>> --- a/drivers/md/dm-vdo/indexer/volume-index.c
>> +++ b/drivers/md/dm-vdo/indexer/volume-index.c
>> @@ -832,11 +832,9 @@ static int 
>> start_restoring_volume_sub_index(struct volume_sub_index *sub_index,
>>           decode_u32_le(buffer, &offset, &header.first_list);
>>           decode_u32_le(buffer, &offset, &header.list_count);
>> -        result = VDO_ASSERT(offset == sizeof(buffer),
>> +        VDO_ASSERT_LOG_ONLY(offset == sizeof(buffer),
>>                       "%zu bytes decoded of %zu expected", offset,
>>                       sizeof(buffer));
>> -        if (result != VDO_SUCCESS)
>> -            result = UDS_CORRUPT_DATA;
> 
> This is not a correct fix so we should not take this.
> 
> This assertion (and the one below) should only fail if the stored 
> metadata has been corrupted, but in that case we definitely need to 
> return the error and fail the load.
> 
> You are correct that we don't seem to use this result, but the correct 
> fix is to return UDS_CORRUPT_DATA when this assertion fails.

Replying to myself, feel free to correct this patch, or I can probably 
get to posting a proper fix later in the week.

> Matt
> 
> 
>>           if (memcmp(header.magic, MAGIC_START_5, MAGIC_SIZE) != 0) {
>>               return vdo_log_warning_strerror(UDS_CORRUPT_DATA,
>> @@ -924,11 +922,9 @@ static int start_restoring_volume_index(struct 
>> volume_index *volume_index,
>>           offset += MAGIC_SIZE;
>>           decode_u32_le(buffer, &offset, &header.sparse_sample_rate);
>> -        result = VDO_ASSERT(offset == sizeof(buffer),
>> +        VDO_ASSERT_LOG_ONLY(offset == sizeof(buffer),
>>                       "%zu bytes decoded of %zu expected", offset,
>>                       sizeof(buffer));
>> -        if (result != VDO_SUCCESS)
>> -            result = UDS_CORRUPT_DATA;
>>           if (memcmp(header.magic, MAGIC_START_6, MAGIC_SIZE) != 0)
>>               return vdo_log_warning_strerror(UDS_CORRUPT_DATA,
>