linux-kernel - Re: [PATCH 25/34] maple_tree: Clear up index and last setting in single entry tree

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: <20230427172544.4vef5cr52elebiom@revolver>
Date:   Thu, 27 Apr 2023 13:25:44 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Peng Zhang <perlyzhang@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        maple-tree@...ts.infradead.org,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 25/34] maple_tree: Clear up index and last setting in
 single entry tree

* Peng Zhang <perlyzhang@...il.com> [230427 07:20]:
> 
> 
> 在 2023/4/25 22:09, Liam R. Howlett 写道:
> > When there is a single entry tree (range of 0-0 pointing to an entry),
> > then ensure the limit is either 0-0 or 1-oo, depending on where the user
> > walks.  Ensure the correct node setting as well; either MAS_ROOT or
> > MAS_NONE.
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> > ---
> >   lib/maple_tree.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 20f0a10dc5608..31cbfd7b44728 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -5099,24 +5099,25 @@ void *mas_walk(struct ma_state *mas)
> >   {
> >   	void *entry;
> > +	if (mas_is_none(mas) || mas_is_paused(mas))
> > +		mas->node = MAS_START;
> >   retry:
> >   	entry = mas_state_walk(mas);
> > -	if (mas_is_start(mas))
> > +	if (mas_is_start(mas)) {
> >   		goto retry;
> > -
> > -	if (mas_is_ptr(mas)) {
> > +	} else if (mas_is_none(mas)) {
> > +		mas->index = 0;
> > +		mas->last = ULONG_MAX;
> > +	} else if (mas_is_ptr(mas)) {
> >   		if (!mas->index) {
> >   			mas->last = 0;
> > -		} else {
> > -			mas->index = 1;
> > -			mas->last = ULONG_MAX;
> > +			return mas_root(mas);
> Why we call mas_root() to get the single entry stored in root again?
> I think it's not safe. In RCU mode, if someone modify the tree to a normal
> tree(not a single entry tree), mas_root() will return a address.
> So, this may cause a race bug. We can return entry directly.

Good catch.  I will address this in v2.

> >   		}
> > -		return entry;
> > -	}
> > -	if (mas_is_none(mas)) {
> > -		mas->index = 0;
> > +		mas->index = 1;
> >   		mas->last = ULONG_MAX;
> > +		mas->node = MAS_NONE;
> > +		return NULL;
> >   	}
> >   	return entry;