linux-kernel - Re: [PATCH v7 06/21] mm/vma: Change munmap to use vma_munmap_struct() for accounting and surrounding vmas

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: <960730aa-d015-417a-846f-4da20965145d@lucifer.local>
Date: Fri, 23 Aug 2024 12:37:18 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Bert Karwatzki <spasswolf@....de>
Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Suren Baghdasaryan <surenb@...gle.com>,
        Matthew Wilcox <willy@...radead.org>, Vlastimil Babka <vbabka@...e.cz>,
        sidhartha.kumar@...cle.com, Jiri Olsa <olsajiri@...il.com>,
        Kees Cook <kees@...nel.org>, "Paul E . McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH v7 06/21] mm/vma: Change munmap to use
 vma_munmap_struct() for accounting and surrounding vmas

On Fri, Aug 23, 2024 at 10:43:11AM GMT, Bert Karwatzki wrote:

[snip]

> > @@ -731,21 +708,31 @@ static void vms_complete_munmap_vmas(struct vma_munmap_struct *vms,
> >  	if (vms->unlock)
> >  		mmap_write_downgrade(mm);
> >
> > -	prev = vma_iter_prev_range(vms->vmi);
> > -	next = vma_next(vms->vmi);
> > -	if (next)
> > -		vma_iter_prev_range(vms->vmi);
> > -
> >  	/*
> >  	 * We can free page tables without write-locking mmap_lock because VMAs
> >  	 * were isolated before we downgraded mmap_lock.
> >  	 */
> >  	mas_set(mas_detach, 1);
> > -	unmap_region(mm, mas_detach, vms->vma, prev, next, vms->start, vms->end,
> > -		     vms->vma_count, !vms->unlock);
> > -	/* Statistics and freeing VMAs */
> > +	unmap_region(mm, mas_detach, vms->vma, vms->prev, vms->next,
> > +		     vms->start, vms->end, vms->vma_count, !vms->unlock);
> > +	/* Update high watermark before we lower total_vm */
> > +	update_hiwater_vm(mm);
> > +	/* Stat accounting */
> > +	WRITE_ONCE(mm->total_vm, READ_ONCE(mm->total_vm) - vms->nr_pages);
> > +	mm->exec_vm -= vms->exec_vm;
> > +	mm->stack_vm -= vms->stack_vm;
> > +	mm->data_vm -= vms->data_vm;
> > +	/* Paranoid bookkeeping */
> > +	VM_WARN_ON(vms->exec_vm > mm->exec_vm);
> > +	VM_WARN_ON(vms->stack_vm > mm->stack_vm);
> > +	VM_WARN_ON(vms->data_vm > mm->data_vm);

Hang on... I didn't read this closely enough (clearly!) we're doing these checks
_after_ we decrement the counters, which is... not correct :)

Your processes must be reducing their data_vm value to something less than what
was reduced during the munmap operation.

Liam - I suggest we put these checks before we decrement.

