From: "Darrick J. Wong" <djwong@kernel.org>
To: Guo Xuenan <guoxuenan@huawei.com>
Cc: david@fromorbit.com, dchinner@redhat.com,
chandan.babu@oracle.com, houtao1@huawei.com, jack.qiu@huawei.com,
linux-xfs@vger.kernel.org, yi.zhang@huawei.com,
zhengbin13@huawei.com
Subject: Re: [PATCH v4] xfs: fix exception caused by unexpected illegal bestcount in leaf dir
Date: Fri, 7 Oct 2022 09:30:31 -0700 [thread overview]
Message-ID: <Y0BUJ8qUhqJZkypd@magnolia> (raw)
In-Reply-To: <0373f977-7198-0c0e-df2c-43889afcd344@huawei.com>
On Fri, Oct 07, 2022 at 07:33:53PM +0800, Guo Xuenan wrote:
>
> On 2022/9/30 4:50, Darrick J. Wong wrote:
> > On Thu, Sep 29, 2022 at 04:51:55PM +0800, Guo Xuenan wrote:
> > > For leaf dir, In most cases, there should be as many bestfree slots
> > > as the dir data blocks that can fit under i_size (except for [1]).
> > >
> > > Root cause is we don't examin the number bestfree slots, when the slots
> > > number less than dir data blocks, if we need to allocate new dir data
> > > block and update the bestfree array, we will use the dir block number as
> > > index to assign bestfree array, while we did not check the leaf buf
> > > boundary which may cause UAF or other memory access problem. This issue
> > > can also triggered with test cases xfs/473 from fstests.
> > >
> > > According to Dave Chinner & Darrick's suggestion, adding buffer verifier
> > > to detect this abnormal situation in time.
> > > Simplify the testcase for fstest xfs/554 [1]
> > >
> > > The error log is shown as follows:
> > > ==================================================================
> > > BUG: KASAN: use-after-free in xfs_dir2_leaf_addname+0x1995/0x1ac0
> > > Write of size 2 at addr ffff88810168b000 by task touch/1552
> > > CPU: 5 PID: 1552 Comm: touch Not tainted 6.0.0-rc3+ #101
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > 1.13.0-1ubuntu1.1 04/01/2014
> > > Call Trace:
> > > <TASK>
> > > dump_stack_lvl+0x4d/0x66
> > > print_report.cold+0xf6/0x691
> > > kasan_report+0xa8/0x120
> > > xfs_dir2_leaf_addname+0x1995/0x1ac0
> > > xfs_dir_createname+0x58c/0x7f0
> > > xfs_create+0x7af/0x1010
> > > xfs_generic_create+0x270/0x5e0
> > > path_openat+0x270b/0x3450
> > > do_filp_open+0x1cf/0x2b0
> > > do_sys_openat2+0x46b/0x7a0
> > > do_sys_open+0xb7/0x130
> > > do_syscall_64+0x35/0x80
> > > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > RIP: 0033:0x7fe4d9e9312b
> > > Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0
> > > 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00
> > > f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
> > > RSP: 002b:00007ffda4c16c20 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> > > RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007fe4d9e9312b
> > > RDX: 0000000000000941 RSI: 00007ffda4c17f33 RDI: 00000000ffffff9c
> > > RBP: 00007ffda4c17f33 R08: 0000000000000000 R09: 0000000000000000
> > > R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000941
> > > R13: 00007fe4d9f631a4 R14: 00007ffda4c17f33 R15: 0000000000000000
> > > </TASK>
> > >
> > > The buggy address belongs to the physical page:
> > > page:ffffea000405a2c0 refcount:0 mapcount:0 mapping:0000000000000000
> > > index:0x0 pfn:0x10168b
> > > flags: 0x2fffff80000000(node=0|zone=2|lastcpupid=0x1fffff)
> > > raw: 002fffff80000000 ffffea0004057788 ffffea000402dbc8 0000000000000000
> > > raw: 0000000000000000 0000000000170000 00000000ffffffff 0000000000000000
> > > page dumped because: kasan: bad access detected
> > >
> > > Memory state around the buggy address:
> > > ffff88810168af00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > ffff88810168af80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > ffff88810168b000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > > ^
> > > ffff88810168b080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > > ffff88810168b100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > > ==================================================================
> > > Disabling lock debugging due to kernel taint
> > > 00000000: 58 44 44 33 5b 53 35 c2 00 00 00 00 00 00 00 78
> > > XDD3[S5........x
> > > XFS (sdb): Internal error xfs_dir2_data_use_free at line 1200 of file
> > > fs/xfs/libxfs/xfs_dir2_data.c. Caller
> > > xfs_dir2_data_use_free+0x28a/0xeb0
> > > CPU: 5 PID: 1552 Comm: touch Tainted: G B 6.0.0-rc3+
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > 1.13.0-1ubuntu1.1 04/01/2014
> > > Call Trace:
> > > <TASK>
> > > dump_stack_lvl+0x4d/0x66
> > > xfs_corruption_error+0x132/0x150
> > > xfs_dir2_data_use_free+0x198/0xeb0
> > > xfs_dir2_leaf_addname+0xa59/0x1ac0
> > > xfs_dir_createname+0x58c/0x7f0
> > > xfs_create+0x7af/0x1010
> > > xfs_generic_create+0x270/0x5e0
> > > path_openat+0x270b/0x3450
> > > do_filp_open+0x1cf/0x2b0
> > > do_sys_openat2+0x46b/0x7a0
> > > do_sys_open+0xb7/0x130
> > > do_syscall_64+0x35/0x80
> > > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > RIP: 0033:0x7fe4d9e9312b
> > > Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0
> > > 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00
> > > f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
> > > RSP: 002b:00007ffda4c16c20 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> > > RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007fe4d9e9312b
> > > RDX: 0000000000000941 RSI: 00007ffda4c17f46 RDI: 00000000ffffff9c
> > > RBP: 00007ffda4c17f46 R08: 0000000000000000 R09: 0000000000000001
> > > R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000941
> > > R13: 00007fe4d9f631a4 R14: 00007ffda4c17f46 R15: 0000000000000000
> > > </TASK>
> > > XFS (sdb): Corruption detected. Unmount and run xfs_repair
> > >
> > > [1] https://lore.kernel.org/all/20220928095355.2074025-1-guoxuenan@huawei.com/
> > > Reviewed-by: Hou Tao <houtao1@huawei.com>
> > > Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
> > > ---
> > > fs/xfs/libxfs/xfs_dir2_leaf.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > > index d9b66306a9a7..bf4633b228cd 100644
> > > --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> > > +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > > @@ -146,6 +146,7 @@ xfs_dir3_leaf_check_int(
> > > xfs_dir2_leaf_tail_t *ltp;
> > > int stale;
> > > int i;
> > > + xfs_dir2_db_t db;
> > > ltp = xfs_dir2_leaf_tail_p(geo, leaf);
> > > @@ -175,6 +176,9 @@ xfs_dir3_leaf_check_int(
> > > }
> > > if (hdr->ents[i].address == cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
> > > stale++;
> > > + db = xfs_dir2_dataptr_to_db(geo, be32_to_cpu(hdr->ents[i].address));
> > > + if (db >= be32_to_cpu(ltp->bestcount))
> > > + return __this_address;
> > What about LEAFN blocks? Those don't have a bests[] array or a
> > bestcount because the free space info is in the third directory
> > partition, so there's nothing to check here. IIUC, only LEAF1 blocks
> > can compare bestcount to the entry addresses, right?
> Hi, Darrick:
>
> Sorry for the late reply, I was on vacation last week.I have confirmed it,
> you're right,may we
> need add an additional restriction, compare only LEAF1 block.
>
> According to the problem you pointed out, amend as below. Looking forward
> for your valuable suggestions :)
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index d9b66306a9a7..cd9ff1322445 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -175,6 +175,12 @@ xfs_dir3_leaf_check_int(
> }
> if (hdr->ents[i].address ==
> cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
> stale++;
> + if ((hdr->magic == XFS_DIR2_LEAF1_MAGIC ||
> + hdr->magic == XFS_DIR3_LEAF1_MAGIC) &&
> + xfs_dir2_dataptr_to_db(geo,
> + be32_to_cpu(hdr->ents[i].address))
> + >= be32_to_cpu(ltp->bestcount))
> + return __this_address;
That looks like a reasonable amendment to the code, though you might
hoist the leaf1 detection to a local variable so that we don't have to
do the same comparison multiple times in the same function.
bool isleaf1 = (hdr->magic == XFS_DIR2_LEAF1_MAGIC ||
hdr->magic == XFS_DIR3_LEAF1_MAGIC);
/* Leaves and bests don't overlap in leaf format. */
if (isleaf1 &&
(char *)&hdr->ents[hdr->count] > (char *)xfs_dir2_leaf_bests_p(ltp))
return __this_address;
...
if (isleaf1 &&
xfs_dir2_dataptr_to_db(geo,
be32_to_cpu(hdr->ents[i].address)) >=
be32_to_cpu(ltp->bestcount))
return __this_address;
Or something like that.
--D
> }
> if (hdr->stale != stale)
> return __this_address;
>
>
>
> Thanks
> Xuenan
> > --D
> >
> > > }
> > > if (hdr->stale != stale)
> > > return __this_address;
> > > --
> > > 2.25.1
> > >
> > .
>
> --
> Guo Xuenan
> -----------------------------
> Email: guoxuenan@huawei.com
>
next prev parent reply other threads:[~2022-10-07 16:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-31 12:16 [PATCH v2] xfs: fix uaf when leaf dir bestcount not match with dir data blocks Guo Xuenan
2022-09-12 1:31 ` Dave Chinner
2022-09-14 16:30 ` Darrick J. Wong
2022-09-28 10:06 ` [PATCH v3] xfs: fix expection caused by unexpected illegal bestcount in leaf dir Guo Xuenan
2022-09-29 8:51 ` [PATCH v4] xfs: fix exception " Guo Xuenan
2022-09-29 20:50 ` Darrick J. Wong
2022-10-07 11:33 ` Guo Xuenan
2022-10-07 16:30 ` Darrick J. Wong [this message]
2022-10-08 3:36 ` [PATCH v5] " Guo Xuenan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y0BUJ8qUhqJZkypd@magnolia \
--to=djwong@kernel.org \
--cc=chandan.babu@oracle.com \
--cc=david@fromorbit.com \
--cc=dchinner@redhat.com \
--cc=guoxuenan@huawei.com \
--cc=houtao1@huawei.com \
--cc=jack.qiu@huawei.com \
--cc=linux-xfs@vger.kernel.org \
--cc=yi.zhang@huawei.com \
--cc=zhengbin13@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).