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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABi2SkWAv4LXvR1Wb1e31eyZ35JfyieXhDOq1bp_ZvHPLLg-qA@mail.gmail.com>
Date: Thu, 17 Oct 2024 11:47:15 -0700
From: Jeff Xu <jeffxu@...omium.org>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: 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, broonie@...nel.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 Thu, Oct 17, 2024 at 11:29 AM Lorenzo Stoakes
<lorenzo.stoakes@...cle.com> wrote:
>
> On Thu, Oct 17, 2024 at 11:14:20AM -0700, Jeff Xu wrote:
> > Hi Lorenzo and Muhammad
> >
> > Reviving this thread since the merging window is closed and we have
> > more time to review /work on this code in the next few weeks.
> >
> > On Fri, Sep 13, 2024 at 3:50 PM Jeff Xu <jeffxu@...omium.org> wrote:
> > >
> > > Hi Lorenzo
> > >
> > > On Sat, Sep 7, 2024 at 12:28 PM Lorenzo Stoakes
> > > <lorenzo.stoakes@...cle.com> wrote:
> > > >
> > > >
> > > > I also suggest we figure out this FAIL_TEST_IF_FALSE() thing at this point
> > > > too - I may be missing something, but I cannot for the life me understand
> > > > why we have to assert negations only, and other self tests do not do this.
> > > >
> > > My most test-infra related comments comes from Muhammad Usama Anjum
> > > (added into this email), e.g. assert is not recommended.[1] ,
> > >
> > > [1] https://lore.kernel.org/all/148fc789-3c03-4490-a653-6a4e58f336b6@collabora.com/
> > >
> > Specifically regarding Lorenzo's comments about FAIL_TEST_IF_FALSE
> >
> > Muhammad Usama Anjum doesn't want assert being used in selftest (see
> > [1] above), and I quote:
> > "We don't want to terminate the test if one test fails because of assert. We
> > want the sub-tests to get executed in-dependent of other tests.
> >
> > ksft_test_result(condition, fmt, ...);
> > ksft_test_result_pass(fmt, ...);"
> >
> > FAIL_TEST_IF_FALSE is a wrapper for ksft_test_result macro, and
> > replacement of assert.
> >
> > Please let me know if you have questions on this and Muhammad might
> > also help to clarify the requirement if needed.
> >
> > Thanks
> > -Jeff
>
> Right this is about not failing the test i.e. equivalent of an expect
> rather than an assert, which makes sense.
>
> What I'm saying is we should have something more like
>
> EXPECT_TRUE()
> EXPECT_FALSE()
>
> etc.
>
> Which would avoid these confusing
>
>         FAIL_TEST_IF_FALSE(!expr)

FAIL_TEST_IF_FALSE(expr) is the right way to use this macro.

It is same syntax as assert(expr), e.g:

man assert(expr)
       assert - abort the program if assertion is false

FAIL_TEST_IF_FALSE is a replacement for assert,  instead of aborting
the program, it just reports failure in this test.

Is this still confusing ?
(The FAIL_TEST_IF_FALSE is already a descriptive name, and the syntax
of assert is well known.)


>
> Things.
>
> Hopefully that's clear? Thanks!