[lvc-project] [PATCH] f2fs: Add a flush_work() call before destroying raw_super

[aitsygunka at yandex.ru]

> On Wed, 29. Apr 13:32, Andrey Tsygunka wrote:
> 
>> KASAN reports a use-after-free in f2fs_record_stop_reason(), called
>> from f2fs_record_error_work() during the execution of s_error_work.
>>
>> The error occurs when attempting to access sbi->raw_super after it
>> was freed in f2fs_fill_super().
>>
>> The reason for this error is that some superblock initialization error
>> handling in f2fs_fill_super() frees raw_super while s_error_work has
>> not yet completed execution, leading to a race condition
>> and use-after-free.
> 
> Добрый вечер. Спасибо за патч.
> 

Добрый день.

> В описании выше стоит подуточнить, что именно за ошибку инициализации
> схватил репродьюсер. На пути обработки ошибки в f2fs_fill_super() есть
> flush этой работы, но по-видимому он не вызывается (?).
>

Да, все верно, репродьюсер приводит к трассе в f2fs_fill_super(), где по ошибке
выполняется переход на метку free_page_array_cache.

> [...]
> 
> Приветствуется "Found by Linux Verification Center ..."
> 

Учту.

>> Fixes: b62e71be2110 ("f2fs: support errors=remount-ro|continue|panic mountoption")
>> Signed-off-by: Andrey Tsygunka <aitsygunka at yandex.ru>
>> ---
>> fs/f2fs/super.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index ccf806b676f5..e9fe57e4aef2 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -5449,6 +5449,8 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
>> /* no need to free dummy_enc_policy, we just keep it in ctx when failed */
>> swap(F2FS_CTX_INFO(ctx).dummy_enc_policy, F2FS_OPTION(sbi).dummy_enc_policy);
>> free_sb_buf:
>> + /* flush s_error_work before raw_super destroy */
> 
> Комментарий в такой форме излишен, т.к. дублирует, что делает код. Если
> хочется добавить комментарий, можно написать, _почему_ здесь требуется
> *дополнительный* flush.
> 
> А в целом почему имеющийся flush_work() не передвинуть на эту позицию,
> чтобы он соотносился с INIT_WORK() выше? После INIT_WORK какая-либо
> ошибка в какой-нибудь из функций инициализации может поставить
> s_error_work в очередь на выполнение, и без flush не обойтись. Не сказал
> бы, что дублирование - идеальный вариант, тут есть над чем поразмыслить.
> 

Согласен, не совсем красивое решение, но т.к. многие из обработчиков могут приводить 
к вызову f2fs_record_error_work(), я пока не придумал какого-то подходящего решения,
чтобы избежать дублирования flush_work().
Пока есть мысли перенести flush_work() в обработчик ошибки перед переходом на метку.

>> + flush_work(&sbi->s_error_work);
>> kfree(raw_super);
>> free_sbi:
>> #ifdef CONFIG_DEBUG_LOCK_ALLOC
>> --
>> 2.25.1