From: Jeff Xu <jeffxu@chromium.org>
To: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: akpm@linux-foundation.org, keescook@chromium.org,
jannh@google.com, sroettger@google.com, willy@infradead.org,
gregkh@linuxfoundation.org, torvalds@linux-foundation.org,
corbet@lwn.net, Liam.Howlett@oracle.com, surenb@google.com,
merimus@google.com, rdunlap@infradead.org, jeffxu@google.com,
jorgelo@chromium.org, groeck@chromium.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-mm@kvack.org, pedro.falcato@gmail.com,
dave.hansen@intel.com, linux-hardening@vger.kernel.org,
deraadt@openbsd.org
Subject: Re: [PATCH v10 3/5] selftest mm/mseal memory sealing
Date: Mon, 15 Apr 2024 13:27:32 -0700 [thread overview]
Message-ID: <CABi2SkVnhcOpeA63XoKq97Uc_VyWdLUqYdOtUXxJ1fos5-Tq2g@mail.gmail.com> (raw)
In-Reply-To: <e1744539-a843-468a-9101-ce7a08669394@collabora.com>
On Mon, Apr 15, 2024 at 11:32 AM Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> Please fix following for this and fifth patch as well:
>
> --> checkpatch.pl --codespell tools/testing/selftests/mm/mseal_test.c
>
> WARNING: Macros with flow control statements should be avoided
> #42: FILE: tools/testing/selftests/mm/mseal_test.c:42:
> +#define FAIL_TEST_IF_FALSE(c) do {\
> + if (!(c)) {\
> + ksft_test_result_fail("%s, line:%d\n", __func__,
> __LINE__);\
> + goto test_end;\
> + } \
> + } \
> + while (0)
>
> WARNING: Macros with flow control statements should be avoided
> #50: FILE: tools/testing/selftests/mm/mseal_test.c:50:
> +#define SKIP_TEST_IF_FALSE(c) do {\
> + if (!(c)) {\
> + ksft_test_result_skip("%s, line:%d\n", __func__,
> __LINE__);\
> + goto test_end;\
> + } \
> + } \
> + while (0)
>
> WARNING: Macros with flow control statements should be avoided
> #59: FILE: tools/testing/selftests/mm/mseal_test.c:59:
> +#define TEST_END_CHECK() {\
> + ksft_test_result_pass("%s\n", __func__);\
> + return;\
> +test_end:\
> + return;\
> +}
>
I tried to fix those warnings of checkpatch in the past, but no good
solution. If I put the condition check in the test, the code will have
too many "if" and decrease readability. If there is a better
solution, I'm happy to do that, suggestions are welcome.
>
> On 4/15/24 9:35 PM, jeffxu@chromium.org wrote:
> > From: Jeff Xu <jeffxu@chromium.org>
> >
> > selftest for memory sealing change in mmap() and mseal().
> >
> > Signed-off-by: Jeff Xu <jeffxu@chromium.org>
> > ---
> > tools/testing/selftests/mm/.gitignore | 1 +
> > tools/testing/selftests/mm/Makefile | 1 +
> > tools/testing/selftests/mm/mseal_test.c | 1836 +++++++++++++++++++++++
> > 3 files changed, 1838 insertions(+)
> > create mode 100644 tools/testing/selftests/mm/mseal_test.c
> >
> > diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> > index d26e962f2ac4..98eaa4590f11 100644
> > --- a/tools/testing/selftests/mm/.gitignore
> > +++ b/tools/testing/selftests/mm/.gitignore
> > @@ -47,3 +47,4 @@ mkdirty
> > va_high_addr_switch
> > hugetlb_fault_after_madv
> > hugetlb_madv_vs_map
> > +mseal_test
> > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> > index eb5f39a2668b..95d10fe1b3c1 100644
> > --- a/tools/testing/selftests/mm/Makefile
> > +++ b/tools/testing/selftests/mm/Makefile
> > @@ -59,6 +59,7 @@ TEST_GEN_FILES += mlock2-tests
> > TEST_GEN_FILES += mrelease_test
> > TEST_GEN_FILES += mremap_dontunmap
> > TEST_GEN_FILES += mremap_test
> > +TEST_GEN_FILES += mseal_test
> > TEST_GEN_FILES += on-fault-limit
> > TEST_GEN_FILES += pagemap_ioctl
> > TEST_GEN_FILES += thuge-gen
> > diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> > new file mode 100644
> > index 000000000000..06c780d1d8e5
> > --- /dev/null
> > +++ b/tools/testing/selftests/mm/mseal_test.
> > +static void __write_pkey_reg(u64 pkey_reg)
> > +{
> > +#if defined(__i386__) || defined(__x86_64__) /* arch */
> > + unsigned int eax = pkey_reg;
> > + unsigned int ecx = 0;
> > + unsigned int edx = 0;
> > +
> > + asm volatile(".byte 0x0f,0x01,0xef\n\t"
> > + : : "a" (eax), "c" (ecx), "d" (edx));
> > + assert(pkey_reg == __read_pkey_reg());
> Use ksft_exit_fail_msg instead of assert to stay inside TAP format if
> condition is false and error is generated.
>
I can remove the usage of assert() from the test.
> > +int main(int argc, char **argv)
> > +{
> > + bool test_seal = seal_support();
> > +
> > + ksft_print_header();
> > +
> > + if (!test_seal)
> > + ksft_exit_skip("sealing not supported, check CONFIG_64BIT\n");
> > +
> > + if (!pkey_supported())
> > + ksft_print_msg("PKEY not supported\n");
> > +
> > + ksft_set_plan(80);
> > +
> > + test_seal_addseal();
> > + test_seal_unmapped_start();
> > + test_seal_unmapped_middle();
> > + test_seal_unmapped_end();
> > + test_seal_multiple_vmas();
> > + test_seal_split_start();
> > + test_seal_split_end();
> > + test_seal_invalid_input();
> > + test_seal_zero_length();
> > + test_seal_twice();
> > +
> > + test_seal_mprotect(false);
> > + test_seal_mprotect(true);
> > +
> > + test_seal_start_mprotect(false);
> > + test_seal_start_mprotect(true);
> > +
> > + test_seal_end_mprotect(false);
> > + test_seal_end_mprotect(true);
> > +
> > + test_seal_mprotect_unalign_len(false);
> > + test_seal_mprotect_unalign_len(true);
> > +
> > + test_seal_mprotect_unalign_len_variant_2(false);
> > + test_seal_mprotect_unalign_len_variant_2(true);
> > +
> > + test_seal_mprotect_two_vma(false);
> > + test_seal_mprotect_two_vma(true);
> > +
> > + test_seal_mprotect_two_vma_with_split(false);
> > + test_seal_mprotect_two_vma_with_split(true);
> > +
> > + test_seal_mprotect_partial_mprotect(false);
> > + test_seal_mprotect_partial_mprotect(true);
> > +
> > + test_seal_mprotect_two_vma_with_gap(false);
> > + test_seal_mprotect_two_vma_with_gap(true);
> > +
> > + test_seal_mprotect_merge(false);
> > + test_seal_mprotect_merge(true);
> > +
> > + test_seal_mprotect_split(false);
> > + test_seal_mprotect_split(true);
> > +
> > + test_seal_munmap(false);
> > + test_seal_munmap(true);
> > + test_seal_munmap_two_vma(false);
> > + test_seal_munmap_two_vma(true);
> > + test_seal_munmap_vma_with_gap(false);
> > + test_seal_munmap_vma_with_gap(true);
> > +
> > + test_munmap_start_freed(false);
> > + test_munmap_start_freed(true);
> > + test_munmap_middle_freed(false);
> > + test_munmap_middle_freed(true);
> > + test_munmap_end_freed(false);
> > + test_munmap_end_freed(true);
> > +
> > + test_seal_mremap_shrink(false);
> > + test_seal_mremap_shrink(true);
> > + test_seal_mremap_expand(false);
> > + test_seal_mremap_expand(true);
> > + test_seal_mremap_move(false);
> > + test_seal_mremap_move(true);
> > +
> > + test_seal_mremap_shrink_fixed(false);
> > + test_seal_mremap_shrink_fixed(true);
> > + test_seal_mremap_expand_fixed(false);
> > + test_seal_mremap_expand_fixed(true);
> > + test_seal_mremap_move_fixed(false);
> > + test_seal_mremap_move_fixed(true);
> > + test_seal_mremap_move_dontunmap(false);
> > + test_seal_mremap_move_dontunmap(true);
> > + test_seal_mremap_move_fixed_zero(false);
> > + test_seal_mremap_move_fixed_zero(true);
> > + test_seal_mremap_move_dontunmap_anyaddr(false);
> > + test_seal_mremap_move_dontunmap_anyaddr(true);
> > + test_seal_discard_ro_anon(false);
> > + test_seal_discard_ro_anon(true);
> > + test_seal_discard_ro_anon_on_rw(false);
> > + test_seal_discard_ro_anon_on_rw(true);
> > + test_seal_discard_ro_anon_on_shared(false);
> > + test_seal_discard_ro_anon_on_shared(true);
> > + test_seal_discard_ro_anon_on_filebacked(false);
> > + test_seal_discard_ro_anon_on_filebacked(true);
> > + test_seal_mmap_overwrite_prot(false);
> > + test_seal_mmap_overwrite_prot(true);
> > + test_seal_mmap_expand(false);
> > + test_seal_mmap_expand(true);
> > + test_seal_mmap_shrink(false);
> > + test_seal_mmap_shrink(true);
> > +
> > + test_seal_merge_and_split();
> > + test_seal_zero_address();
> > +
> > + test_seal_discard_ro_anon_on_pkey(false);
> > + test_seal_discard_ro_anon_on_pkey(true);
> > +
> > + ksft_finished();
> > + return 0;
> The return isn't needed as ksft_finished() calls exit() with right exit code.
>
Sure. I can remove "return 0"
Thanks
-Jeff
- Jeff
> > +}
>
> --
> BR,
> Muhammad Usama Anjum
next prev parent reply other threads:[~2024-04-15 20:27 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 16:35 [PATCH v10 0/5] Introduce mseal jeffxu
2024-04-15 16:35 ` [PATCH v10 1/5] mseal: Wire up mseal syscall jeffxu
2024-04-15 18:12 ` Muhammad Usama Anjum
2024-04-15 18:21 ` Linus Torvalds
2024-04-15 19:06 ` Jeff Xu
2024-04-15 16:35 ` [PATCH v10 2/5] mseal: add " jeffxu
2024-04-16 14:59 ` Liam R. Howlett
2024-04-16 15:17 ` Jann Horn
2024-04-16 16:42 ` Theo de Raadt
2024-04-15 16:35 ` [PATCH v10 3/5] selftest mm/mseal memory sealing jeffxu
2024-04-15 18:32 ` Muhammad Usama Anjum
2024-04-15 20:27 ` Jeff Xu [this message]
2024-04-16 0:34 ` Kees Cook
2024-05-02 11:24 ` Ryan Roberts
2024-05-02 15:18 ` Jeff Xu
2024-05-02 22:39 ` Jeff Xu
2024-05-03 8:30 ` Ryan Roberts
2024-04-15 16:35 ` [PATCH v10 4/5] mseal:add documentation jeffxu
2024-04-15 16:35 ` [PATCH v10 5/5] selftest mm/mseal read-only elf memory segment jeffxu
2024-04-16 15:13 ` [PATCH v10 0/5] Introduce mseal Liam R. Howlett
2024-04-16 19:40 ` Jeff Xu
2024-04-18 20:19 ` Suren Baghdasaryan
2024-04-19 1:22 ` Jeff Xu
2024-04-19 14:57 ` Suren Baghdasaryan
2024-04-19 15:14 ` Jeff Xu
2024-04-19 16:54 ` Suren Baghdasaryan
2024-04-19 17:59 ` Pedro Falcato
2024-04-20 1:23 ` Jeff Xu
2024-05-14 17:46 ` Andrew Morton
2024-05-14 19:52 ` Kees Cook
2024-05-23 23:32 ` Kees Cook
2024-05-23 23:54 ` Andrew Morton
2024-05-24 15:19 ` Linus Torvalds
2024-05-14 20:59 ` Jonathan Corbet
2024-05-14 21:28 ` Matthew Wilcox
2024-05-14 22:48 ` Theo de Raadt
2024-05-14 23:01 ` Andrew Morton
2024-05-14 23:47 ` Theo de Raadt
2024-05-15 2:58 ` Willy Tarreau
2024-05-15 3:36 ` Linus Torvalds
2024-05-15 4:14 ` Linus Torvalds
2024-05-15 6:14 ` Willy Tarreau
2024-05-15 0:43 ` Linus Torvalds
2024-05-15 0:57 ` Theo de Raadt
2024-05-15 1:20 ` Linus Torvalds
2024-05-15 1:47 ` Theo de Raadt
2024-05-15 2:28 ` Linus Torvalds
2024-05-15 2:42 ` Theo de Raadt
2024-05-15 4:53 ` Liam R. Howlett
2024-05-14 21:28 ` Liam R. Howlett
2024-05-15 17:18 ` Jeff Xu
2024-05-15 22:19 ` Liam R. Howlett
2024-05-16 0:59 ` Jeff Xu
2024-05-21 16:00 ` Liam R. Howlett
2024-05-21 20:55 ` Jeff Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABi2SkVnhcOpeA63XoKq97Uc_VyWdLUqYdOtUXxJ1fos5-Tq2g@mail.gmail.com \
--to=jeffxu@chromium.org \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=corbet@lwn.net \
--cc=dave.hansen@intel.com \
--cc=deraadt@openbsd.org \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@chromium.org \
--cc=jannh@google.com \
--cc=jeffxu@google.com \
--cc=jorgelo@chromium.org \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=merimus@google.com \
--cc=pedro.falcato@gmail.com \
--cc=rdunlap@infradead.org \
--cc=sroettger@google.com \
--cc=surenb@google.com \
--cc=torvalds@linux-foundation.org \
--cc=usama.anjum@collabora.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).