| 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
| ||
|
Message-ID: <CABVgOSnNwWUsRcTaAJgVy76ikJ740OYuDjdFLwhnTN1rQg_x3w@mail.gmail.com>
Date: Sat, 2 Sep 2023 16:24:27 +0800
From: David Gow <davidgow@...gle.com>
To: Richard Fitzgerald <rf@...nsource.cirrus.com>
Cc: brendan.higgins@...ux.dev, rmoar@...gle.com,
linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH v6 08/10] kunit: string-stream: Add tests for freeing
resource-managed string_stream
On Mon, 28 Aug 2023 at 18:41, Richard Fitzgerald
<rf@...nsource.cirrus.com> wrote:
>
> string_stream_managed_free_test() allocates a resource-managed
> string_stream and tests that kunit_free_string_stream() calls
> string_stream_destroy().
>
> string_stream_resource_free_test() allocates a resource-managed
> string_stream and tests that string_stream_destroy() is called
> when the test resources are cleaned up.
>
> The old string_stream_init_test() has been split into two tests,
> one for kunit_alloc_string_stream() and the other for
> alloc_string_stream().
>
> Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
> ---
> Changes since v5:
> - Fix memory leak when calling the redirected string_stream_destroy_stub().
> ---
Much better, thanks.
Reviewed-by: David Gow <davidgow@...gle.com>
Cheers,
-- David
> lib/kunit/string-stream-test.c | 147 +++++++++++++++++++++++++++++++--
> lib/kunit/string-stream.c | 3 +
> 2 files changed, 145 insertions(+), 5 deletions(-)
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 58ba1ef5207f..b759974d9cec 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -6,17 +6,33 @@
> * Author: Brendan Higgins <brendanhiggins@...gle.com>
> */
>
> +#include <kunit/static_stub.h>
> #include <kunit/test.h>
> #include <linux/slab.h>
>
> #include "string-stream.h"
>
> -/* This avoids a cast warning if kfree() is passed direct to kunit_add_action(). */
> +struct string_stream_test_priv {
> + /* For testing resource-managed free. */
> + struct string_stream *expected_free_stream;
> + bool stream_was_freed;
> + bool stream_free_again;
> +};
> +
> +/* Avoids a cast warning if kfree() is passed direct to kunit_add_action(). */
> static void kfree_wrapper(void *p)
> {
> kfree(p);
> }
>
> +/* Avoids a cast warning if string_stream_destroy() is passed direct to kunit_add_action(). */
> +static void cleanup_raw_stream(void *p)
> +{
> + struct string_stream *stream = p;
> +
> + string_stream_destroy(stream);
> +}
> +
> static char *get_concatenated_string(struct kunit *test, struct string_stream *stream)
> {
> char *str = string_stream_get_string(stream);
> @@ -27,11 +43,12 @@ static char *get_concatenated_string(struct kunit *test, struct string_stream *s
> return str;
> }
>
> -/* string_stream object is initialized correctly. */
> -static void string_stream_init_test(struct kunit *test)
> +/* Managed string_stream object is initialized correctly. */
> +static void string_stream_managed_init_test(struct kunit *test)
> {
> struct string_stream *stream;
>
> + /* Resource-managed initialization. */
> stream = kunit_alloc_string_stream(test, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
>
> @@ -42,6 +59,109 @@ static void string_stream_init_test(struct kunit *test)
> KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
> }
>
> +/* Unmanaged string_stream object is initialized correctly. */
> +static void string_stream_unmanaged_init_test(struct kunit *test)
> +{
> + struct string_stream *stream;
> +
> + stream = alloc_string_stream(GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
> + kunit_add_action(test, cleanup_raw_stream, stream);
> +
> + KUNIT_EXPECT_EQ(test, stream->length, 0);
> + KUNIT_EXPECT_TRUE(test, list_empty(&stream->fragments));
> + KUNIT_EXPECT_EQ(test, stream->gfp, GFP_KERNEL);
> + KUNIT_EXPECT_FALSE(test, stream->append_newlines);
> +
> + KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
> +}
> +
> +static void string_stream_destroy_stub(struct string_stream *stream)
> +{
> + struct kunit *fake_test = kunit_get_current_test();
> + struct string_stream_test_priv *priv = fake_test->priv;
> +
> + /* The kunit could own string_streams other than the one we are testing. */
> + if (stream == priv->expected_free_stream) {
> + if (priv->stream_was_freed)
> + priv->stream_free_again = true;
> + else
> + priv->stream_was_freed = true;
> + }
> +
> + /*
> + * Calling string_stream_destroy() will only call this function again
> + * because the redirection stub is still active.
> + * Avoid calling deactivate_static_stub() or changing current->kunit_test
> + * during cleanup.
> + */
> + string_stream_clear(stream);
> + kfree(stream);
> +}
> +
> +/* kunit_free_string_stream() calls string_stream_desrtoy() */
> +static void string_stream_managed_free_test(struct kunit *test)
> +{
> + struct string_stream_test_priv *priv = test->priv;
> +
> + priv->expected_free_stream = NULL;
> + priv->stream_was_freed = false;
> + priv->stream_free_again = false;
> +
> + kunit_activate_static_stub(test,
> + string_stream_destroy,
> + string_stream_destroy_stub);
> +
> + priv->expected_free_stream = kunit_alloc_string_stream(test, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->expected_free_stream);
> +
> + /* This should call the stub function. */
> + kunit_free_string_stream(test, priv->expected_free_stream);
> +
> + KUNIT_EXPECT_TRUE(test, priv->stream_was_freed);
> + KUNIT_EXPECT_FALSE(test, priv->stream_free_again);
> +}
> +
> +/* string_stream object is freed when test is cleaned up. */
> +static void string_stream_resource_free_test(struct kunit *test)
> +{
> + struct string_stream_test_priv *priv = test->priv;
> + struct kunit *fake_test;
> +
> + fake_test = kunit_kzalloc(test, sizeof(*fake_test), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fake_test);
> +
> + kunit_init_test(fake_test, "string_stream_fake_test", NULL);
> + fake_test->priv = priv;
> +
> + /*
> + * Activate stub before creating string_stream so the
> + * string_stream will be cleaned up first.
> + */
> + priv->expected_free_stream = NULL;
> + priv->stream_was_freed = false;
> + priv->stream_free_again = false;
> +
> + kunit_activate_static_stub(fake_test,
> + string_stream_destroy,
> + string_stream_destroy_stub);
> +
> + priv->expected_free_stream = kunit_alloc_string_stream(fake_test, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->expected_free_stream);
> +
> + /* Set current->kunit_test to fake_test so the static stub will be called. */
> + current->kunit_test = fake_test;
> +
> + /* Cleanup test - the stub function should be called */
> + kunit_cleanup(fake_test);
> +
> + /* Set current->kunit_test back to current test. */
> + current->kunit_test = test;
> +
> + KUNIT_EXPECT_TRUE(test, priv->stream_was_freed);
> + KUNIT_EXPECT_FALSE(test, priv->stream_free_again);
> +}
> +
> /*
> * Add a series of lines to a string_stream. Check that all lines
> * appear in the correct order and no characters are dropped.
> @@ -334,8 +454,24 @@ static void string_stream_auto_newline_test(struct kunit *test)
> "One\nTwo\nThree\nFour\nFive\nSix\nSeven\n\nEight\n");
> }
>
> +static int string_stream_test_init(struct kunit *test)
> +{
> + struct string_stream_test_priv *priv;
> +
> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + test->priv = priv;
> +
> + return 0;
> +}
> +
> static struct kunit_case string_stream_test_cases[] = {
> - KUNIT_CASE(string_stream_init_test),
> + KUNIT_CASE(string_stream_managed_init_test),
> + KUNIT_CASE(string_stream_unmanaged_init_test),
> + KUNIT_CASE(string_stream_managed_free_test),
> + KUNIT_CASE(string_stream_resource_free_test),
> KUNIT_CASE(string_stream_line_add_test),
> KUNIT_CASE(string_stream_variable_length_line_test),
> KUNIT_CASE(string_stream_append_test),
> @@ -348,6 +484,7 @@ static struct kunit_case string_stream_test_cases[] = {
>
> static struct kunit_suite string_stream_test_suite = {
> .name = "string-stream-test",
> - .test_cases = string_stream_test_cases
> + .test_cases = string_stream_test_cases,
> + .init = string_stream_test_init,
> };
> kunit_test_suites(&string_stream_test_suite);
> diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> index 64abceb7b716..a6f3616c2048 100644
> --- a/lib/kunit/string-stream.c
> +++ b/lib/kunit/string-stream.c
> @@ -6,6 +6,7 @@
> * Author: Brendan Higgins <brendanhiggins@...gle.com>
> */
>
> +#include <kunit/static_stub.h>
> #include <kunit/test.h>
> #include <linux/list.h>
> #include <linux/slab.h>
> @@ -170,6 +171,8 @@ struct string_stream *alloc_string_stream(gfp_t gfp)
>
> void string_stream_destroy(struct string_stream *stream)
> {
> + KUNIT_STATIC_STUB_REDIRECT(string_stream_destroy, stream);
> +
> if (!stream)
> return;
>
> --
> 2.30.2
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)
Powered by blists - more mailing lists