From b9880ec496293d7a68e4f9b4e59d875b4ae77255 Mon Sep 17 00:00:00 2001 From: Sahitya Tummala Date: Wed, 17 Jun 2020 01:55:58 -0700 Subject: [PATCH] FROMLIST: f2fs: fix use-after-free when accessing bio->bi_crypt_context There could be a potential race between these two paths below, leading to use-after-free when accessing bio->bi_crypt_context. f2fs_write_cache_pages ->f2fs_do_write_data_page on page#1 ->f2fs_inplace_write_data ->f2fs_merge_page_bio ->add_bio_entry ->f2fs_do_write_data_page on page#2 ->f2fs_inplace_write_data ->f2fs_merge_page_bio ->f2fs_crypt_mergeable_bio ->fscrypt_mergeable_bio f2fs_write_begin on page#1 ->f2fs_wait_on_page_writeback ->f2fs_submit_merged_ipu_write ->__submit_bio The bio gets completed, calling bio_endio ->bio_uninit ->bio_crypt_free_ctx ->use-after-free issue Fix this by moving f2fs_crypt_mergeable_bio() check within add_ipu_page() so that it's done under bio_list_lock to prevent the above race. Bug: 137270441 Link: https://lore.kernel.org/linux-f2fs-devel/1592193588-21701-1-git-send-email-stummala@codeaurora.org/ Fixes: fb710731b64b ("f2fs: add inline encryption support") Signed-off-by: Sahitya Tummala Signed-off-by: Satya Tangirala Change-Id: I1bd2cfa430423ba2a8d7c1da505322ded097cd9e --- fs/f2fs/data.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index d091675ef116..076ec4324a58 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -834,9 +834,10 @@ static void del_bio_entry(struct bio_entry *be) kmem_cache_free(bio_entry_slab, be); } -static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio, +static int add_ipu_page(struct f2fs_io_info *fio, struct bio **bio, struct page *page) { + struct f2fs_sb_info *sbi = fio->sbi; enum temp_type temp; bool found = false; int ret = -EAGAIN; @@ -853,13 +854,18 @@ static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio, found = true; - if (bio_add_page(*bio, page, PAGE_SIZE, 0) == - PAGE_SIZE) { + if (page_is_mergeable(sbi, *bio, *fio->last_block, + fio->new_blkaddr) && + f2fs_crypt_mergeable_bio(*bio, + fio->page->mapping->host, + fio->page->index, fio) && + bio_add_page(*bio, page, PAGE_SIZE, 0) == + PAGE_SIZE) { ret = 0; break; } - /* bio is full */ + /* page can't be merged into bio; submit the bio */ del_bio_entry(be); __submit_bio(sbi, *bio, DATA); break; @@ -944,11 +950,6 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio) trace_f2fs_submit_page_bio(page, fio); f2fs_trace_ios(fio, 0); - if (bio && (!page_is_mergeable(fio->sbi, bio, *fio->last_block, - fio->new_blkaddr) || - !f2fs_crypt_mergeable_bio(bio, fio->page->mapping->host, - fio->page->index, fio))) - f2fs_submit_merged_ipu_write(fio->sbi, &bio, NULL); alloc_new: if (!bio) { bio = __bio_alloc(fio, BIO_MAX_PAGES); @@ -960,7 +961,7 @@ alloc_new: add_bio_entry(fio->sbi, bio, page, fio->temp); } else { - if (add_ipu_page(fio->sbi, &bio, page)) + if (add_ipu_page(fio, &bio, page)) goto alloc_new; }