linux-kernel - Re: [syzbot] [mm?] KASAN: slab-use-after-free Read in do_sync_mmap

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] [day] [month] [year] [list]

Message-ID: <20250624003032.1564-1-hdanton@sina.com>
Date: Tue, 24 Jun 2025 08:30:30 +0800
From: Hillf Danton <hdanton@...a.com>
To: Ryan Roberts <ryan.roberts@....com>
Cc: david@...hat.com,
	jhubbard@...dia.com,
	linux-kernel@...r.kernel.org,
	Jan Kara <jack@...e.cz>,
	linux-mm@...ck.org,
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [mm?] KASAN: slab-use-after-free Read in do_sync_mmap_readahead

On Mon, 23 Jun 2025 13:51:37 +0100 Ryan Roberts wrote:
> On 21/06/2025 02:20, Hillf Danton wrote:
> > On Thu, 19 Jun 2025 11:52:43 +0200 Jan Kara wrote
> >> On Wed 18-06-25 05:56:30, syzbot wrote:
> >>> Hello,
> >>>
> >>> syzbot found the following issue on:
> >>>
> >>> HEAD commit:    bc6e0ba6c9ba Add linux-next specific files for 20250613
> >>> git tree:       linux-next
> >>> console+strace: https://syzkaller.appspot.com/x/log.txt?x=108c710c580000
> >>> kernel config:  https://syzkaller.appspot.com/x/.config?x=2f7a2e4d17ed458f
> >>> dashboard link: https://syzkaller.appspot.com/bug?extid=8e4be574cb8c40140a2a
> >>> compiler:       Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
> >>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=148c710c580000
> >>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=179025d4580000
> >>>
> >>> Downloadable assets:
> >>> disk image: https://storage.googleapis.com/syzbot-assets/2430bb0465cc/disk-bc6e0ba6.raw.xz
> >>> vmlinux: https://storage.googleapis.com/syzbot-assets/436a39deef0a/vmlinux-bc6e0ba6.xz
> >>> kernel image: https://storage.googleapis.com/syzbot-assets/e314ca5b1eb3/bzImage-bc6e0ba6.xz
> >>>
> >>> The issue was bisected to:
> >>>
> >>> commit 3b61a3f08949297815b2c77ae2696f54cd339419
> >>> Author: Ryan Roberts <ryan.roberts@....com>
> >>> Date:   Mon Jun 9 09:27:27 2025 +0000
> >>>
> >>>     mm/filemap: allow arch to request folio size for exec memory
> >>
> >> Indeed. The crash is in:
> >>
> >> 	fpin = maybe_unlock_mmap_for_io(vmf, fpin);
> >> 	if (vm_flags & VM_EXEC) {
> >> 		/*
> >> 		 * Allow arch to request a preferred minimum folio order for
> >> 		 * executable memory. This can often be beneficial to
> >> 		 * performance if (e.g.) arm64 can contpte-map the folio.
> >> 		 * Executable memory rarely benefits from readahead, due to its
> >> 		 * random access nature, so set async_size to 0.
> >> 		 *
> >> 		 * Limit to the boundaries of the VMA to avoid reading in any
> >> 		 * pad that might exist between sections, which would be a waste
> >> 		 * of memory.
> >> 		 */
> >> 		struct vm_area_struct *vma = vmf->vma;
> >> 		unsigned long start = vma->vm_pgoff;
> >> 				^^^^ here
> >> which is not surprising because we've unlocked mmap_sem (or vma lock) just
> >> above this if and thus vma could have been released before we got here. The
> >> easiest fix is to move maybe_unlock_mmap_for_io() below this if. There's
> >> nothing in there that would be problematic with the locks still held.
> >>
> > In the fault path (arch/arm64/mm/fault.c), vma is locked for read.
> > 
> > 	do_page_fault()
> > 	vma = lock_vma_under_rcu(mm, addr)
> > 	handle_mm_fault()
> > 
> > While in the mmap path [1], mm is locked for write but vma is removed without
> > locking vma for write.
> > 
> > 	vm_mmap_pgoff()
> > 	mmap_write_lock_killable(mm)
> > 	do_mmap()
> >  	mmap_regionC()
> > 	__mmap_region()
> > 	__mmap_complete()
> > 	vms_complete_munmap_vmas()
> > 	remove_vma()
> > 
> > Thus the correct fix looks like locking vma in both mmap and gup pathes [2].
> 
> Hi Hillf,
> 
> do_sync_mmap_readahead() was already accessing the vma prior to my change, but
> it was doing so before calling maybe_unlock_mmap_for_io(). I think that you are
> saying that there exists a separate race whereby it's possible for a vma to be
> removed even when the vma is locked?
> 
The comment above faultin_page() [3] shows that mmap_lock plays its role.
/*
 * mmap_lock must be held on entry.  If @flags has FOLL_UNLOCKABLE but not
 * FOLL_NOWAIT, the mmap_lock may be released.  If it is, *@...ked will be set
 * to 0 and -EBUSY returned.
 */

> In which case, I think we need both fixes? FWIW, Andrew has already updated

We can revisit this once syzbot reports again.

> mm-unstable to include the fix to ensure we don't access the vma after calling
> maybe_unlock_mmap_for_io().
> 
> Thanks,
> Ryan
> 
> > 
> > [1] https://lore.kernel.org/lkml/685535d2.a00a0220.137b3.0045.GAE@google.com/
> > [2] https://lore.kernel.org/lkml/68555d6e.a00a0220.137b3.004c.GAE@google.com/
[3] https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/mm/gup.c#n1143