* [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
@ 2023-05-22 10:24 Luís Henriques
0 siblings, 0 replies; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 10:24 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, Luís Henriques
It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
fstest generic/452. After mounting a filesystem as read-only, quotas are
suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
unmounting the filesystem, an UAF access to the oinfo will eventually cause a
crash.
Cc: <stable@vger.kernel.org>
Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
fs/ocfs2/super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..988d1c076861 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
if (!sb_has_quota_loaded(sb, type))
continue;
- oinfo = sb_dqinfo(sb, type)->dqi_priv;
- cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ if (!sb_has_quota_suspended(sb, type)) {
+ oinfo = sb_dqinfo(sb, type)->dqi_priv;
+ cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ }
inode = igrab(sb->s_dquot.files[type]);
/* Turn off quotas. This will remove all dquot structures from
* memory and so they will be automatically synced to global
Cheers,
--
Luís
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
@ 2023-05-22 10:25 Luís Henriques
2023-05-22 12:01 ` Joseph Qi
0 siblings, 1 reply; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 10:25 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, Luís Henriques, stable
It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
fstest generic/452. After mounting a filesystem as read-only, quotas are
suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
unmounting the filesystem, an UAF access to the oinfo will eventually cause a
crash.
Cc: <stable@vger.kernel.org>
Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
fs/ocfs2/super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..988d1c076861 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
if (!sb_has_quota_loaded(sb, type))
continue;
- oinfo = sb_dqinfo(sb, type)->dqi_priv;
- cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ if (!sb_has_quota_suspended(sb, type)) {
+ oinfo = sb_dqinfo(sb, type)->dqi_priv;
+ cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ }
inode = igrab(sb->s_dquot.files[type]);
/* Turn off quotas. This will remove all dquot structures from
* memory and so they will be automatically synced to global
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 10:25 Luís Henriques
@ 2023-05-22 12:01 ` Joseph Qi
2023-05-22 12:23 ` Luís Henriques
0 siblings, 1 reply; 10+ messages in thread
From: Joseph Qi @ 2023-05-22 12:01 UTC (permalink / raw)
To: Luís Henriques
Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker, Heming Zhao
On 5/22/23 6:25 PM, Luís Henriques wrote:
> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
> fstest generic/452. After mounting a filesystem as read-only, quotas are
generic/452 is for testing ext4 mounted with dax and ro.
But ocfs2 doesn't support dax yet.
> suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
> unmounting the filesystem, an UAF access to the oinfo will eventually cause a
> crash.
In ocfs2_fill_super(), it won't enable quota if is a readonly mount.
Do you mean remount as readonly?
Thanks,
Joseph
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
> fs/ocfs2/super.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 0b0e6a132101..988d1c076861 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
> for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
> if (!sb_has_quota_loaded(sb, type))
> continue;
> - oinfo = sb_dqinfo(sb, type)->dqi_priv;
> - cancel_delayed_work_sync(&oinfo->dqi_sync_work);
> + if (!sb_has_quota_suspended(sb, type)) {
> + oinfo = sb_dqinfo(sb, type)->dqi_priv;
> + cancel_delayed_work_sync(&oinfo->dqi_sync_work);
> + }
> inode = igrab(sb->s_dquot.files[type]);
> /* Turn off quotas. This will remove all dquot structures from
> * memory and so they will be automatically synced to global
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 12:01 ` Joseph Qi
@ 2023-05-22 12:23 ` Luís Henriques
2023-05-22 12:36 ` Heming Zhao
2023-05-23 2:41 ` Joseph Qi
0 siblings, 2 replies; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 12:23 UTC (permalink / raw)
To: Joseph Qi
Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker, Heming Zhao
Joseph Qi <joseph.qi@linux.alibaba.com> writes:
> On 5/22/23 6:25 PM, Luís Henriques wrote:
>> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
>> fstest generic/452. After mounting a filesystem as read-only, quotas are
>
> generic/452 is for testing ext4 mounted with dax and ro.
> But ocfs2 doesn't support dax yet.
Right, but I think it's still useful to run the 'generic' test-suite in a
filesystem. We can always find issues in the test itself or, in this
case, a bug in the filesystem.
>> suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
>> unmounting the filesystem, an UAF access to the oinfo will eventually cause a
>> crash.
>
> In ocfs2_fill_super(), it won't enable quota if is a readonly mount.
> Do you mean remount as readonly?
Yes, sorry. Instead of "mounting", the patch changelog should say
"After remounting a filesystem as read-only..."
Cheers,
--
Luís
>
> Thanks,
> Joseph
>
>>
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Luís Henriques <lhenriques@suse.de>
>> ---
>> fs/ocfs2/super.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>> index 0b0e6a132101..988d1c076861 100644
>> --- a/fs/ocfs2/super.c
>> +++ b/fs/ocfs2/super.c
>> @@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
>> for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
>> if (!sb_has_quota_loaded(sb, type))
>> continue;
>> - oinfo = sb_dqinfo(sb, type)->dqi_priv;
>> - cancel_delayed_work_sync(&oinfo->dqi_sync_work);
>> + if (!sb_has_quota_suspended(sb, type)) {
>> + oinfo = sb_dqinfo(sb, type)->dqi_priv;
>> + cancel_delayed_work_sync(&oinfo->dqi_sync_work);
>> + }
>> inode = igrab(sb->s_dquot.files[type]);
>> /* Turn off quotas. This will remove all dquot structures from
>> * memory and so they will be automatically synced to global
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 12:23 ` Luís Henriques
@ 2023-05-22 12:36 ` Heming Zhao
2023-05-22 13:22 ` Luís Henriques
2023-05-23 2:41 ` Joseph Qi
1 sibling, 1 reply; 10+ messages in thread
From: Heming Zhao @ 2023-05-22 12:36 UTC (permalink / raw)
To: Luís Henriques
Cc: Joseph Qi, ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker
On Mon, May 22, 2023 at 01:23:07PM +0100, Luís Henriques wrote:
> Joseph Qi <joseph.qi@linux.alibaba.com> writes:
>
> > On 5/22/23 6:25 PM, Luís Henriques wrote:
> >> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
> >> fstest generic/452. After mounting a filesystem as read-only, quotas are
> >
> > generic/452 is for testing ext4 mounted with dax and ro.
> > But ocfs2 doesn't support dax yet.
>
> Right, but I think it's still useful to run the 'generic' test-suite in a
> filesystem. We can always find issues in the test itself or, in this
> case, a bug in the filesystem.
It looks you did some special steps for 452. In my env, without changing
anything, I could pass this case successfully.
- Heming
>
> >> suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
> >> unmounting the filesystem, an UAF access to the oinfo will eventually cause a
> >> crash.
> >
> > In ocfs2_fill_super(), it won't enable quota if is a readonly mount.
> > Do you mean remount as readonly?
>
> Yes, sorry. Instead of "mounting", the patch changelog should say
>
> "After remounting a filesystem as read-only..."
>
> Cheers,
> --
> Luís
>
> >
> > Thanks,
> > Joseph
> >
> >>
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> >> ---
> >> fs/ocfs2/super.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> >> index 0b0e6a132101..988d1c076861 100644
> >> --- a/fs/ocfs2/super.c
> >> +++ b/fs/ocfs2/super.c
> >> @@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
> >> for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
> >> if (!sb_has_quota_loaded(sb, type))
> >> continue;
> >> - oinfo = sb_dqinfo(sb, type)->dqi_priv;
> >> - cancel_delayed_work_sync(&oinfo->dqi_sync_work);
> >> + if (!sb_has_quota_suspended(sb, type)) {
> >> + oinfo = sb_dqinfo(sb, type)->dqi_priv;
> >> + cancel_delayed_work_sync(&oinfo->dqi_sync_work);
> >> + }
> >> inode = igrab(sb->s_dquot.files[type]);
> >> /* Turn off quotas. This will remove all dquot structures from
> >> * memory and so they will be automatically synced to global
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 12:36 ` Heming Zhao
@ 2023-05-22 13:22 ` Luís Henriques
2023-05-23 2:43 ` Joseph Qi
0 siblings, 1 reply; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 13:22 UTC (permalink / raw)
To: Heming Zhao
Cc: Joseph Qi, ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker
Heming Zhao <heming.zhao@suse.com> writes:
> On Mon, May 22, 2023 at 01:23:07PM +0100, Luís Henriques wrote:
>> Joseph Qi <joseph.qi@linux.alibaba.com> writes:
>>
>> > On 5/22/23 6:25 PM, Luís Henriques wrote:
>> >> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
>> >> fstest generic/452. After mounting a filesystem as read-only, quotas are
>> >
>> > generic/452 is for testing ext4 mounted with dax and ro.
>> > But ocfs2 doesn't support dax yet.
>>
>> Right, but I think it's still useful to run the 'generic' test-suite in a
>> filesystem. We can always find issues in the test itself or, in this
>> case, a bug in the filesystem.
>
> It looks you did some special steps for 452. In my env, without changing
> anything, I could pass this case successfully.
No, I haven't changed anything to the test. I just make sure there's a
scratch device to be used.
Maybe you can try to enable KASAN to catch the UAF. I've found the bug
without KASAN (i.e. I saw a NULL pointer panic), but enabling it also
detects the issue -- see below.
Cheers,
--
Luís
[ 91.928109] ==================================================================
[ 91.929519] BUG: KASAN: slab-use-after-free in timer_delete+0x54/0xc0
[ 91.930869] Read of size 8 at addr ffff8880389a8208 by task umount/669
[ 91.932533] CPU: 1 PID: 669 Comm: umount Not tainted 6.4.0-rc3 #236
[ 91.933807] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552c-rebuilt.opensuse.org 04/01/2014
[ 91.936158] Call Trace:
[ 91.936678] <TASK>
[ 91.937123] dump_stack_lvl+0x32/0x50
[ 91.937909] print_report+0xc5/0x5f0
[ 91.938685] ? _raw_spin_lock_irqsave+0x72/0xc0
[ 91.939642] ? __virt_addr_valid+0xac/0x130
[ 91.940534] ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[ 91.941574] ? timer_delete+0x54/0xc0
[ 91.942357] kasan_report+0x9e/0xd0
[ 91.943110] ? timer_delete+0x54/0xc0
[ 91.943902] timer_delete+0x54/0xc0
[ 91.944643] ? __pfx_timer_delete+0x10/0x10
[ 91.945492] ? detach_if_pending+0x112/0x140
[ 91.946405] try_to_grab_pending+0x31/0x230
[ 91.947252] __cancel_work_timer+0x6c/0x270
[ 91.948102] ? __pfx___cancel_work_timer+0x10/0x10
[ 91.949073] ? try_to_grab_pending+0x31/0x230
[ 91.949956] ? __cancel_work+0xe3/0x130
[ 91.950746] ? mutex_unlock+0x6b/0xb0
[ 91.951485] ocfs2_disable_quotas.isra.0+0x3e/0xf0 [ocfs2]
[ 91.952635] ocfs2_dismount_volume+0xdd/0x450 [ocfs2]
[ 91.953676] ? __pfx___filemap_fdatawrite_range+0x10/0x10
[ 91.954757] ? __pfx_ocfs2_dismount_volume+0x10/0x10 [ocfs2]
[ 91.955898] ? filemap_check_errors+0x46/0xb0
[ 91.956737] generic_shutdown_super+0xaa/0x280
[ 91.957604] kill_block_super+0x46/0x70
[ 91.958415] deactivate_locked_super+0x4d/0xb0
[ 91.959861] cleanup_mnt+0x135/0x1f0
[ 91.960862] task_work_run+0xe3/0x140
[ 91.961887] ? __pfx_task_work_run+0x10/0x10
[ 91.962887] ? __x64_sys_umount+0xbb/0xd0
[ 91.963343] exit_to_user_mode_prepare+0xda/0xe0
[ 91.963867] syscall_exit_to_user_mode+0x1d/0x50
[ 91.964392] do_syscall_64+0x4f/0x90
[ 91.964800] entry_SYSCALL_64_after_hwframe+0x72/0xdc
[ 91.965387] RIP: 0033:0x7fa7664baacb
[ 91.965796] Code: fa 90 90 31 f6 e9 13 00 00 00 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 90 90 b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 058
[ 91.967851] RSP: 002b:00007ffc8d30da28 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[ 91.968671] RAX: 0000000000000000 RBX: 00005628ef17a9c0 RCX: 00007fa7664baacb
[ 91.969464] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00005628ef17cdd0
[ 91.970253] RBP: 00005628ef17aad8 R08: 0000000000000073 R09: 0000000000000001
[ 91.971031] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 91.971790] R13: 00005628ef17cdd0 R14: 00005628ef17abf0 R15: 00007ffc8d310088
[ 91.972546] </TASK>
[ 91.972961] Allocated by task 632:
[ 91.973342] kasan_save_stack+0x1c/0x40
[ 91.973760] kasan_set_track+0x21/0x30
[ 91.974160] __kasan_kmalloc+0x8b/0x90
[ 91.974584] ocfs2_local_read_info+0xe3/0x9a0 [ocfs2]
[ 91.975168] dquot_load_quota_sb+0x34b/0x680
[ 91.975624] dquot_load_quota_inode+0xfe/0x1a0
[ 91.976092] ocfs2_enable_quotas+0x190/0x2f0 [ocfs2]
[ 91.976679] ocfs2_fill_super+0x14ef/0x2120 [ocfs2]
[ 91.977249] mount_bdev+0x1be/0x200
[ 91.977622] legacy_get_tree+0x6c/0xb0
[ 91.978014] vfs_get_tree+0x3e/0x110
[ 91.978415] path_mount+0xa90/0xe10
[ 91.978774] __x64_sys_mount+0x16f/0x1a0
[ 91.979171] do_syscall_64+0x43/0x90
[ 91.979537] entry_SYSCALL_64_after_hwframe+0x72/0xdc
[ 91.980213] Freed by task 650:
[ 91.980535] kasan_save_stack+0x1c/0x40
[ 91.980925] kasan_set_track+0x21/0x30
[ 91.981317] kasan_save_free_info+0x2a/0x50
[ 91.981744] __kasan_slab_free+0xf9/0x150
[ 91.982151] __kmem_cache_free+0x89/0x180
[ 91.982568] ocfs2_local_free_info+0x2ba/0x3f0 [ocfs2]
[ 91.983140] dquot_disable+0x35f/0xa70
[ 91.983509] ocfs2_susp_quotas.isra.0+0x159/0x1a0 [ocfs2]
[ 91.984096] ocfs2_remount+0x150/0x580 [ocfs2]
[ 91.984584] reconfigure_super+0x1a5/0x3a0
[ 91.984993] path_mount+0xc8a/0xe10
[ 91.985357] __x64_sys_mount+0x16f/0x1a0
[ 91.985750] do_syscall_64+0x43/0x90
[ 91.986125] entry_SYSCALL_64_after_hwframe+0x72/0xdc
[ 91.986799] The buggy address belongs to the object at ffff8880389a8000
which belongs to the cache kmalloc-1k of size 1024
[ 91.987965] The buggy address is located 520 bytes inside of
freed 1024-byte region [ffff8880389a8000, ffff8880389a8400)
[ 91.989287] The buggy address belongs to the physical page:
[ 91.989812] page:00000000bc93f4e4 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x389a8
[ 91.990719] head:00000000bc93f4e4 order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 91.991483] flags: 0x4000000000010200(slab|head|zone=1)
[ 91.991981] page_type: 0xffffffff()
[ 91.992321] raw: 4000000000010200 ffff888003041dc0 dead000000000100 dead000000000122
[ 91.993064] raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
[ 91.993812] page dumped because: kasan: bad access detected
[ 91.994505] Memory state around the buggy address:
[ 91.994973] ffff8880389a8100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 91.995656] ffff8880389a8180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 91.996366] >ffff8880389a8200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 91.997050] ^
[ 91.997399] ffff8880389a8280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 91.998087] ffff8880389a8300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 91.998774] ==================================================================
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 13:22 ` Luís Henriques
@ 2023-05-23 2:43 ` Joseph Qi
0 siblings, 0 replies; 10+ messages in thread
From: Joseph Qi @ 2023-05-23 2:43 UTC (permalink / raw)
To: Luís Henriques, Heming Zhao
Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker
On 5/22/23 9:22 PM, Luís Henriques wrote:
> Heming Zhao <heming.zhao@suse.com> writes:
>
>> On Mon, May 22, 2023 at 01:23:07PM +0100, Luís Henriques wrote:
>>> Joseph Qi <joseph.qi@linux.alibaba.com> writes:
>>>
>>>> On 5/22/23 6:25 PM, Luís Henriques wrote:
>>>>> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
>>>>> fstest generic/452. After mounting a filesystem as read-only, quotas are
>>>>
>>>> generic/452 is for testing ext4 mounted with dax and ro.
>>>> But ocfs2 doesn't support dax yet.
>>>
>>> Right, but I think it's still useful to run the 'generic' test-suite in a
>>> filesystem. We can always find issues in the test itself or, in this
>>> case, a bug in the filesystem.
>>
>> It looks you did some special steps for 452. In my env, without changing
>> anything, I could pass this case successfully.
>
> No, I haven't changed anything to the test. I just make sure there's a
> scratch device to be used.
>
> Maybe you can try to enable KASAN to catch the UAF. I've found the bug
> without KASAN (i.e. I saw a NULL pointer panic), but enabling it also
> detects the issue -- see below.
>
> Cheers,
We'd better append panic dmesg to patch description as well.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
2023-05-22 12:23 ` Luís Henriques
2023-05-22 12:36 ` Heming Zhao
@ 2023-05-23 2:41 ` Joseph Qi
1 sibling, 0 replies; 10+ messages in thread
From: Joseph Qi @ 2023-05-23 2:41 UTC (permalink / raw)
To: Luís Henriques
Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker, Heming Zhao
On 5/22/23 8:23 PM, Luís Henriques wrote:
> Joseph Qi <joseph.qi@linux.alibaba.com> writes:
>
>> On 5/22/23 6:25 PM, Luís Henriques wrote:
>>> It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
>>> fstest generic/452. After mounting a filesystem as read-only, quotas are
>>
>> generic/452 is for testing ext4 mounted with dax and ro.
>> But ocfs2 doesn't support dax yet.
>
> Right, but I think it's still useful to run the 'generic' test-suite in a
> filesystem. We can always find issues in the test itself or, in this
> case, a bug in the filesystem.
>
>>> suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
>>> unmounting the filesystem, an UAF access to the oinfo will eventually cause a
>>> crash.
>>
>> In ocfs2_fill_super(), it won't enable quota if is a readonly mount.
>> Do you mean remount as readonly?
>
> Yes, sorry. Instead of "mounting", the patch changelog should say
>
> "After remounting a filesystem as read-only..."
>
> Cheers,
Okay, look into the code flow, it does have problem when remount with
read-only.
BTW, it seems that we can't call into quota_disable() again, which will
call free_file_info().
Thanks,
Joseph
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
@ 2023-05-22 10:21 Luís Henriques
0 siblings, 0 replies; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 10:21 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, Luís Henriques, stable
It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
fstest generic/452. After mounting a filesystem as read-only, quotas are
suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
unmounting the filesystem, an UAF access to the oinfo will eventually cause a
crash.
Cc: <stable@vger.kernel.org>
Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
fs/ocfs2/super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..988d1c076861 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
if (!sb_has_quota_loaded(sb, type))
continue;
- oinfo = sb_dqinfo(sb, type)->dqi_priv;
- cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ if (!sb_has_quota_suspended(sb, type)) {
+ oinfo = sb_dqinfo(sb, type)->dqi_priv;
+ cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ }
inode = igrab(sb->s_dquot.files[type]);
/* Turn off quotas. This will remove all dquot structures from
* memory and so they will be automatically synced to global
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem
@ 2023-05-22 10:20 Luís Henriques
0 siblings, 0 replies; 10+ messages in thread
From: Luís Henriques @ 2023-05-22 10:20 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, Luís Henriques, stable
It's trivial to trigger a use-after-free bug in the ocfs2 quotas code using
fstest generic/452. After mounting a filesystem as read-only, quotas are
suspended and ocfs2_mem_dqinfo is freed through ->ocfs2_local_free_info(). When
unmounting the filesystem, an UAF access to the oinfo will eventually cause a
crash.
Cc: <stable@vger.kernel.org>
Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
fs/ocfs2/super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..988d1c076861 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -952,8 +952,10 @@ static void ocfs2_disable_quotas(struct ocfs2_super *osb)
for (type = 0; type < OCFS2_MAXQUOTAS; type++) {
if (!sb_has_quota_loaded(sb, type))
continue;
- oinfo = sb_dqinfo(sb, type)->dqi_priv;
- cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ if (!sb_has_quota_suspended(sb, type)) {
+ oinfo = sb_dqinfo(sb, type)->dqi_priv;
+ cancel_delayed_work_sync(&oinfo->dqi_sync_work);
+ }
inode = igrab(sb->s_dquot.files[type]);
/* Turn off quotas. This will remove all dquot structures from
* memory and so they will be automatically synced to global
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-05-23 2:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-22 10:24 [PATCH] ocfs2: fix use-after-free when unmounting read-only filesystem Luís Henriques
-- strict thread matches above, loose matches on Subject: below --
2023-05-22 10:25 Luís Henriques
2023-05-22 12:01 ` Joseph Qi
2023-05-22 12:23 ` Luís Henriques
2023-05-22 12:36 ` Heming Zhao
2023-05-22 13:22 ` Luís Henriques
2023-05-23 2:43 ` Joseph Qi
2023-05-23 2:41 ` Joseph Qi
2023-05-22 10:21 Luís Henriques
2023-05-22 10:20 Luís Henriques
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox