linux-kernel - Re: [PATCH v2] mm/damon/tests/vaddr-kunit: init maple tree without MT_FLAGS_LOCK

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: <kuy4nfsjxjul54dj6u7p3gazov4mb2no7svxs37arbhcv34hhf@qmk7fmgc3ns4>
Date: Wed, 4 Sep 2024 15:28:07 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: SeongJae Park <sj@...nel.org>
Cc: Guenter Roeck <linux@...ck-us.net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        David Gow <davidgow@...gle.com>, damon@...ts.linux.dev,
        linux-mm@...ck.org, kunit-dev@...glegroups.com,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm/damon/tests/vaddr-kunit: init maple tree without
 MT_FLAGS_LOCK_EXTERN

* SeongJae Park <sj@...nel.org> [240904 14:53]:
> On Wed, 4 Sep 2024 11:41:28 -0700 Guenter Roeck <linux@...ck-us.net> wrote:
> 
> > On 9/4/24 10:29, SeongJae Park wrote:
> > > damon_test_three_regions_in_vmas() initializes a maple tree with
> > > MM_MT_FLAGS.  The flags contains MT_FLAGS_LOCK_EXTERN, which means
> > > mt_lock of the maple tree will not be used.  And therefore the maple
> > > tree initialization code skips initialization of the mt_lock.  However,
> > > __link_vmas(), which adds vmas for test to the maple tree, uses the
> > > mt_lock.  In other words, the uninitialized spinlock is used.  The
> > > problem becomes clear when spinlock debugging is turned on, since it
> > > reports spinlock bad magic bug.
> > > 
> > > Fix the issue by excluding MT_FLAGS_LOCK_EXTERN from the maple tree
> > > initialization flags.  Note that we don't use empty flags to make it
> > > further similar to the usage of mm maple tree, and to be prepared for
> > > possible future changes, as suggested by Liam.
> > > 
> > > Reported-by: Guenter Roeck <linux@...ck-us.net>
> > > Closes: https://lore.kernel.org/1453b2b2-6119-4082-ad9e-f3c5239bf87e@roeck-us.net
> > > Fixes: d0cf3dd47f0d ("damon: convert __damon_va_three_regions to use the VMA iterator")
> > > Suggested-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> > > Signed-off-by: SeongJae Park <sj@...nel.org>
> > > ---
> > > Changes from v1
> > > (https://lore.kernel.org/20240904004534.1189-1-sj@kernel.org)
> > > - Keep lock usage and update the initialization flags (Liam)
> > 
> > Not sure I understand how this is better. Is the resulting rcu warning
> > considered to be irrelevant or a separate problem ?
> 
> I believe it's a separate potential problem (warning) that deserves a look.
> Meanwhile, this patch is fixing an issue that is definitely a bug, so better to
> fix right now, imho.
> 

Whenever you walk the tree, you need to hold the write lock or the rcu
read lock.  I believe that __damon_va_three_regions() needs to have the
rcu_read_lock()/ruc_read_unlock() around the for_each_vma() loop.

I sent a patch to the old thread that I suspect will fix it - but I
don't see the issue so it is hard to say for sure.

Thanks,
Liam