linux-hardening - 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZurTHc1C27iqofjp@finisterre.sirena.org.uk>
Date: Wed, 18 Sep 2024 15:18:21 +0200
From: Mark Brown <broonie@...nel.org>
To: Jeff Xu <jeffxu@...omium.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

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)