From 6448353549158d425664b22be763502a3d44d7f5 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 23 Nov 2022 09:24:46 +0100 Subject: [PATCH] 5.10-stable patches added patches: ntfs-check-overflow-when-iterating-attr_records.patch ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch ntfs-fix-use-after-free-in-ntfs_attr_find.patch --- ...overflow-when-iterating-attr_records.patch | 52 ++++++++ ...out-of-bounds-read-in-ntfs_attr_find.patch | 116 ++++++++++++++++ ...fix-use-after-free-in-ntfs_attr_find.patch | 124 ++++++++++++++++++ queue-5.10/series | 3 + 4 files changed, 295 insertions(+) create mode 100644 queue-5.10/ntfs-check-overflow-when-iterating-attr_records.patch create mode 100644 queue-5.10/ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch create mode 100644 queue-5.10/ntfs-fix-use-after-free-in-ntfs_attr_find.patch diff --git a/queue-5.10/ntfs-check-overflow-when-iterating-attr_records.patch b/queue-5.10/ntfs-check-overflow-when-iterating-attr_records.patch new file mode 100644 index 00000000000..9d473bc714f --- /dev/null +++ b/queue-5.10/ntfs-check-overflow-when-iterating-attr_records.patch @@ -0,0 +1,52 @@ +From 63095f4f3af59322bea984a6ae44337439348fe0 Mon Sep 17 00:00:00 2001 +From: Hawkins Jiawei +Date: Thu, 1 Sep 2022 00:09:38 +0800 +Subject: ntfs: check overflow when iterating ATTR_RECORDs + +From: Hawkins Jiawei + +commit 63095f4f3af59322bea984a6ae44337439348fe0 upstream. + +Kernel iterates over ATTR_RECORDs in mft record in ntfs_attr_find(). +Because the ATTR_RECORDs are next to each other, kernel can get the next +ATTR_RECORD from end address of current ATTR_RECORD, through current +ATTR_RECORD length field. + +The problem is that during iteration, when kernel calculates the end +address of current ATTR_RECORD, kernel may trigger an integer overflow bug +in executing `a = (ATTR_RECORD*)((u8*)a + le32_to_cpu(a->length))`. This +may wrap, leading to a forever iteration on 32bit systems. + +This patch solves it by adding some checks on calculating end address +of current ATTR_RECORD during iteration. + +Link: https://lkml.kernel.org/r/20220831160935.3409-4-yin31149@gmail.com +Link: https://lore.kernel.org/all/20220827105842.GM2030@kadam/ +Signed-off-by: Hawkins Jiawei +Suggested-by: Dan Carpenter +Cc: Anton Altaparmakov +Cc: chenxiaosong (A) +Cc: syzkaller-bugs +Signed-off-by: Andrew Morton +Signed-off-by: Greg Kroah-Hartman +--- + fs/ntfs/attrib.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +--- a/fs/ntfs/attrib.c ++++ b/fs/ntfs/attrib.c +@@ -617,6 +617,14 @@ static int ntfs_attr_find(const ATTR_TYP + return -ENOENT; + if (unlikely(!a->length)) + break; ++ ++ /* check whether ATTR_RECORD's length wrap */ ++ if ((u8 *)a + le32_to_cpu(a->length) < (u8 *)a) ++ break; ++ /* check whether ATTR_RECORD's length is within bounds */ ++ if ((u8 *)a + le32_to_cpu(a->length) > mrec_end) ++ break; ++ + if (a->type != type) + continue; + /* diff --git a/queue-5.10/ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch b/queue-5.10/ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch new file mode 100644 index 00000000000..bf44f5755b2 --- /dev/null +++ b/queue-5.10/ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch @@ -0,0 +1,116 @@ +From 36a4d82dddbbd421d2b8e79e1cab68c8126d5075 Mon Sep 17 00:00:00 2001 +From: Hawkins Jiawei +Date: Thu, 1 Sep 2022 00:09:36 +0800 +Subject: ntfs: fix out-of-bounds read in ntfs_attr_find() + +From: Hawkins Jiawei + +commit 36a4d82dddbbd421d2b8e79e1cab68c8126d5075 upstream. + +Kernel iterates over ATTR_RECORDs in mft record in ntfs_attr_find(). To +ensure access on these ATTR_RECORDs are within bounds, kernel will do some +checking during iteration. + +The problem is that during checking whether ATTR_RECORD's name is within +bounds, kernel will dereferences the ATTR_RECORD name_offset field, before +checking this ATTR_RECORD strcture is within bounds. This problem may +result out-of-bounds read in ntfs_attr_find(), reported by Syzkaller: + +================================================================== +BUG: KASAN: use-after-free in ntfs_attr_find+0xc02/0xce0 fs/ntfs/attrib.c:597 +Read of size 2 at addr ffff88807e352009 by task syz-executor153/3607 + +[...] +Call Trace: + + __dump_stack lib/dump_stack.c:88 [inline] + dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 + print_address_description mm/kasan/report.c:317 [inline] + print_report.cold+0x2ba/0x719 mm/kasan/report.c:433 + kasan_report+0xb1/0x1e0 mm/kasan/report.c:495 + ntfs_attr_find+0xc02/0xce0 fs/ntfs/attrib.c:597 + ntfs_attr_lookup+0x1056/0x2070 fs/ntfs/attrib.c:1193 + ntfs_read_inode_mount+0x89a/0x2580 fs/ntfs/inode.c:1845 + ntfs_fill_super+0x1799/0x9320 fs/ntfs/super.c:2854 + mount_bdev+0x34d/0x410 fs/super.c:1400 + legacy_get_tree+0x105/0x220 fs/fs_context.c:610 + vfs_get_tree+0x89/0x2f0 fs/super.c:1530 + do_new_mount fs/namespace.c:3040 [inline] + path_mount+0x1326/0x1e20 fs/namespace.c:3370 + do_mount fs/namespace.c:3383 [inline] + __do_sys_mount fs/namespace.c:3591 [inline] + __se_sys_mount fs/namespace.c:3568 [inline] + __x64_sys_mount+0x27f/0x300 fs/namespace.c:3568 + do_syscall_x64 arch/x86/entry/common.c:50 [inline] + do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 + entry_SYSCALL_64_after_hwframe+0x63/0xcd + [...] + + +The buggy address belongs to the physical page: +page:ffffea0001f8d400 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x7e350 +head:ffffea0001f8d400 order:3 compound_mapcount:0 compound_pincount:0 +flags: 0xfff00000010200(slab|head|node=0|zone=1|lastcpupid=0x7ff) +raw: 00fff00000010200 0000000000000000 dead000000000122 ffff888011842140 +raw: 0000000000000000 0000000000040004 00000001ffffffff 0000000000000000 +page dumped because: kasan: bad access detected +Memory state around the buggy address: + ffff88807e351f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc + ffff88807e351f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +>ffff88807e352000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb + ^ + ffff88807e352080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb + ffff88807e352100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +================================================================== + +This patch solves it by moving the ATTR_RECORD strcture's bounds checking +earlier, then checking whether ATTR_RECORD's name is within bounds. +What's more, this patch also add some comments to improve its +maintainability. + +Link: https://lkml.kernel.org/r/20220831160935.3409-3-yin31149@gmail.com +Link: https://lore.kernel.org/all/1636796c-c85e-7f47-e96f-e074fee3c7d3@huawei.com/ +Link: https://groups.google.com/g/syzkaller-bugs/c/t_XdeKPGTR4/m/LECAuIGcBgAJ +Signed-off-by: chenxiaosong (A) +Signed-off-by: Dan Carpenter +Signed-off-by: Hawkins Jiawei +Reported-by: syzbot+5f8dcabe4a3b2c51c607@syzkaller.appspotmail.com +Tested-by: syzbot+5f8dcabe4a3b2c51c607@syzkaller.appspotmail.com +Cc: Anton Altaparmakov +Cc: syzkaller-bugs +Signed-off-by: Andrew Morton +Signed-off-by: Greg Kroah-Hartman +--- + fs/ntfs/attrib.c | 20 ++++++++++++++++---- + 1 file changed, 16 insertions(+), 4 deletions(-) + +--- a/fs/ntfs/attrib.c ++++ b/fs/ntfs/attrib.c +@@ -594,11 +594,23 @@ static int ntfs_attr_find(const ATTR_TYP + for (;; a = (ATTR_RECORD*)((u8*)a + le32_to_cpu(a->length))) { + u8 *mrec_end = (u8 *)ctx->mrec + + le32_to_cpu(ctx->mrec->bytes_allocated); +- u8 *name_end = (u8 *)a + le16_to_cpu(a->name_offset) + +- a->name_length * sizeof(ntfschar); +- if ((u8*)a < (u8*)ctx->mrec || (u8*)a > mrec_end || +- name_end > mrec_end) ++ u8 *name_end; ++ ++ /* check whether ATTR_RECORD wrap */ ++ if ((u8 *)a < (u8 *)ctx->mrec) + break; ++ ++ /* check whether Attribute Record Header is within bounds */ ++ if ((u8 *)a > mrec_end || ++ (u8 *)a + sizeof(ATTR_RECORD) > mrec_end) ++ break; ++ ++ /* check whether ATTR_RECORD's name is within bounds */ ++ name_end = (u8 *)a + le16_to_cpu(a->name_offset) + ++ a->name_length * sizeof(ntfschar); ++ if (name_end > mrec_end) ++ break; ++ + ctx->attr = a; + if (unlikely(le32_to_cpu(a->type) > le32_to_cpu(type) || + a->type == AT_END)) diff --git a/queue-5.10/ntfs-fix-use-after-free-in-ntfs_attr_find.patch b/queue-5.10/ntfs-fix-use-after-free-in-ntfs_attr_find.patch new file mode 100644 index 00000000000..b7c10d63f28 --- /dev/null +++ b/queue-5.10/ntfs-fix-use-after-free-in-ntfs_attr_find.patch @@ -0,0 +1,124 @@ +From d85a1bec8e8d552ab13163ca1874dcd82f3d1550 Mon Sep 17 00:00:00 2001 +From: Hawkins Jiawei +Date: Thu, 1 Sep 2022 00:09:34 +0800 +Subject: ntfs: fix use-after-free in ntfs_attr_find() + +From: Hawkins Jiawei + +commit d85a1bec8e8d552ab13163ca1874dcd82f3d1550 upstream. + +Patch series "ntfs: fix bugs about Attribute", v2. + +This patchset fixes three bugs relative to Attribute in record: + +Patch 1 adds a sanity check to ensure that, attrs_offset field in first +mft record loading from disk is within bounds. + +Patch 2 moves the ATTR_RECORD's bounds checking earlier, to avoid +dereferencing ATTR_RECORD before checking this ATTR_RECORD is within +bounds. + +Patch 3 adds an overflow checking to avoid possible forever loop in +ntfs_attr_find(). + +Without patch 1 and patch 2, the kernel triggersa KASAN use-after-free +detection as reported by Syzkaller. + +Although one of patch 1 or patch 2 can fix this, we still need both of +them. Because patch 1 fixes the root cause, and patch 2 not only fixes +the direct cause, but also fixes the potential out-of-bounds bug. + + +This patch (of 3): + +Syzkaller reported use-after-free read as follows: +================================================================== +BUG: KASAN: use-after-free in ntfs_attr_find+0xc02/0xce0 fs/ntfs/attrib.c:597 +Read of size 2 at addr ffff88807e352009 by task syz-executor153/3607 + +[...] +Call Trace: + + __dump_stack lib/dump_stack.c:88 [inline] + dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 + print_address_description mm/kasan/report.c:317 [inline] + print_report.cold+0x2ba/0x719 mm/kasan/report.c:433 + kasan_report+0xb1/0x1e0 mm/kasan/report.c:495 + ntfs_attr_find+0xc02/0xce0 fs/ntfs/attrib.c:597 + ntfs_attr_lookup+0x1056/0x2070 fs/ntfs/attrib.c:1193 + ntfs_read_inode_mount+0x89a/0x2580 fs/ntfs/inode.c:1845 + ntfs_fill_super+0x1799/0x9320 fs/ntfs/super.c:2854 + mount_bdev+0x34d/0x410 fs/super.c:1400 + legacy_get_tree+0x105/0x220 fs/fs_context.c:610 + vfs_get_tree+0x89/0x2f0 fs/super.c:1530 + do_new_mount fs/namespace.c:3040 [inline] + path_mount+0x1326/0x1e20 fs/namespace.c:3370 + do_mount fs/namespace.c:3383 [inline] + __do_sys_mount fs/namespace.c:3591 [inline] + __se_sys_mount fs/namespace.c:3568 [inline] + __x64_sys_mount+0x27f/0x300 fs/namespace.c:3568 + do_syscall_x64 arch/x86/entry/common.c:50 [inline] + do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 + entry_SYSCALL_64_after_hwframe+0x63/0xcd + [...] + + +The buggy address belongs to the physical page: +page:ffffea0001f8d400 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x7e350 +head:ffffea0001f8d400 order:3 compound_mapcount:0 compound_pincount:0 +flags: 0xfff00000010200(slab|head|node=0|zone=1|lastcpupid=0x7ff) +raw: 00fff00000010200 0000000000000000 dead000000000122 ffff888011842140 +raw: 0000000000000000 0000000000040004 00000001ffffffff 0000000000000000 +page dumped because: kasan: bad access detected +Memory state around the buggy address: + ffff88807e351f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc + ffff88807e351f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +>ffff88807e352000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb + ^ + ffff88807e352080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb + ffff88807e352100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +================================================================== + +Kernel will loads $MFT/$DATA's first mft record in +ntfs_read_inode_mount(). + +Yet the problem is that after loading, kernel doesn't check whether +attrs_offset field is a valid value. + +To be more specific, if attrs_offset field is larger than bytes_allocated +field, then it may trigger the out-of-bounds read bug(reported as +use-after-free bug) in ntfs_attr_find(), when kernel tries to access the +corresponding mft record's attribute. + +This patch solves it by adding the sanity check between attrs_offset field +and bytes_allocated field, after loading the first mft record. + +Link: https://lkml.kernel.org/r/20220831160935.3409-1-yin31149@gmail.com +Link: https://lkml.kernel.org/r/20220831160935.3409-2-yin31149@gmail.com +Signed-off-by: Hawkins Jiawei +Cc: Anton Altaparmakov +Cc: ChenXiaoSong +Cc: syzkaller-bugs +Cc: Dan Carpenter +Signed-off-by: Andrew Morton +Signed-off-by: Greg Kroah-Hartman +--- + fs/ntfs/inode.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +--- a/fs/ntfs/inode.c ++++ b/fs/ntfs/inode.c +@@ -1829,6 +1829,13 @@ int ntfs_read_inode_mount(struct inode * + goto err_out; + } + ++ /* Sanity check offset to the first attribute */ ++ if (le16_to_cpu(m->attrs_offset) >= le32_to_cpu(m->bytes_allocated)) { ++ ntfs_error(sb, "Incorrect mft offset to the first attribute %u in superblock.", ++ le16_to_cpu(m->attrs_offset)); ++ goto err_out; ++ } ++ + /* Need this to sanity check attribute list references to $MFT. */ + vi->i_generation = ni->seq_no = le16_to_cpu(m->sequence_number); + diff --git a/queue-5.10/series b/queue-5.10/series index 6c71548ca2e..016f676a6f5 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -144,3 +144,6 @@ gfs2-check-sb_bsize_shift-after-reading-superblock.patch gfs2-switch-from-strlcpy-to-strscpy.patch 9p-trans_fd-always-use-o_nonblock-read-write.patch mm-fs-initialize-fsdata-passed-to-write_begin-write_end-interface.patch +ntfs-fix-use-after-free-in-ntfs_attr_find.patch +ntfs-fix-out-of-bounds-read-in-ntfs_attr_find.patch +ntfs-check-overflow-when-iterating-attr_records.patch -- 2.47.3