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

[Fedor Pchelkin]

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 этой работы, но по-видимому он не вызывается (?).

[...]

Приветствуется "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 не обойтись.  Не сказал
бы, что дублирование - идеальный вариант, тут есть над чем поразмыслить.

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