linux-kernel - Re: [PATCH v2 10/61] kernel/fork: Use maple tree for dup

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

Message-ID: <20210818145420.hoieffhzxnxc64qr@revolver>
Date:   Wed, 18 Aug 2021 14:54:29 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Hillf Danton <hdanton@...a.com>
CC:     "maple-tree@...ts.infradead.org" <maple-tree@...ts.infradead.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Michel Lespinasse <walken.cr@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 10/61] kernel/fork: Use maple tree for dup_mmap()
 during forking

* Hillf Danton <hdanton@...a.com> [210818 04:36]:
> On Tue, 17 Aug 2021 15:47:11 +0000 Liam R. Howlett wrote:
> > 
> >  static inline void mmget(struct mm_struct *mm)
> >  {
> > +	mt_set_in_rcu(&mm->mm_mt);
> >  	atomic_inc(&mm->mm_users);
> >  }
> > 
> >  static inline bool mmget_not_zero(struct mm_struct *mm)
> >  {
> > +	/*
> > +	 * There is a race below during task tear down that can cause the maple
> > +	 * tree to enter rcu mode with only a single user.  If this race
> > +	 * happens, the result would be that the maple tree nodes would remain
> > +	 * active for an extra RCU read cycle.
> > +	 */
> > +	mt_set_in_rcu(&mm->mm_mt);
> >  	return atomic_inc_not_zero(&mm->mm_users);
> >  }
> 
> Nit, leave the mm with zero refcount intact.
> 
>  	if (atomic_inc_not_zero(&mm->mm_users)) {
> 		mt_set_in_rcu(&mm->mm_mt);
> 		return true;
> 	}
> 	return false;

Thanks for looking at this.

I thought about that, but came up with the following scenario:

thread 1	thread 2
mmget(mm)
		mmget_not_zero() enter..
		atomic_inc_not_zero(&mm->mm_users)
mmput(mm)
		mt_set_in_rcu(&mm->mm_mt);
		return true;


So I think the above does not remove the race, but does add instructions
to each call to mmget_not_zero().  I thought the race of having nodes
sitting around for an rcu read cycle was worth the trade off.

Cheers,
Liam