fs/ext4/mballoc.c | 2 ++ 1 file changed, 2 insertions(+)
ext4_mb_check_group_pa drops the reference count on the previous
best PA using atomic_dec(&cpa->pa_count) without holding the
cpa->pa_lock.
This causes race with ext4_discard_preallocations() which checks
pa_count to decide whether a PA is still in use. If the pa_count
is dec between the check and the discard, the PA can be freed
while ext4_mb_check_group_pa() still holds a reference to
it.
Fix this by taking the cpa->pa_lock around the atomic_dec.
Similar to pa->pa_lock which is taken outside of the
ext4_mb_check_group_pa() function.
The race was found while testing a change related to a
Coccinelle warning from atomic_as_refcounter.cocci. The refcount
conversion was found to be incorrect but the change had revealed
the pre-exiting race condition.
Signed-off-by: rafad900 <rafad900@gmail.com>
---
Changes in v2:
- Identified correct race location: ext4_mb_check_group_pa rather
than ext4_mb_use_preallocated (the inode PA path already holds
pa_lock correctly)
- Dropped refcount_t conversion — incompatible with PA lifecycle
where count=0 represents an idle but reusable PA
- Added spin_lock(&cpa->pa_lock) around atomic_dec in
ext4_mb_check_group_pa
fs/ext4/mballoc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index ed1bd00e11cd..c9a118ae4658 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4833,7 +4833,9 @@ ext4_mb_check_group_pa(ext4_fsblk_t goal_block,
return cpa;
/* drop the previous reference */
+ spin_lock(&cpa->pa_lock);
atomic_dec(&cpa->pa_count);
+ spin_unlock(&cpa->pa_lock);
atomic_inc(&pa->pa_count);
return pa;
}
--
2.43.0
On Jul 20, 2026, at 00:54, rafad900 <rafad900@gmail.com> wrote:
> ext4_mb_check_group_pa drops the reference count on the previous
> best PA using atomic_dec(&cpa->pa_count) without holding the
> cpa->pa_lock.
>
> This causes race with ext4_discard_preallocations() which checks
> pa_count to decide whether a PA is still in use. If the pa_count
> is dec between the check and the discard, the PA can be freed
> while ext4_mb_check_group_pa() still holds a reference to it.
Can you please explain this race condition further? I don't see where
ext4_mb_check_group_pa() is using cpa after the reference is dropped.
> Fix this by taking the cpa->pa_lock around the atomic_dec.
> Similar to pa->pa_lock which is taken outside of the
> ext4_mb_check_group_pa() function.
At this point, it wouldn't be clear why `pa_count` needs to be an
atomic at all, if `pa_lock` is always held during inc/dec/check?
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index ed1bd00e11cd..c9a118ae4658 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4833,7 +4833,9 @@ ext4_mb_check_group_pa(ext4_fsblk_t goal_block,
> return cpa;
>
> /* drop the previous reference */
> + spin_lock(&cpa->pa_lock);
> atomic_dec(&cpa->pa_count);
> + spin_unlock(&cpa->pa_lock);
> atomic_inc(&pa->pa_count);
> return pa;
> }
In ext4_mb_check_group_pa() there is no reference to `cpa` after the
refcount is dropped. In its one caller ext4_mb_use_preallocated():
list_for_each_entry_rcu(tmp_pa, &lg->lg_prealloc_list[i],
pa_node.lg_list) {
spin_lock(&tmp_pa->pa_lock);
if (tmp_pa->pa_deleted == 0 &&
tmp_pa->pa_free >= ac->ac_o_ex.fe_len) {
cpa = ext4_mb_check_group_pa(goal_block,
tmp_pa, cpa);
}
spin_unlock(&tmp_pa->pa_lock);
}
rcu_read_unlock();
}
if (cpa) {
ext4_mb_use_group_pa(ac, cpa);
return true;
}
return false;
}
It *looks* like 'cpa' is used after ext4_mb_check_group_pa(), but it is
replaced on the return by 'tmp_pa' in that case, so there is no further
use after the refcount is dropped AFAICS. Even the list iteration is
using 'tmp_pa', so that couldn't be it either.
There may be a race condition somewhere, but the commit message doesn't
provide clear details of what it is.
Cheers, Andreas
I think I misinterpreted what was going on after the change in the first patch. When making the change from atomic_t to refcount_t, I get this kernel error while testing on my x86 QEMU: ------------[ cut here ]------------ [ 12.954674] refcount_t: addition on 0; use-after-free. [ 12.956476] WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x6a/0x90, CPU#0: dhcpcd-run-hook/229 [ 12.959648] Modules linked in: [ 12.960735] CPU: 0 UID: 0 PID: 229 Comm: dhcpcd-run-hook Not tainted 7.2.0-rc4-00001-g453fc8a20cd2-dirty #6 PREEMPT(lazy) [ 12.964446] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 12.968371] RIP: 0010:refcount_warn_saturate+0x6a/0x90 [ 12.969892] Code: 00 48 8d 3d b8 92 71 01 67 48 0f b9 3a e9 2e cd ad 00 48 8d 3d b7 92 71 01 67 48 0f b9 3a e9 1d cd ad 00 48 8d 3d b6 92 71 01 <67> 48 0f b9 3a e9 0c cd ad 00 48 8d 3d b5 92 71 01 67 48 0f b9 3a [ 12.975114] RSP: 0018:ffffb4c9c03a7838 EFLAGS: 00010246 [ 12.976742] RAX: 0000000000000001 RBX: ffff8c96fdc32688 RCX: 0000000000000000 [ 12.979085] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffffffff8b703940 [ 12.981349] RBP: ffff8c96848a1738 R08: 000000000000000c R09: 0000000000000200 [ 12.983382] R10: ffff8c96848a1738 R11: 000000000000001f R12: ffff8c96848a3a10 [ 12.985415] R13: 0000000000000000 R14: 0000000000000008 R15: ffff8c96848a3a48 [ 12.987455] FS: 00007f2995026780(0000) GS:ffff8c9771c6f000(0000) knlGS:0000000000000000 [ 12.989751] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 12.991296] CR2: 00005593d8eb6640 CR3: 0000000001919000 CR4: 0000000000350ef0 [ 12.992879] Call Trace: [ 12.993483] <TASK> [ 12.993982] ext4_mb_use_preallocated.constprop.0+0x346/0x3d0 [ 12.995293] ext4_mb_new_blocks+0x15b/0xe80 [ 12.996246] ? srso_return_thunk+0x5/0x5f [ 12.997178] ? ext4_find_extent+0xc8/0x320 [ 12.998117] ext4_ext_map_blocks+0x3f1/0x17e0 [ 12.999121] ? srso_return_thunk+0x5/0x5f [ 13.000067] ? mpage_prepare_extent_to_map+0x47d/0x4c0 [ 13.001224] ext4_map_create_blocks+0x40/0x170 [ 13.002239] ext4_map_blocks+0x193/0x3c0 [ 13.003142] ext4_do_writepages+0x7e7/0xf20 [ 13.004109] ? ext4_writepages+0xbb/0x180 [ 13.004694] ext4_writepages+0xbb/0x180 [ 13.005275] do_writepages+0xb6/0x150 [ 13.005826] filemap_writeback+0xa1/0xc0 [ 13.006435] ext4_release_file+0x6c/0xa0 [ 13.007041] __fput+0xe1/0x2b0 [ 13.007503] task_work_run+0x57/0x80 [ 13.008046] exit_to_user_mode_loop+0x139/0x4e0 [ 13.008705] ? srso_return_thunk+0x5/0x5f [ 13.009319] do_syscall_64+0x42c/0x540 [ 13.009878] ? srso_return_thunk+0x5/0x5f [ 13.010477] entry_SYSCALL_64_after_hwframe+0x77/0x7f [ 13.011235] RIP: 0033:0x7f2995128887 [ 13.011765] Code: 73 01 c3 48 8b 0d 89 55 0e 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 b8 21 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 59 55 0e 00 f7 d8 64 89 02 b8 [ 13.014433] RSP: 002b:00007ffe0ef0ac48 EFLAGS: 00000246 ORIG_RAX: 0000000000000021 [ 13.015542] RAX: 0000000000000001 RBX: 0000000000000001 RCX: 00007f2995128887 [ 13.016591] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 000000000000000c [ 13.017635] RBP: 00005593d8ebbef0 R08: 0000000000000000 R09: 0000000000000000 [ 13.018685] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 13.019737] R13: 0000000000000001 R14: 0000000000000000 R15: 00005593d8ebced0 [ 13.020798] </TASK> [ 13.021158] ---[ end trace 0000000000000000 ]--- After reading the code a little closer, I realized that pa_count is intentionally decremented to 0. This of course would trigger the warning above when using refcount_t since it treats 0 as "object is completely dead". I assumed it was because a lock was not held and so I added the locks around the atomic_inc(&cpa->pa_count); The reference count is being dropped to 0 on purpose right? -Rafael
© 2016 - 2026 Red Hat, Inc.