* [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
@ 2023-01-23 7:04 Eric Biggers
2023-01-23 8:58 ` Alexander Potapenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Biggers @ 2023-01-23 7:04 UTC (permalink / raw)
To: linux-f2fs-devel, Jaegeuk Kim, Chao Yu; +Cc: Alexander Potapenko, stable
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
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
2023-01-23 7:04 [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents() Eric Biggers
@ 2023-01-23 8:58 ` Alexander Potapenko
2023-01-23 18:19 ` Eric Biggers
2023-01-29 9:57 ` Chao Yu
2023-01-30 23:00 ` [f2fs-dev] " patchwork-bot+f2fs
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Potapenko @ 2023-01-23 8:58 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-f2fs-devel, Jaegeuk Kim, Chao Yu, stable
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
2023-01-23 8:58 ` Alexander Potapenko
@ 2023-01-23 18:19 ` Eric Biggers
2023-01-25 10:10 ` Alexander Potapenko
0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2023-01-23 18:19 UTC (permalink / raw)
To: Alexander Potapenko; +Cc: linux-f2fs-devel, Jaegeuk Kim, Chao Yu, stable
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] =====================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
2023-01-23 18:19 ` Eric Biggers
@ 2023-01-25 10:10 ` Alexander Potapenko
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Potapenko @ 2023-01-25 10:10 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-f2fs-devel, Jaegeuk Kim, Chao Yu, stable
> > 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 :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
2023-01-23 7:04 [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents() Eric Biggers
2023-01-23 8:58 ` Alexander Potapenko
@ 2023-01-29 9:57 ` Chao Yu
2023-01-30 23:00 ` [f2fs-dev] " patchwork-bot+f2fs
2 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2023-01-29 9:57 UTC (permalink / raw)
To: Eric Biggers, linux-f2fs-devel, Jaegeuk Kim; +Cc: Alexander Potapenko, stable
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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents()
2023-01-23 7:04 [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents() Eric Biggers
2023-01-23 8:58 ` Alexander Potapenko
2023-01-29 9:57 ` Chao Yu
@ 2023-01-30 23:00 ` patchwork-bot+f2fs
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+f2fs @ 2023-01-30 23:00 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-f2fs-devel, jaegeuk, chao, glider, stable
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-30 23:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-23 7:04 [PATCH] f2fs: fix information leak in f2fs_move_inline_dirents() Eric Biggers
2023-01-23 8:58 ` Alexander Potapenko
2023-01-23 18:19 ` Eric Biggers
2023-01-25 10:10 ` Alexander Potapenko
2023-01-29 9:57 ` Chao Yu
2023-01-30 23:00 ` [f2fs-dev] " patchwork-bot+f2fs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).