Re: [PATCH v3 0/7] Convert overflow and stackinit to KUnit

[David Gow]

On Mon, Feb 28, 2022 at 2:45 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> These changes all build on each other, so I'm sending this as a series
> to hopefully reduce confusion. I chose "v3" because it seemed the least
> confusing of various options.
>
> The um changes are needed to get Clang building with um to test the
> stackinit KUnit test more easily (i.e. needing neither GCC 12 nor QEMU).
>
> -Kees
>
> David Gow (2):
>   um: Cleanup syscall_handler_t definition/cast, fix warning
>   um: Remove unused timeval_to_ns() function
>
> Kees Cook (5):
>   overflow: Provide constant expression struct_size
>   lib: overflow: Convert to Kunit
>   um: Allow builds with Clang
>   lib: stackinit: Convert to KUnit
>   UAPI: Introduce KUnit userspace compatibility
>
>  arch/um/os-Linux/execvp.c                   |   1 +
>  arch/um/os-Linux/time.c                     |   6 -
>  arch/x86/um/shared/sysdep/syscalls_64.h     |   5 +-
>  arch/x86/um/user-offsets.c                  |   9 +-
>  include/linux/overflow.h                    |  10 +-
>  include/uapi/misc/kunit.h                   | 181 +++++++
>  lib/Kconfig.debug                           |  38 +-
>  lib/Makefile                                |   6 +-
>  lib/{test_overflow.c => overflow_kunit.c}   | 562 +++++++++-----------
>  lib/{test_stackinit.c => stackinit_kunit.c} | 268 ++++------
>  scripts/Makefile.clang                      |   1 +
>  11 files changed, 593 insertions(+), 494 deletions(-)
>  create mode 100644 include/uapi/misc/kunit.h
>  rename lib/{test_overflow.c => overflow_kunit.c} (54%)
>  rename lib/{test_stackinit.c => stackinit_kunit.c} (67%)
>
> --

Thanks for putting all of these together. Apart from some minor
checkpatch warnings (some of which are inevitable, some of which are
trivial "don't put a space here" things) and an issue with the KUnit
userspace compatibility layer misreporting failed suites, this all
worked pretty well on my machine.

(There's also still a warning with -mno-global-merge on uml
USER_CFLAGS users which shows up with clang, but that's a minor issue
at best. I've sent out a patch to fix it up, though I'm not 100%
convinced it's the right solution:
https://lore.kernel.org/linux-kselftest/20220303090643.241747-1-davidgow@xxxxxxxxxx/
)

Otherwise, this series is:
Tested-by: David Gow <davidgow@xxxxxxxxxx>

(And, as a note to anyone else trying to apply it on another branch,
it has a prerequisite of:
https://lore.kernel.org/linux-hardening/20220124232342.3113350-1-keescook@xxxxxxxxxxxx/
)

Cheers,
-- David

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature