From: Eric Biggers ebiggers@google.com
When converting an inline directory to a regular one, f2fs is leaking uninitialized memory to disk because it doesn't initialize the entire directory block. Fix this by zero-initializing the block.
This bug was introduced by commit 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry"), which didn't consider the security implications of leaking uninitialized memory to disk.
This was found by running xfstest generic/435 on a KMSAN-enabled kernel.
Fixes: 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry") Cc: stable@vger.kernel.org # v4.3+ Signed-off-by: Eric Biggers ebiggers@google.com --- fs/f2fs/inline.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index 08e302d32118d..72269e7efd260 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -421,18 +421,17 @@ static int f2fs_move_inline_dirents(struct inode *dir, struct page *ipage,
dentry_blk = page_address(page);
+ /* + * Start by zeroing the full block, to ensure that all unused space is + * zeroed and no uninitialized memory is leaked to disk. + */ + memset(dentry_blk, 0, F2FS_BLKSIZE); + make_dentry_ptr_inline(dir, &src, inline_dentry); make_dentry_ptr_block(dir, &dst, dentry_blk);
/* copy data from inline dentry block to new dentry block */ memcpy(dst.bitmap, src.bitmap, src.nr_bitmap); - memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap); - /* - * we do not need to zero out remainder part of dentry and filename - * field, since we have used bitmap for marking the usage status of - * them, besides, we can also ignore copying/zeroing reserved space - * of dentry block, because them haven't been used so far. - */ memcpy(dst.dentry, src.dentry, SIZE_OF_DIR_ENTRY * src.max); memcpy(dst.filename, src.filename, src.max * F2FS_SLOT_LEN);
base-commit: 7a2b15cfa8dbbd54beb4e2ce7b2f42eb0ad00425
On Mon, Jan 23, 2023 at 8:05 AM Eric Biggers ebiggers@kernel.org wrote:
From: Eric Biggers ebiggers@google.com
When converting an inline directory to a regular one, f2fs is leaking uninitialized memory to disk because it doesn't initialize the entire directory block. Fix this by zero-initializing the block.
This bug was introduced by commit 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry"), which didn't consider the security implications of leaking uninitialized memory to disk.
This was found by running xfstest generic/435 on a KMSAN-enabled kernel.
Out of curiosity, did you add any extra annotations to detect uninit writes to the disk?
On Mon, Jan 23, 2023 at 09:58:22AM +0100, Alexander Potapenko wrote:
On Mon, Jan 23, 2023 at 8:05 AM Eric Biggers ebiggers@kernel.org wrote:
From: Eric Biggers ebiggers@google.com
When converting an inline directory to a regular one, f2fs is leaking uninitialized memory to disk because it doesn't initialize the entire directory block. Fix this by zero-initializing the block.
This bug was introduced by commit 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry"), which didn't consider the security implications of leaking uninitialized memory to disk.
This was found by running xfstest generic/435 on a KMSAN-enabled kernel.
Out of curiosity, did you add any extra annotations to detect uninit writes to the disk?
No. This is the report I got:
[ 145.280969] ===================================================== [ 145.285368] BUG: KMSAN: uninit-value in virtqueue_add+0x1ba5/0x6ac0 [ 145.289123] virtqueue_add+0x1ba5/0x6ac0 [ 145.291539] virtqueue_add_sgs+0x2ae/0x2c0 [ 145.294036] virtio_queue_rqs+0xa59/0x1270 [ 145.296566] blk_mq_flush_plug_list+0x34a/0x9e0 [ 145.299389] __blk_flush_plug+0x6b5/0x760 [ 145.301801] blk_finish_plug+0x92/0xd0 [ 145.304084] __f2fs_write_data_pages+0x3953/0x4140 [ 145.307011] f2fs_write_data_pages+0xf1/0x120 [ 145.309665] do_writepages+0x45c/0x980 [ 145.312017] filemap_fdatawrite+0x1d2/0x260 [ 145.314533] f2fs_sync_dirty_inodes+0x3a3/0xa40 [ 145.317272] f2fs_write_checkpoint+0xecc/0x2890 [ 145.320013] __checkpoint_and_complete_reqs+0x12e/0x660 [ 145.323126] issue_checkpoint_thread+0x10a/0x420 [ 145.325900] kthread+0x35c/0x490 [ 145.327995] ret_from_fork+0x1f/0x30 [ 145.330208] [ 145.331242] Uninit was created at: [ 145.333399] __alloc_pages+0x6f9/0xc90 [ 145.335716] alloc_pages+0x8aa/0xa20 [ 145.337921] folio_alloc+0x34/0x40 [ 145.340069] __filemap_get_folio+0xa9f/0x11a0 [ 145.342719] pagecache_get_page+0x5b/0x1b0 [ 145.345209] grab_cache_page_write_begin+0xa5/0xc0 [ 145.348096] do_convert_inline_dir+0x8dc/0x3680 [ 145.350812] f2fs_add_inline_entry+0x76a/0x11a0 [ 145.353532] f2fs_do_add_link+0x59b/0x970 [ 145.356009] f2fs_create+0x5b6/0x8c0 [ 145.358213] path_openat+0x25a5/0x46e0 [ 145.360547] do_filp_open+0x2ab/0x700 [ 145.362785] do_sys_openat2+0x20e/0x780 [ 145.365138] __x64_sys_openat+0x259/0x360 [ 145.367562] do_syscall_64+0x41/0x90 [ 145.369819] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 145.372895] [ 145.373900] Bytes 2043-2050 of 4096 are uninitialized [ 145.376857] Memory access of size 4096 starts at ffff888076818000 [ 145.380391] [ 145.381429] CPU: 1 PID: 1907 Comm: f2fs_ckpt-254:3 Not tainted 6.2.0-rc5-next-20230123 #1 [ 145.386152] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.1-0-g3208b098f51a-prebuilt.qemu.org 04/01/2014 [ 145.392541] ===================================================== [ 145.396096] Disabling lock debugging due to kernel taint [ 145.399241] =====================================================
Out of curiosity, did you add any extra annotations to detect uninit writes to the disk?
No. This is the report I got:
[ 145.280969] ===================================================== [ 145.285368] BUG: KMSAN: uninit-value in virtqueue_add+0x1ba5/0x6ac0
Oh cool, nice to see these are actually working :)
On 2023/1/23 15:04, Eric Biggers wrote:
From: Eric Biggers ebiggers@google.com
When converting an inline directory to a regular one, f2fs is leaking uninitialized memory to disk because it doesn't initialize the entire directory block. Fix this by zero-initializing the block.
This bug was introduced by commit 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry"), which didn't consider the security implications of leaking uninitialized memory to disk.
This was found by running xfstest generic/435 on a KMSAN-enabled kernel.
Fixes: 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry") Cc: stable@vger.kernel.org # v4.3+ Signed-off-by: Eric Biggers ebiggers@google.com
Reviewed-by: Chao Yu chao@kernel.org
Thanks,
Hello:
This patch was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim jaegeuk@kernel.org:
On Sun, 22 Jan 2023 23:04:14 -0800 you wrote:
From: Eric Biggers ebiggers@google.com
When converting an inline directory to a regular one, f2fs is leaking uninitialized memory to disk because it doesn't initialize the entire directory block. Fix this by zero-initializing the block.
This bug was introduced by commit 4ec17d688d74 ("f2fs: avoid unneeded initializing when converting inline dentry"), which didn't consider the security implications of leaking uninitialized memory to disk.
[...]
Here is the summary with links: - [f2fs-dev] f2fs: fix information leak in f2fs_move_inline_dirents() https://git.kernel.org/jaegeuk/f2fs/c/638ce5437867
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org
-
Alexander Potapenko -
Chao Yu -
Eric Biggers -
patchwork-bot+f2fs＠kernel.org