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: <5a312d38-4591-47b1-9a6c-4a7242dbe20d@lucifer.local>
Date: Thu, 29 Aug 2024 16:44:15 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jeff Xu <jeffxu@...omium.org>
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

On Thu, Aug 29, 2024 at 08:30:11AM GMT, Jeff Xu wrote:
> 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.
>

Jeff you've ignored pretty much everything I've said here. This is not
collaboration. And you keep doing this + causing disharmony among other
devleopers. It's getting tiresome, and you need to do better.

If you insist on review for this patch as it stands - NACK.

The commit message is ludicriously short, you've not sent the series
correctly, and you are ignoring feedback.

Resend this with a substantially improved commit message and ideally some
actual comments in your tests rather than a giant lump of code which
constitutes 'how Jeff feels mseal() should work'.

Then when people give feedback - listen.