linux-kernel - Re: [PATCH V2] mm: swap: check for stable address space before operating on the VMA

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: <CAGsJ_4zhL=4qJDmGA5E-z7=VixkUXOi+qBnN3S-WWH2UbYj=ZA@mail.gmail.com>
Date: Fri, 26 Sep 2025 16:30:38 +0800
From: Barry Song <21cnbao@...il.com>
To: Chris Li <chrisl@...nel.org>
Cc: Charan Teja Kalla <charan.kalla@....qualcomm.com>, david@...hat.com, 
	Liam.Howlett@...cle.com, lorenzo.stoakes@...cle.com, 
	akpm@...ux-foundation.org, shikemeng@...weicloud.com, kasong@...cent.com, 
	nphamcs@...il.com, bhe@...hat.com, zhangpeng.00@...edance.com, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] mm: swap: check for stable address space before
 operating on the VMA

On Fri, Sep 26, 2025 at 9:03 AM Chris Li <chrisl@...nel.org> wrote:
[...]
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -2389,6 +2389,8 @@ static int unuse_mm(struct mm_struct *mm, unsigned int type)
> >         VMA_ITERATOR(vmi, mm, 0);
> >
> >         mmap_read_lock(mm);
> > +       if (check_stable_address_space(mm))
> > +               goto unlock;
>
> This is checking the MMF_UNSTABLE bit in the mm flags.
> What is the locking requirement for accessing the mm flags MMF_UNSTABLE bit?
> Here we hold the mm mmap read lock.
>
> As far as I can tell, there are two paths that can set that bit.
>
> 1) dup_mm()
> It holds the mm mmap write lock. This path is fine due to the write lock.
> So far the above race against dup_mm(), adding this  check is fine.
>
> 2)  __oom_reap_task_mm()
> It holds the mmap read lock when setting the MMF_UNSTABLE as far as I can tell.
> So checking the MMF_UNSTABLE with another __oom_reap_task_mm() does
> not exclude each other.
> This is more of a question for oom reaping.
> Does MMF_UNSTABLE have the test vs set racing here? It seems this
> check does not protect against  __oom_reap_task_mm(). I have no idea
> if this race is triggerable. Just want someone else to double check if
> my understanding is correct or not.

I haven’t actually run the code.
My guess is there’s a race when checking MMF_UNSTABLE against the
OOM reaper. I think it’s fine either way—whether we skip an OOM-reaped
mm upfront or take a middle path—since the OOM reaper will handle those
PTEs with the PTL just like unuse_pte() does and eventually free the mm
of the reaped process. It’s probably better to skip it early and avoid
unnecessary unuse_pte() calls.

>
> I can see this patch does protect the intended race in dup_mm() vs
> unuse_mm(), it adds value.

This also seems to add values for OOM-reaped processes to avoid a
useless unuse(), in case we aren’t skipping this mm right now. I’m
not sure if we’ve been skipping OOM-reaped processes elsewhere.

Hi Charan, do you have any observations on this? If an additional value is
added, could we record it in the changelog? Otherwise, can we add some
description in the changelog to address Chris’ concern?

Thanks
Barry