* [PATCH] f2fs: fix to do sanity check on summary info
@ 2022-09-13 2:25 ` Chao Yu
0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2022-09-13 2:25 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable, Wenqing Liu
As Wenqing Liu reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=216456
BUG: KASAN: use-after-free in recover_data+0x63ae/0x6ae0 [f2fs]
Read of size 4 at addr ffff8881464dcd80 by task mount/1013
CPU: 3 PID: 1013 Comm: mount Tainted: G W 6.0.0-rc4 #1
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
Call Trace:
dump_stack_lvl+0x45/0x5e
print_report.cold+0xf3/0x68d
kasan_report+0xa8/0x130
recover_data+0x63ae/0x6ae0 [f2fs]
f2fs_recover_fsync_data+0x120d/0x1fc0 [f2fs]
f2fs_fill_super+0x4665/0x61e0 [f2fs]
mount_bdev+0x2cf/0x3b0
legacy_get_tree+0xed/0x1d0
vfs_get_tree+0x81/0x2b0
path_mount+0x47e/0x19d0
do_mount+0xce/0xf0
__x64_sys_mount+0x12c/0x1a0
do_syscall_64+0x38/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
The root cause is: in fuzzed image, SSA table is corrupted: ofs_in_node
is larger than ADDRS_PER_PAGE(), result in out-of-range access on 4k-size
page.
- recover_data
- do_recover_data
- check_index_in_prev_nodes
- f2fs_data_blkaddr
This patch adds sanity check on summary info in recovery and GC flow
in where the flows rely on them.
After patch:
[ 29.310883] F2FS-fs (loop0): Inconsistent ofs_in_node:65286 in summary, ino:0, nid:6, max:1018
Cc: <stable@kernel.org>
Reported-by: Wenqing Liu <wenqingliu0120@gmail.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/gc.c | 10 +++++++++-
fs/f2fs/recovery.c | 15 ++++++++++++---
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index fd400d148afb..3a820e5cdaee 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1078,7 +1078,7 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
{
struct page *node_page;
nid_t nid;
- unsigned int ofs_in_node;
+ unsigned int ofs_in_node, max_addrs;
block_t source_blkaddr;
nid = le32_to_cpu(sum->nid);
@@ -1104,6 +1104,14 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
return false;
}
+ max_addrs = IS_INODE(node_page) ? DEF_ADDRS_PER_INODE :
+ DEF_ADDRS_PER_BLOCK;
+ if (ofs_in_node >= max_addrs) {
+ f2fs_err(sbi, "Inconsistent ofs_in_node:%u in summary, ino:%u, nid:%u, max:%u",
+ ofs_in_node, dni->ino, dni->nid, max_addrs);
+ return false;
+ }
+
*nofs = ofs_of_node(node_page);
source_blkaddr = data_blkaddr(NULL, node_page, ofs_in_node);
f2fs_put_page(node_page, 1);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 8326003e6918..9b361fff30aa 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -474,7 +474,7 @@ static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
struct dnode_of_data tdn = *dn;
nid_t ino, nid;
struct inode *inode;
- unsigned int offset;
+ unsigned int offset, ofs_in_node, max_addrs;
block_t bidx;
int i;
@@ -501,15 +501,24 @@ static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
got_it:
/* Use the locked dnode page and inode */
nid = le32_to_cpu(sum.nid);
+ ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+
+ max_addrs = ADDRS_PER_PAGE(dn->node_page, dn->inode);
+ if (ofs_in_node >= max_addrs) {
+ f2fs_err(sbi, "Inconsistent ofs_in_node:%u in summary, ino:%u, nid:%u, max:%u",
+ ofs_in_node, ino, nid, max_addrs);
+ return -EFSCORRUPTED;
+ }
+
if (dn->inode->i_ino == nid) {
tdn.nid = nid;
if (!dn->inode_page_locked)
lock_page(dn->inode_page);
tdn.node_page = dn->inode_page;
- tdn.ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+ tdn.ofs_in_node = ofs_in_node;
goto truncate_out;
} else if (dn->nid == nid) {
- tdn.ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+ tdn.ofs_in_node = ofs_in_node;
goto truncate_out;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [f2fs-dev] [PATCH] f2fs: fix to do sanity check on summary info
@ 2022-09-13 2:25 ` Chao Yu
0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2022-09-13 2:25 UTC (permalink / raw)
To: jaegeuk; +Cc: stable, Wenqing Liu, linux-kernel, linux-f2fs-devel
As Wenqing Liu reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=216456
BUG: KASAN: use-after-free in recover_data+0x63ae/0x6ae0 [f2fs]
Read of size 4 at addr ffff8881464dcd80 by task mount/1013
CPU: 3 PID: 1013 Comm: mount Tainted: G W 6.0.0-rc4 #1
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
Call Trace:
dump_stack_lvl+0x45/0x5e
print_report.cold+0xf3/0x68d
kasan_report+0xa8/0x130
recover_data+0x63ae/0x6ae0 [f2fs]
f2fs_recover_fsync_data+0x120d/0x1fc0 [f2fs]
f2fs_fill_super+0x4665/0x61e0 [f2fs]
mount_bdev+0x2cf/0x3b0
legacy_get_tree+0xed/0x1d0
vfs_get_tree+0x81/0x2b0
path_mount+0x47e/0x19d0
do_mount+0xce/0xf0
__x64_sys_mount+0x12c/0x1a0
do_syscall_64+0x38/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
The root cause is: in fuzzed image, SSA table is corrupted: ofs_in_node
is larger than ADDRS_PER_PAGE(), result in out-of-range access on 4k-size
page.
- recover_data
- do_recover_data
- check_index_in_prev_nodes
- f2fs_data_blkaddr
This patch adds sanity check on summary info in recovery and GC flow
in where the flows rely on them.
After patch:
[ 29.310883] F2FS-fs (loop0): Inconsistent ofs_in_node:65286 in summary, ino:0, nid:6, max:1018
Cc: <stable@kernel.org>
Reported-by: Wenqing Liu <wenqingliu0120@gmail.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/gc.c | 10 +++++++++-
fs/f2fs/recovery.c | 15 ++++++++++++---
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index fd400d148afb..3a820e5cdaee 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1078,7 +1078,7 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
{
struct page *node_page;
nid_t nid;
- unsigned int ofs_in_node;
+ unsigned int ofs_in_node, max_addrs;
block_t source_blkaddr;
nid = le32_to_cpu(sum->nid);
@@ -1104,6 +1104,14 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
return false;
}
+ max_addrs = IS_INODE(node_page) ? DEF_ADDRS_PER_INODE :
+ DEF_ADDRS_PER_BLOCK;
+ if (ofs_in_node >= max_addrs) {
+ f2fs_err(sbi, "Inconsistent ofs_in_node:%u in summary, ino:%u, nid:%u, max:%u",
+ ofs_in_node, dni->ino, dni->nid, max_addrs);
+ return false;
+ }
+
*nofs = ofs_of_node(node_page);
source_blkaddr = data_blkaddr(NULL, node_page, ofs_in_node);
f2fs_put_page(node_page, 1);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 8326003e6918..9b361fff30aa 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -474,7 +474,7 @@ static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
struct dnode_of_data tdn = *dn;
nid_t ino, nid;
struct inode *inode;
- unsigned int offset;
+ unsigned int offset, ofs_in_node, max_addrs;
block_t bidx;
int i;
@@ -501,15 +501,24 @@ static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
got_it:
/* Use the locked dnode page and inode */
nid = le32_to_cpu(sum.nid);
+ ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+
+ max_addrs = ADDRS_PER_PAGE(dn->node_page, dn->inode);
+ if (ofs_in_node >= max_addrs) {
+ f2fs_err(sbi, "Inconsistent ofs_in_node:%u in summary, ino:%u, nid:%u, max:%u",
+ ofs_in_node, ino, nid, max_addrs);
+ return -EFSCORRUPTED;
+ }
+
if (dn->inode->i_ino == nid) {
tdn.nid = nid;
if (!dn->inode_page_locked)
lock_page(dn->inode_page);
tdn.node_page = dn->inode_page;
- tdn.ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+ tdn.ofs_in_node = ofs_in_node;
goto truncate_out;
} else if (dn->nid == nid) {
- tdn.ofs_in_node = le16_to_cpu(sum.ofs_in_node);
+ tdn.ofs_in_node = ofs_in_node;
goto truncate_out;
}
--
2.25.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] f2fs: fix to do sanity check on summary info
2022-09-13 2:25 ` [f2fs-dev] " Chao Yu
(?)
@ 2022-09-13 17:04 ` kernel test robot
-1 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-09-13 17:04 UTC (permalink / raw)
To: Chao Yu, Chao Yu; +Cc: llvm, kbuild-all
Hi Chao,
I love your patch! Perhaps something to improve:
[auto build test WARNING on jaegeuk-f2fs/dev-test]
[also build test WARNING on linus/master v6.0-rc5 next-20220913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Chao-Yu/f2fs-fix-to-do-sanity-check-on-summary-info/20220913-102758
base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: s390-randconfig-r026-20220912 (https://download.01.org/0day-ci/archive/20220914/202209140010.CjzPrtP4-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 1546df49f5a6d09df78f569e4137ddb365a3e827)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1f931953416c73680c5315b083e0519499ce43f8
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Chao-Yu/f2fs-fix-to-do-sanity-check-on-summary-info/20220913-102758
git checkout 1f931953416c73680c5315b083e0519499ce43f8
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash fs/f2fs/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> fs/f2fs/recovery.c:509:17: warning: variable 'ino' is uninitialized when used here [-Wuninitialized]
ofs_in_node, ino, nid, max_addrs);
^~~
fs/f2fs/f2fs.h:2376:35: note: expanded from macro 'f2fs_err'
f2fs_printk(sbi, KERN_ERR fmt, ##__VA_ARGS__)
^~~~~~~~~~~
fs/f2fs/recovery.c:475:11: note: initialize the variable 'ino' to silence this warning
nid_t ino, nid;
^
= 0
1 warning generated.
vim +/ino +509 fs/f2fs/recovery.c
464
465 static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
466 block_t blkaddr, struct dnode_of_data *dn)
467 {
468 struct seg_entry *sentry;
469 unsigned int segno = GET_SEGNO(sbi, blkaddr);
470 unsigned short blkoff = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
471 struct f2fs_summary_block *sum_node;
472 struct f2fs_summary sum;
473 struct page *sum_page, *node_page;
474 struct dnode_of_data tdn = *dn;
475 nid_t ino, nid;
476 struct inode *inode;
477 unsigned int offset, ofs_in_node, max_addrs;
478 block_t bidx;
479 int i;
480
481 sentry = get_seg_entry(sbi, segno);
482 if (!f2fs_test_bit(blkoff, sentry->cur_valid_map))
483 return 0;
484
485 /* Get the previous summary */
486 for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
487 struct curseg_info *curseg = CURSEG_I(sbi, i);
488
489 if (curseg->segno == segno) {
490 sum = curseg->sum_blk->entries[blkoff];
491 goto got_it;
492 }
493 }
494
495 sum_page = f2fs_get_sum_page(sbi, segno);
496 if (IS_ERR(sum_page))
497 return PTR_ERR(sum_page);
498 sum_node = (struct f2fs_summary_block *)page_address(sum_page);
499 sum = sum_node->entries[blkoff];
500 f2fs_put_page(sum_page, 1);
501 got_it:
502 /* Use the locked dnode page and inode */
503 nid = le32_to_cpu(sum.nid);
504 ofs_in_node = le16_to_cpu(sum.ofs_in_node);
505
506 max_addrs = ADDRS_PER_PAGE(dn->node_page, dn->inode);
507 if (ofs_in_node >= max_addrs) {
508 f2fs_err(sbi, "Inconsistent ofs_in_node:%u in summary, ino:%u, nid:%u, max:%u",
> 509 ofs_in_node, ino, nid, max_addrs);
510 return -EFSCORRUPTED;
511 }
512
513 if (dn->inode->i_ino == nid) {
514 tdn.nid = nid;
515 if (!dn->inode_page_locked)
516 lock_page(dn->inode_page);
517 tdn.node_page = dn->inode_page;
518 tdn.ofs_in_node = ofs_in_node;
519 goto truncate_out;
520 } else if (dn->nid == nid) {
521 tdn.ofs_in_node = ofs_in_node;
522 goto truncate_out;
523 }
524
525 /* Get the node page */
526 node_page = f2fs_get_node_page(sbi, nid);
527 if (IS_ERR(node_page))
528 return PTR_ERR(node_page);
529
530 offset = ofs_of_node(node_page);
531 ino = ino_of_node(node_page);
532 f2fs_put_page(node_page, 1);
533
534 if (ino != dn->inode->i_ino) {
535 int ret;
536
537 /* Deallocate previous index in the node page */
538 inode = f2fs_iget_retry(sbi->sb, ino);
539 if (IS_ERR(inode))
540 return PTR_ERR(inode);
541
542 ret = f2fs_dquot_initialize(inode);
543 if (ret) {
544 iput(inode);
545 return ret;
546 }
547 } else {
548 inode = dn->inode;
549 }
550
551 bidx = f2fs_start_bidx_of_node(offset, inode) +
552 le16_to_cpu(sum.ofs_in_node);
553
554 /*
555 * if inode page is locked, unlock temporarily, but its reference
556 * count keeps alive.
557 */
558 if (ino == dn->inode->i_ino && dn->inode_page_locked)
559 unlock_page(dn->inode_page);
560
561 set_new_dnode(&tdn, inode, NULL, NULL, 0);
562 if (f2fs_get_dnode_of_data(&tdn, bidx, LOOKUP_NODE))
563 goto out;
564
565 if (tdn.data_blkaddr == blkaddr)
566 f2fs_truncate_data_blocks_range(&tdn, 1);
567
568 f2fs_put_dnode(&tdn);
569 out:
570 if (ino != dn->inode->i_ino)
571 iput(inode);
572 else if (dn->inode_page_locked)
573 lock_page(dn->inode_page);
574 return 0;
575
576 truncate_out:
577 if (f2fs_data_blkaddr(&tdn) == blkaddr)
578 f2fs_truncate_data_blocks_range(&tdn, 1);
579 if (dn->inode->i_ino == nid && !dn->inode_page_locked)
580 unlock_page(dn->inode_page);
581 return 0;
582 }
583
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-13 17:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-13 2:25 [PATCH] f2fs: fix to do sanity check on summary info Chao Yu
2022-09-13 2:25 ` [f2fs-dev] " Chao Yu
2022-09-13 17:04 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.