linux-kernel - Re: [PATCH v5] kunit: Cover 'assert.c' with tests

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

Message-ID: <CABVgOSkFt5beXua-DsFPzebSm1S=o3rqnctzzHHtg-pZC=zssA@mail.gmail.com>
Date: Thu, 30 May 2024 12:22:31 +0800
From: David Gow <davidgow@...gle.com>
To: Ivan Orlov <ivan.orlov0322@...il.com>
Cc: brendan.higgins@...ux.dev, rmoar@...gle.com, 
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kunit-dev@...glegroups.com, skhan@...uxfoundation.org
Subject: Re: [PATCH v5] kunit: Cover 'assert.c' with tests

On Fri, 17 May 2024 at 05:17, Ivan Orlov <ivan.orlov0322@...il.com> wrote:
>
> There are multiple assertion formatting functions in the `assert.c`
> file, which are not covered with tests yet. Implement the KUnit test
> for these functions.
>
> The test consists of 11 test cases for the following functions:
>
> 1) 'is_literal'
> 2) 'is_str_literal'
> 3) 'kunit_assert_prologue', test case for multiple assert types
> 4) 'kunit_assert_print_msg'
> 5) 'kunit_unary_assert_format'
> 6) 'kunit_ptr_not_err_assert_format'
> 7) 'kunit_binary_assert_format'
> 8) 'kunit_binary_ptr_assert_format'
> 9) 'kunit_binary_str_assert_format'
> 10) 'kunit_assert_hexdump'
> 11) 'kunit_mem_assert_format'
>
> The test aims at maximizing the branch coverage for the assertion
> formatting functions.
>
> As you can see, it covers some of the static helper functions as
> well, so mark the static functions in `assert.c` as 'VISIBLE_IF_KUNIT'
> and conditionally export them with EXPORT_SYMBOL_IF_KUNIT. Add the
> corresponding definitions to `assert.h`.
>
> Build the assert test when CONFIG_KUNIT_TEST is enabled, similar to
> how it is done for the string stream test.
>
> Signed-off-by: Ivan Orlov <ivan.orlov0322@...il.com>
> ---

This looks good to me, and passes all of my tests locally.

Note that this does have some checkpatch warnings:

WARNING: Using vsprintf specifier '%px' potentially exposes the kernel
memory layout, if you don't really need the address please consider
using '%p'.
#426: FILE: lib/kunit/assert_test.c:250:
+       snprintf(addr_var_a, TEST_PTR_EXPECTED_BUF_SIZE, "%px", var_a);

WARNING: Using vsprintf specifier '%px' potentially exposes the kernel
memory layout, if you don't really need the address please consider
using '%p'.
#427: FILE: lib/kunit/assert_test.c:251:
+       snprintf(addr_var_b, TEST_PTR_EXPECTED_BUF_SIZE, "%px", var_b);

These are necessary, as KUnit _does_ expose the kernel memory layout
(on purpose), as it's useful for debugging. This is one of the reasons
why KUnit tests taint the kernel: to make it clear that this could be
a problem.

Acked-by: David Gow <davidgow@...gle.com>

Cheers,
-- David

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4014 bytes)