lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <hd2uf7odgxxuadeym76nlsvsfxr5mvfveenaqv7rqwy2jyaan6@ts6gf2wpujsk>
Date: Thu, 24 Apr 2025 13:09:08 +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: Re: [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.

Regrads,
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