linux-kernel - Re: [PATCH v3 3/4] Documentation: kunit: Warn that exit functions run even if init fails

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: <CAO2JNKUus7h7=Q4rdzYenuFgBqi6Y_SM2=XBLo86yoFKBt1qrA@mail.gmail.com>
Date:   Tue, 25 Apr 2023 11:18:02 +0530
From:   Sadiya Kazi <sadiyakazi@...gle.com>
To:     David Gow <davidgow@...gle.com>
Cc:     Benjamin Berg <benjamin@...solutions.net>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Rae Moar <rmoar@...gle.com>,
        Daniel Latypov <dlatypov@...gle.com>, maxime@...no.tech,
        Stephen Boyd <sboyd@...nel.org>, kunit-dev@...glegroups.com,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] Documentation: kunit: Warn that exit functions run
 even if init fails

On Fri, Apr 21, 2023 at 9:32 AM David Gow <davidgow@...gle.com> wrote:
>
> KUnit's exit functions will run even if the corresponding init function
> fails. It's easy, when writing an exit function, to assume the init
> function succeeded, and (for example) access uninitialised memory or
> dereference NULL pointers.
>
> Note that this case exists and should be handled in the documentation.
>
> Suggested-by: Benjamin Berg <benjamin@...solutions.net>
> Link: https://lore.kernel.org/linux-kselftest/a39af0400abedb2e9b31d84c37551cecc3eed0e1.camel@sipsolutions.net/
> Signed-off-by: David Gow <davidgow@...gle.com>
> ---

Thank you, David. Except for a spelling error, this looks fine to me.
Reviewed-by: Sadiya Kazi <sadiyakazi@...gle.com>

Regards,
Sadiya
>
> No changes since v2:
> https://lore.kernel.org/linux-kselftest/20230419085426.1671703-3-davidgow@google.com/
>
> This patch was introduced in v2.
>
> ---
>  Documentation/dev-tools/kunit/usage.rst | 12 ++++++++++--
>  include/kunit/test.h                    |  3 +++
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index 9f720f1317d3..f6d6c9a9ff54 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -166,7 +166,12 @@ many similar tests. In order to reduce duplication in these closely related
>  tests, most unit testing frameworks (including KUnit) provide the concept of a
>  *test suite*. A test suite is a collection of test cases for a unit of code
>  with optional setup and teardown functions that run before/after the whole
> -suite and/or every test case. For example:
> +suite and/or every test case.
> +
> +.. note::
> +   A test case will only run if it is associated with a test suite.
> +
> +For example:
>
>  .. code-block:: c
>
> @@ -196,7 +201,10 @@ after everything else. ``kunit_test_suite(example_test_suite)`` registers the
>  test suite with the KUnit test framework.
>
>  .. note::
> -   A test case will only run if it is associated with a test suite.
> +   The ``exit`` and ``suite_exit`` functions will run even if ``init`` or
> +   ``suite_init`` fail. Make sure that they can handle any inconsistent
> +   state which may result from ``init`` or ``suite_init`` encoutering errors

Nit: Spelling of "encountering"

> +   or exiting early.
>
>  ``kunit_test_suite(...)`` is a macro which tells the linker to put the
>  specified test suite in a special linker section so that it can be run by KUnit
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 57b309c6ca27..3028a1a3fcad 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -168,6 +168,9 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status)
>   * test case, similar to the notion of a *test fixture* or a *test class*
>   * in other unit testing frameworks like JUnit or Googletest.
>   *
> + * Note that @exit and @suite_exit will run even if @init or @suite_init
> + * fail: make sure they can handle any inconsistent state which may result.
> + *
>   * Every &struct kunit_case must be associated with a kunit_suite for KUnit
>   * to run it.
>   */
> --
> 2.40.0.634.g4ca3ef3211-goog
>