linux-kernel - Re: [PATCH v3 4/5] selftests/mseal: add more tests for 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: <CABi2SkUQM8girk31jDYMfx8kVoG0xXMwaSDWmu90b4ZmtdmHHQ@mail.gmail.com>
Date: Fri, 20 Sep 2024 09:37:17 -0700
From: Jeff Xu <jeffxu@...omium.org>
To: Mark Brown <broonie@...nel.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	Muhammad Usama Anjum <usama.anjum@...labora.com>, akpm@...ux-foundation.org, 
	linux-kselftest@...r.kernel.org, linux-mm@...ck.org, 
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org, 
	pedro.falcato@...il.com, willy@...radead.org, vbabka@...e.cz, 
	Liam.Howlett@...cle.com, rientjes@...gle.com, keescook@...omium.org
Subject: Re: [PATCH v3 4/5] selftests/mseal: add more tests for mmap

Hi Mark

On Wed, Sep 18, 2024 at 6:18 AM Mark Brown <broonie@...nel.org> wrote:
>
> On Fri, Sep 13, 2024 at 03:50:00PM -0700, Jeff Xu wrote:
>
> > Even though the number of lines is large in these patches, its main
> > intention is to test Pedro's in-place change (from can_modify_mm to
> > can_modify_vma). Before this patch,  the test had a common pattern:
> > setup memory layout, seal the memory, perform a few mm-api steps, verify
> > return code (not zero).  Because of the nature of out-of-loop,  it is
> > sufficient to just verify the error code in a few cases.
>
> > With Pedro's in-loop change, the sealing check happens later in the
> > stack, thus there are more things and scenarios to verify. And there were
> > feedback to me during in-loop change that selftest should be extensive
> > enough to discover all regressions.  Even though this viewpoint is subject
> > to debate. Since none would want to do it, I thought I would just do it.
>
> > So the Patch V3 1/5 is dedicated entirely to increasing the verification
> > for existing scenarios, this including checking return code code, vma-size,
> > etc after mm api return.
>
> > Patch V3 3/5 are for unmap(), during review of V2 of Pedro's in-loop
> > change, we discovered a bug in unmap(), and unmap() is not atomic.
> > This leads to 4/5(mmap), 5/5(mremap), which calls munmap().
> > In addition, I add scenarios to cover cross-multiple-vma cases.
>
> > The  high-level goal of mseal test are two folds:
> > 1> make sure sealing is working correctly under different scenarios,
> > i.e. sealed mapping are not modified.
> > 2> For unsealed memory, added mseal code  doesn't regress on regular mm API.
>
> > The goal 2 is as important as 1, that is why tests usually are done in
> > two phases, one with sealing, the other without.
>
> That's vastly more detail than is in the changelogs for the actual
> patches (which are just a few lines each) or the cover letter of the
> series.  I don't have the MM knowledge to assess the detail of what
> you're saying but I can't help but think that it'd help a lot with
> review if all this detail were part of the actual submission.
Agreed, will update and give more detail in the next version of the patch.

Thanks
-Jeff