Received-SPF: pass (google.com: domain of linux-kernel+bounces-88306-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240301002416.it.092-kees@kernel.org> <20240301002650.1544829-1-keescook@chromium.org>
In-Reply-To: <20240301002650.1544829-1-keescook@chromium.org>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Fri, 1 Mar 2024 13:09:27 +0200
Message-ID: <CAHp75VdhY6-OmNYtbpnb+dv1Du=LTnignsTRSU9+8yqpbFqPsQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] string: Convert selftest to KUnit
To: Kees Cook <keescook@chromium.org>
Cc: Andy Shevchenko <andy@kernel.org>, linux-hardening@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 1, 2024 at 2:26=E2=80=AFAM Kees Cook <keescook@chromium.org> wr=
ote:
>
> Convert test_string.c to KUnit so it can be easily run with everything
> else.

Have you run it?

..

>         if (i < 256)
> -               return (i << 24) | (j << 16) | k | 0x8000;
> -       return 0;
> +               KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k | 0x80=
00);

First of all, this special value encodes the problematic patterns, so
you missed proper messaging.
Second, the returned value has a constant, how do you expect 0 to be
equal to something (guaranteed not to be 0)?

This needs a good rethink of what you should do in the KUnit approach.

..

> +               KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k | 0x80=
00);

Ditto.

..

> +               KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k | 0x80=
00);

Ditto.

..

>         for (i =3D 0; i < strlen(test_string) + 1; i++) {
>                 result =3D strchr(test_string, test_string[i]);
> -               if (result - test_string !=3D i)
> -                       return i + 'a';
> +               KUNIT_ASSERT_EQ(test, result - test_string, i);

In a similar way, all returned values are *special*, you really need
to think about them before converting to a simple (and sometimes
wrong) checks)

..

I dunno if KUnit has a fault ejection simulation. It should, in order
to be sure that test cases are fine when they fail.

--=20
With Best Regards,
Andy Shevchenko