linux-hardening - Re: [PATCH v1 2/2] selftests/mm: mseal

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: <CABi2SkVe6Y4xypBw0n8QbqKJgsfy9YRNJWvBZ3bjTa=-Z5Zn2g@mail.gmail.com>
Date: Thu, 29 Aug 2024 08:30:11 -0700
From: Jeff Xu <jeffxu@...omium.org>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: akpm@...ux-foundation.org, linux-kselftest@...r.kernel.org, 
	linux-mm@...ck.org, linux-hardening@...r.kernel.org, pedro.falcato@...il.com, 
	rientjes@...gle.com, keescook@...omium.org, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>
Subject: Re: [PATCH v1 2/2] selftests/mm: mseal_test add more tests

Hi Lorenzo

On Thu, Aug 29, 2024 at 8:14 AM Lorenzo Stoakes
<lorenzo.stoakes@...cle.com> wrote:
>
> On Thu, Aug 29, 2024 at 07:45:56AM GMT, Jeff Xu wrote:
> > HI Andrew
> >
> > On Wed, Aug 28, 2024 at 3:55 PM <jeffxu@...omium.org> wrote:
> > >
> > > From: Jeff Xu <jeffxu@...omium.org>
> > >
> > > Add more testcases and increase test coverage, e.g. add
> > > get_vma_size to check VMA size and prot bits.
>
> This commit message is ridiculously short for such a massive change, even for
> test code.
>
> > >
> >
> > Could you please pull the self-test part of this patch series to mm-unstable ?
> > It will help to prevent regression.
>
> No, please don't.
>
> This needs review.
>
> These tests establish a precedent as to how mseal should behave, this is
> something that needs community review, not to just be taken.
>
> There's already been a great deal of confusion/contentious discussion
> around mseal() and its implementation.
>
> Pushing in ~800 lines of test code asserting how mseal() should behave
> without review isn't helping things.
>
> Also, this is a really unusual way to send a series - why is this a 2/2 in
> reply to the 1/2 and no cover letter? Why is this change totally unrelated
> to the other patch?
>
> Can you send this as a separate patch, preferably as an RFC so we can
> ensure that we all agree on how mseal() should behave?
>
> Sorry to be contentious here, but I think we need to find a more
> constructive, collaborative way forward with mseal() and to act with a
> little more caution, given the problems that the original series has caused
> I'd think this is in the best interests of all.
>
> Thanks for understanding!
>
There have been two bugs I found recently on mseal.
One during V2 of in-loop change and the other mentioned in 1/2 of this patch.

These additional tests will increase the test coverage of mseal and
ensure the functionality of in-loop change is correct, also help to
detect future regression.

If you feel it will take time to review the test case, please do so
and comment on the tests itself directly, I will send V2 after that.

Thanks
-Jeff


> >
> > The first part ([PATCH v1 1/2] mseal: fix mmap(FIXED) error code) can
> > be ignored as Liam proposed to fix it differently.
> >
> > Thanks
> > -Jeff
> >
> > -Jeff
> >
> > > Signed-off-by: Jeff Xu <jeffxu@...omium.org>
> > > ---
> > >  tools/testing/selftests/mm/mseal_test.c | 826 ++++++++++++++++++++++--
> > >  1 file changed, 759 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> > > index e7991e5fdcf3..4b3f883aae17 100644
> > > --- a/tools/testing/selftests/mm/mseal_test.c
> > > +++ b/tools/testing/selftests/mm/mseal_test.c