[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6268df0f-e7a5-4b4f-84d0-082f5767f6d7@gmail.com>
Date: Thu, 24 Apr 2025 13:15:24 +0200
From: Klara Modin <klarasmodin@...il.com>
To: Daniel Vacek <neelx@...e.com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH] btrfs: unlock all extent buffer folios in failure case
Hi,
On Tue, Apr 22, 2025 at 02:57:01PM +0200, Daniel Vacek wrote:
> When attaching a folio fails, for example if another one is already mapped,
> we need to unlock all newly allocated folios before putting them. And as a
> consequence we do not need to flag the eb UNMAPPED anymore.
>
> Signed-off-by: Daniel Vacek <neelx@...e.com>
I hit a null pointer dereference in next-20250424 which seems to point
to this commit. Reverting it resolves the issue for me (did not bisect).
Please let me know if there's anything else you need.
Regards,
Klara Modin
BUG: kernel NULL pointer dereference, address: 0000000000000000
nct6683 nct6683.2592: NCT6687D EC firmware version 1.0 build 05/07/20
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP NOPTI
CPU: 19 UID: 0 PID: 1138 Comm: (udev-worker) Not tainted
6.15.0-rc3-next-20250424 #474 PREEMPTLAZY
Hardware name: Micro-Star International Co., Ltd. MS-7C91/MAG B550
TOMAHAWK (MS-7C91), BIOS A.G0 03/12/2024
RIP: 0010:alloc_extent_buffer (arch/x86/include/asm/bitops.h:206
arch/x86/include/asm/bitops.h:238
include/asm-generic/bitops/instrumented-non-atomic.h:142
include/linux/page-flags.h:860 include/linux/page-flags.h:881
include/linux/mm.h:994 fs/btrfs/extent_io.h:290
fs/btrfs/extent_io.c:3360) Code: ad 8f cb ff f0 ff 4b 34 0f 84 9a 01 00
00 4b c7 84 f4 a8 00 00 00 00 00 00 00 41 8b 74 24 08 49 8b 8c 24 a8 00
00 00 41 ff c5 <48> 8b 01 a8 40 74 0b 80 79 40 00 b8 01 00 00 00 75 0d
89 f0 ba 01
All code
========
0: ad lods %ds:(%rsi),%eax
1: 8f (bad)
2: cb lret
3: ff f0 push %rax
5: ff 4b 34 decl 0x34(%rbx)
8: 0f 84 9a 01 00 00 je 0x1a8
e: 4b c7 84 f4 a8 00 00 movq $0x0,0xa8(%r12,%r14,8)
15: 00 00 00 00 00 1a: 41 8b 74 24 08 mov 0x8(%r12),%esi
1f: 49 8b 8c 24 a8 00 00 mov 0xa8(%r12),%rcx
26: 00 27: 41 ff c5 inc %r13d
2a:* 48 8b 01 mov (%rcx),%rax <-- trapping instruction
2d: a8 40 test $0x40,%al
2f: 74 0b je 0x3c
31: 80 79 40 00 cmpb $0x0,0x40(%rcx)
35: b8 01 00 00 00 mov $0x1,%eax
3a: 75 0d jne 0x49
3c: 89 f0 mov %esi,%eax
3e: ba .byte 0xba
3f: 01 .byte 0x1
Code starting with the faulting instruction
===========================================
0: 48 8b 01 mov (%rcx),%rax
3: a8 40 test $0x40,%al
5: 74 0b je 0x12
7: 80 79 40 00 cmpb $0x0,0x40(%rcx)
b: b8 01 00 00 00 mov $0x1,%eax
10: 75 0d jne 0x1f
12: 89 f0 mov %esi,%eax
14: ba .byte 0xba
15: 01 .byte 0x1
RSP: 0018:ffffac790119b740 EFLAGS: 00010202
RAX: 0000000000000013 RBX: fffff9cd44725bc0 RCX: 0000000000000000
RDX: 00000000000003a4 RSI: 0000000000004000 RDI: ffff95ca3efd3040
RBP: 0000000000000000 R08: 0000000000000000 R09: 00000000000094b5
R10: 0000000000000000 R11: 00000000000005df R12: ffff95c351019848
R13: 0000000000000001 R14: 0000000000000000 R15: ffff95c34b438250
FS: 00007fd880dad840(0000) GS:ffff95caa9f0e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000108412000 CR4: 0000000000350ef0
Call Trace:
<TASK>
read_block_for_search (fs/btrfs/ctree.c:1533) btrfs_search_slot
(fs/btrfs/ctree.c:2173 (discriminator 1)) ? srso_return_thunk
(arch/x86/lib/retpoline.S:224) ? mempool_alloc_noprof (mm/mempool.c:402)
btrfs_lookup_csum (fs/btrfs/file-item.c:249) ? btrfs_csum_root
(fs/btrfs/disk-io.c:828) btrfs_lookup_bio_sums (fs/btrfs/file-item.c:312
(discriminator 1) fs/btrfs/file-item.c:406 (discriminator 1))
btrfs_submit_chunk (fs/btrfs/bio.c:717) ?
btrfs_clear_extent_bit_changeset (fs/btrfs/extent-io-tree.c:751) ?
srso_untrain_ret (arch/x86/lib/retpoline.S:209) ?
btrfs_clear_extent_bit_changeset (fs/btrfs/extent-io-tree.c:751)
btrfs_submit_bbio (fs/btrfs/bio.c:791 (discriminator 2)) submit_one_bio
(fs/btrfs/extent_io.c:132) btrfs_readahead (fs/btrfs/extent_io.c:2535) ?
srso_return_thunk (arch/x86/lib/retpoline.S:224) ?
__pfx_end_bbio_data_read (fs/btrfs/extent_io.c:502) read_pages
(include/linux/pagemap.h:1400 include/linux/pagemap.h:1426
mm/readahead.c:162) page_cache_ra_unbounded (include/linux/fs.h:933
mm/readahead.c:298) filemap_get_pages (mm/filemap.c:2592) ?
srso_return_thunk (arch/x86/lib/retpoline.S:224) ? dput (fs/dcache.c:858
fs/dcache.c:896) ? srso_return_thunk (arch/x86/lib/retpoline.S:224)
filemap_read (mm/filemap.c:2702) ? srso_return_thunk
(arch/x86/lib/retpoline.S:224) ? do_filp_open (fs/namei.c:4074
(discriminator 2)) ? srso_return_thunk (arch/x86/lib/retpoline.S:224) ?
__pfx_page_put_link (fs/namei.c:5454) ? kmem_cache_alloc_noprof
(arch/x86/include/asm/jump_label.h:46 include/linux/memcontrol.h:1696
mm/slub.c:2190 mm/slub.c:4174 mm/slub.c:4213 mm/slub.c:4220) vfs_read
(fs/read_write.c:489 fs/read_write.c:570) ksys_read
(fs/read_write.c:714) do_syscall_64 (arch/x86/entry/syscall_64.c:63
(discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) RIP:
0033:0x7fd880694207
Code: 00 49 89 d0 48 89 fa 4c 89 df e8 74 b8 00 00 8b 93 08 03 00 00 59
5e 48 83 f8 fc 74 16 5b c3 0f 1f 40 00 48 8b 44 24 10 0f 05 <5b> c3 0f
1f 80 00 00 00 00 83 e2 39 83 fa 08 75 e2 e8 23 ff ff ff
All code
========
0: 00 49 89 add %cl,-0x77(%rcx)
3: d0 48 89 rorb $1,-0x77(%rax)
6: fa cli
7: 4c 89 df mov %r11,%rdi
a: e8 74 b8 00 00 call 0xb883
f: 8b 93 08 03 00 00 mov 0x308(%rbx),%edx
15: 59 pop %rcx
16: 5e pop %rsi
17: 48 83 f8 fc cmp $0xfffffffffffffffc,%rax
1b: 74 16 je 0x33
1d: 5b pop %rbx
1e: c3 ret
1f: 0f 1f 40 00 nopl 0x0(%rax)
23: 48 8b 44 24 10 mov 0x10(%rsp),%rax
28: 0f 05 syscall
2a:* 5b pop %rbx <-- trapping instruction
2b: c3 ret
2c: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
33: 83 e2 39 and $0x39,%edx
36: 83 fa 08 cmp $0x8,%edx
39: 75 e2 jne 0x1d
3b: e8 23 ff ff ff call 0xffffffffffffff63
Code starting with the faulting instruction
===========================================
0: 5b pop %rbx
1: c3 ret
2: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
9: 83 e2 39 and $0x39,%edx
c: 83 fa 08 cmp $0x8,%edx
f: 75 e2 jne 0xfffffffffffffff3
11: e8 23 ff ff ff call 0xffffffffffffff39
RSP: 002b:00007ffc7372ce60 EFLAGS: 00000202 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 00007fd880dad840 RCX: 00007fd880694207
RDX: 0000000000000006 RSI: 00007ffc7372cf01 RDI: 0000000000000049
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffc7372cf01
R13: 00007ffc7372cf01 R14: 0000000000000049 R15: 00007ffc7372cf01
</TASK>
Modules linked in: drm_exec btbcm iwlwifi(+) nct6683 gpu_sched snd_hwdep
msi_ec(-) drm_suballoc_helper evdev kvm ee1004 bluetooth battery joydev
mac_hid snd_hda_core video irqbypass cfg80211 drm_panel_backlight_quirks
ghash_clmulni_intel snd_pcm cec sha512_ssse3 sp5100_tco sha256_ssse3
drm_buddy snd_timer sha1_ssse3 watchdog drm_display_helper ccp snd
tpm_crb aesni_intel rfkill soundcore pcspkr tpm_tis tpm_tis_core
i2c_piix4 k10temp i2c_smbus tpm libaescfb ecdh_generic gpio_amdpt ecc
gpio_generic button wmi
CR2: 0000000000000000
---[ end trace 0000000000000000 ]---
> ---
> fs/btrfs/extent_io.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 197f5e51c4744..7023dd527d3e7 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3385,30 +3385,26 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
> * we'll lookup the folio for that index, and grab that EB. We do not
> * want that to grab this eb, as we're getting ready to free it. So we
> * have to detach it first and then unlock it.
> - *
> - * We have to drop our reference and NULL it out here because in the
> - * subpage case detaching does a btrfs_folio_dec_eb_refs() for our eb.
> - * Below when we call btrfs_release_extent_buffer() we will call
> - * detach_extent_buffer_folio() on our remaining pages in the !subpage
> - * case. If we left eb->folios[i] populated in the subpage case we'd
> - * double put our reference and be super sad.
> */
> - for (int i = 0; i < attached; i++) {
> - ASSERT(eb->folios[i]);
> - detach_extent_buffer_folio(eb, eb->folios[i]);
> - folio_unlock(eb->folios[i]);
> - folio_put(eb->folios[i]);
> + for (int i = 0; i < num_extent_folios(eb); i++) {
> + struct folio *folio = eb->folios[i];
> +
> + if (i < attached) {
> + ASSERT(folio);
> + detach_extent_buffer_folio(eb, folio);
> + } else if (!folio)
> + continue;
> +
> + ASSERT(!folio_test_private(folio));
> + folio_unlock(folio);
> + folio_put(folio);
> eb->folios[i] = NULL;
> }
> - /*
> - * Now all pages of that extent buffer is unmapped, set UNMAPPED flag,
> - * so it can be cleaned up without utilizing folio->mapping.
> - */
> - set_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags);
> -
> btrfs_release_extent_buffer(eb);
> +
> if (ret < 0)
> return ERR_PTR(ret);
> +
> ASSERT(existing_eb);
> return existing_eb;
> }
> --
> 2.47.2
>
Download attachment "btrfs_oops3_decoded.gz" of type "application/gzip" (19018 bytes)
Download attachment "config.gz" of type "application/gzip" (45147 bytes)
Powered by blists - more mailing lists