> > +
>
> I'm running the v7 Patchset on linux-next-20240822 and I get lots of these
> errors (right on boot) (both when using the complete patchset and when using
> only the patches up to this):
>
> [  T620] WARNING: CPU: 6 PID: 620 at mm/vma.c:725
> vms_complete_munmap_vmas+0x1d8/0x200
> [  T620] Modules linked in: amd_atl ecc mc sparse_keymap wmi_bmof edac_mce_amd
> snd snd_pci_acp3x k10temp soundcore ccp battery ac button hid_sensor_gyro_3d
> hid_sensor_als hid_sensor_magn_3d hid_sensor_prox hid_sensor_accel_3d
> hid_sensor_trigger industrialio_triggered_buffer kfifo_buf industrialio amd_pmc
> hid_sensor_iio_common joydev evdev serio_raw mt7921e mt7921_common mt792x_lib
> mt76_connac_lib mt76 mac80211 libarc4 cfg80211 rfkill msr nvme_fabrics fuse
> efi_pstore configfs efivarfs autofs4 ext4 crc32c_generic mbcache jbd2 usbhid
> amdgpu i2c_algo_bit drm_ttm_helper ttm drm_exec drm_suballoc_helper amdxcp
> xhci_pci drm_buddy hid_sensor_hub xhci_hcd nvme mfd_core gpu_sched
> hid_multitouch hid_generic crc32c_intel psmouse usbcore i2c_piix4
> drm_display_helper amd_sfh i2c_hid_acpi i2c_smbus usb_common crc16 nvme_core
> r8169 i2c_hid hid i2c_designware_platform i2c_designware_core
> [  T620] CPU: 6 UID: 0 PID: 620 Comm: fsck.vfat Not tainted 6.11.0-rc4-next-
> 20240822-liamh-v7-00021-gc6686c81601f #322
> [  T620] Hardware name: Micro-Star International Co., Ltd. Alpha 15 B5EEK/MS-
> 158L, BIOS E158LAMS.107 11/10/2021
> [  T620] RIP: 0010:vms_complete_munmap_vmas+0x1d8/0x200
> [  T620] Code: 8b 85 a8 00 00 00 a8 01 74 35 8b 85 e0 00 00 00 48 8d bd a8 00 00
> 00 83 c0 01 89 85 e0 00 00 00 e8 7d 39 e8 ff e9 63 fe ff ff <0f> 0b e9 eb fe ff
> ff 0f 0b e9 d0 fe ff ff 0f 0b e9 d3 fe ff ff 0f
> [  T620] RSP: 0018:ffffa415c09d7d10 EFLAGS: 00010283
> [  T620] RAX: 00000000000000cd RBX: ffffa415c09d7d90 RCX: 000000000000018e
> [  T620] RDX: 0000000000000021 RSI: 00000000000019d9 RDI: ffff9073ee7a6400
> [  T620] RBP: ffff906541341f80 R08: 0000000000000000 R09: 000000000000080a
> [  T620] R10: 000000000001d4de R11: 0000000000000140 R12: ffffa415c09d7d48
> [  T620] R13: 00007fbd5ea5f000 R14: 00007fbd5eb5efff R15: ffffa415c09d7d90
> [  T620] FS:  00007fbd5ec38740(0000) GS:ffff9073ee780000(0000)
> knlGS:0000000000000000
> [  T620] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  T620] CR2: 00007fc336339c90 CR3: 000000010a39e000 CR4: 0000000000750ef0
> [  T620] PKRU: 55555554
> [  T620] Call Trace:
> [  T620]  <TASK>
> [  T620]  ? __warn.cold+0x90/0x9e
> [  T620]  ? vms_complete_munmap_vmas+0x1d8/0x200
> [  T620]  ? report_bug+0xfa/0x140
> [  T620]  ? handle_bug+0x53/0x90
> [  T620]  ? exc_invalid_op+0x17/0x70
> [  T620]  ? asm_exc_invalid_op+0x1a/0x20
> [  T620]  ? vms_complete_munmap_vmas+0x1d8/0x200
> [  T620]  do_vmi_align_munmap+0x1e0/0x260
> [  T620]  do_vmi_munmap+0xbe/0x160
> [  T620]  __vm_munmap+0x96/0x110
> [  T620]  __x64_sys_munmap+0x16/0x20
> [  T620]  do_syscall_64+0x5f/0x170
> [  T620]  entry_SYSCALL_64_after_hwframe+0x55/0x5d
> [  T620] RIP: 0033:0x7fbd5ed3ec57
> [  T620] Code: 73 01 c3 48 8b 0d c1 71 0d 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 0b 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> 73 01 c3 48 8b 0d 91 71 0d 00 f7 d8 64 89 01 48
> [  T620] RSP: 002b:00007fff0b04d298 EFLAGS: 00000202 ORIG_RAX: 000000000000000b
> [  T620] RAX: ffffffffffffffda RBX: ffffffffffffff88 RCX: 00007fbd5ed3ec57
> [  T620] RDX: 0000000000000000 RSI: 0000000000100000 RDI: 00007fbd5ea5f000
> [  T620] RBP: 0000000000000002 R08: 0000000000100000 R09: 0000000000000007
> [  T620] R10: 0000000000000007 R11: 0000000000000202 R12: 00007fff0b04d588
> [  T620] R13: 000055b76c789fc6 R14: 00007fff0b04d360 R15: 00007fff0b04d3c0
> [  T620]  </TASK>
> [  T620] ---[ end trace 0000000000000000 ]---
>
>
> Bert Karwatzki