* [PATCH v2] ntfs: fix use-after-free in ntfs_ucsncmp()
@ 2022-07-07 10:53 ChenXiaoSong
2022-07-09 0:54 ` Hawkins Jiawei
0 siblings, 1 reply; 3+ messages in thread
From: ChenXiaoSong @ 2022-07-07 10:53 UTC (permalink / raw)
To: anton, akpm
Cc: linux-ntfs-dev, linux-kernel, chenxiaosong2, liuyongqiang13,
yi.zhang, zhangxiaoxu5
Syzkaller reported use-after-free bug as follows:
==================================================================
BUG: KASAN: use-after-free in ntfs_ucsncmp+0x123/0x130
Read of size 2 at addr ffff8880751acee8 by task a.out/879
CPU: 7 PID: 879 Comm: a.out Not tainted 5.19.0-rc4-next-20220630-00001-gcc5218c8bd2c-dirty #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x1c0/0x2b0
print_address_description.constprop.0.cold+0xd4/0x484
print_report.cold+0x55/0x232
kasan_report+0xbf/0xf0
ntfs_ucsncmp+0x123/0x130
ntfs_are_names_equal.cold+0x2b/0x41
ntfs_attr_find+0x43b/0xb90
ntfs_attr_lookup+0x16d/0x1e0
ntfs_read_locked_attr_inode+0x4aa/0x2360
ntfs_attr_iget+0x1af/0x220
ntfs_read_locked_inode+0x246c/0x5120
ntfs_iget+0x132/0x180
load_system_files+0x1cc6/0x3480
ntfs_fill_super+0xa66/0x1cf0
mount_bdev+0x38d/0x460
legacy_get_tree+0x10d/0x220
vfs_get_tree+0x93/0x300
do_new_mount+0x2da/0x6d0
path_mount+0x496/0x19d0
__x64_sys_mount+0x284/0x300
do_syscall_64+0x3b/0xc0
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7f3f2118d9ea
Code: 48 8b 0d a9 f4 0b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 76 f4 0b 00 f7 d8 64 89 01 48
RSP: 002b:00007ffc269deac8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3f2118d9ea
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007ffc269dec00
RBP: 00007ffc269dec80 R08: 00007ffc269deb00 R09: 00007ffc269dec44
R10: 0000000000000000 R11: 0000000000000202 R12: 000055f81ab1d220
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>
The buggy address belongs to the physical page:
page:0000000085430378 refcount:1 mapcount:1 mapping:0000000000000000 index:0x555c6a81d pfn:0x751ac
memcg:ffff888101f7e180
anon flags: 0xfffffc00a0014(uptodate|lru|mappedtodisk|swapbacked|node=0|zone=1|lastcpupid=0x1fffff)
raw: 000fffffc00a0014 ffffea0001bf2988 ffffea0001de2448 ffff88801712e201
raw: 0000000555c6a81d 0000000000000000 0000000100000000 ffff888101f7e180
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8880751acd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8880751ace00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8880751ace80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
ffff8880751acf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8880751acf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
The reason is that struct ATTR_RECORD->name_offset is 6485, end address of
name string is out of bounds.
Fix this by adding sanity check on end address of attibute name string.
Signed-off-by: ChenXiaoSong <chenxiaosong2@huawei.com>
---
v1 -> v2: remove redundant statement
fs/ntfs/attrib.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 4de597a83b88..9ab8766872d2 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -592,8 +592,12 @@ static int ntfs_attr_find(const ATTR_TYPE type, const ntfschar *name,
a = (ATTR_RECORD*)((u8*)ctx->attr +
le32_to_cpu(ctx->attr->length));
for (;; a = (ATTR_RECORD*)((u8*)a + le32_to_cpu(a->length))) {
- if ((u8*)a < (u8*)ctx->mrec || (u8*)a > (u8*)ctx->mrec +
- le32_to_cpu(ctx->mrec->bytes_allocated))
+ 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;
+ if ((u8*)a < (u8*)ctx->mrec || (u8*)a > mrec_end ||
+ name_end > mrec_end)
break;
ctx->attr = a;
if (unlikely(le32_to_cpu(a->type) > le32_to_cpu(type) ||
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ntfs: fix use-after-free in ntfs_ucsncmp()
2022-07-07 10:53 [PATCH v2] ntfs: fix use-after-free in ntfs_ucsncmp() ChenXiaoSong
@ 2022-07-09 0:54 ` Hawkins Jiawei
2022-07-09 3:14 ` chenxiaosong (A)
0 siblings, 1 reply; 3+ messages in thread
From: Hawkins Jiawei @ 2022-07-09 0:54 UTC (permalink / raw)
To: chenxiaosong2
Cc: akpm, anton, linux-kernel, linux-ntfs-dev, liuyongqiang13,
yi.zhang, zhangxiaoxu5, 18801353760, skhan, paskripkin,
linux-kernel-mentees
> Fix this by adding sanity check on end address of attibute name string.
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index 4de597a83b88..9ab8766872d2 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -592,8 +592,12 @@ static int ntfs_attr_find(const ATTR_TYPE type, const ntfschar *name,
> a = (ATTR_RECORD*)((u8*)ctx->attr +
> le32_to_cpu(ctx->attr->length));
> for (;; a = (ATTR_RECORD*)((u8*)a + le32_to_cpu(a->length))) {
> - if ((u8*)a < (u8*)ctx->mrec || (u8*)a > (u8*)ctx->mrec +
> - le32_to_cpu(ctx->mrec->bytes_allocated))
> + 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;
In my opinion, name_length field just means the number of characters,
yet each character is a ntfschar type. So name should be
name_length * sizeof(ntfschar) bytes. The example is at
https://elixir.bootlin.com/linux/v5.19-rc5/source/fs/ntfs/attrib.c#L1667
> + if ((u8*)a < (u8*)ctx->mrec || (u8*)a > mrec_end ||
> + name_end > mrec_end)
> break;
> ctx->attr = a;
> if (unlikely(le32_to_cpu(a->type) > le32_to_cpu(type) ||
> --
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ntfs: fix use-after-free in ntfs_ucsncmp()
2022-07-09 0:54 ` Hawkins Jiawei
@ 2022-07-09 3:14 ` chenxiaosong (A)
0 siblings, 0 replies; 3+ messages in thread
From: chenxiaosong (A) @ 2022-07-09 3:14 UTC (permalink / raw)
To: Hawkins Jiawei
Cc: akpm, anton, linux-kernel, linux-ntfs-dev, liuyongqiang13,
yi.zhang, zhangxiaoxu5, 18801353760, skhan, paskripkin,
linux-kernel-mentees
在 2022/7/9 8:54, Hawkins Jiawei 写道:> In my opinion, name_length field
just means the number of characters,
> yet each character is a ntfschar type. So name should be
> name_length * sizeof(ntfschar) bytes. The example is at
> https://elixir.bootlin.com/linux/v5.19-rc5/source/fs/ntfs/attrib.c#L1667
Yes, thank you for your reply, I will send v3 patch.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-09 3:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 10:53 [PATCH v2] ntfs: fix use-after-free in ntfs_ucsncmp() ChenXiaoSong
2022-07-09 0:54 ` Hawkins Jiawei
2022-07-09 3:14 ` chenxiaosong (A)
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).