From: Kees Cook <keescook@chromium.org>
To: David Gow <davidgow@google.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Nick Desaulniers <ndesaulniers@google.com>,
Vitor Massaru Iha <vitor@massaru.org>,
Daniel Latypov <dlatypov@google.com>,
Nathan Chancellor <nathan@kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
KUnit Development <kunit-dev@googlegroups.com>,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2] lib: overflow: Convert to Kunit
Date: Sun, 27 Feb 2022 09:33:46 -0800 [thread overview]
Message-ID: <202202270929.5FD6A89B@keescook> (raw)
In-Reply-To: <CABVgOS=TWVh649_Vjo3wnMu9gZnq66gkV-LtGgsksAWMqc+MSA@mail.gmail.com>
On Fri, Feb 25, 2022 at 03:57:17PM +0800, David Gow wrote:
> On Thu, Feb 24, 2022 at 1:48 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Convert overflow unit tests to KUnit, for better integration into the
> > kernel self test framework. Includes a rename of test_overflow.c to
> > overflow_kunit.c, and CONFIG_TEST_OVERFLOW to CONFIG_OVERFLOW_KUNIT_TEST.
> >
> > $ ./tools/testing/kunit/kunit.py config
> > ...
> (This 'config' step should be unnecessary... See below)
Ah yeah. I've removed that now.
> > $ ./tools/testing/kunit/kunit.py run overflow
> > ...
> > [14:33:51] Starting KUnit Kernel (1/1)...
> > [14:33:51] ============================================================
> > [14:33:51] ================== overflow (11 subtests) ==================
> > [14:33:51] [PASSED] u8_overflow_test
> > [14:33:51] [PASSED] s8_overflow_test
> > [14:33:51] [PASSED] u16_overflow_test
> > [14:33:51] [PASSED] s16_overflow_test
> > [14:33:51] [PASSED] u32_overflow_test
> > [14:33:51] [PASSED] s32_overflow_test
> > [14:33:51] [PASSED] u64_overflow_test
> > [14:33:51] [PASSED] s64_overflow_test
> > [14:33:51] [PASSED] overflow_shift_test
> > [14:33:51] [PASSED] overflow_allocation_test
> > [14:33:51] [PASSED] overflow_size_helpers_test
> > [14:33:51] ==================== [PASSED] overflow =====================
> > [14:33:51] ============================================================
> > [14:33:51] Testing complete. Passed: 11, Failed: 0, Crashed: 0, Skipped: 0, Errors: 0
> > [14:33:51] Elapsed time: 12.525s total, 0.001s configuring, 12.402s building, 0.101s running
> >
> > Cc: David Gow <davidgow@google.com>
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > Link: https://lore.kernel.org/lkml/20200720224418.200495-1-vitor@massaru.org/
> > Co-developed-by: Daniel Latypov <dlatypov@google.com>
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > Link: https://lore.kernel.org/linux-kselftest/20210503211536.1384578-1-dlatypov@google.com/
> > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> > Link: https://lore.kernel.org/lkml/CAKwvOdm62iA1dNiC6Q11UJ-MnTqtc4kXkm-ubPaFMK824_k0nw@mail.gmail.com
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > v1: https://lore.kernel.org/lkml/20220216224153.2242451-1-keescook@chromium.org/
> > v2: Fixed up the authorship more, pulled in other prior changes (Daniel)
>
> This looks good to me. Some of the test cases _could_ probably be done
> as parameterised tests (given they're already basically table-driven),
> but I admit that could end up being excessively verbose.
In the end there was so much churn already I decided to leave it as-is.
> Apart from my not being able to find a tree this applies cleanly on,
> the tests work well here.
Ah, sorry about that! (More below...)
> Reviewed-by: David Gow <davidgow@google.com>
Thanks!
> [...]
> > @@ -600,127 +554,118 @@ struct __test_flex_array {
> > unsigned long data[];
> "> };
> >
> > -static int __init test_overflow_size_helpers(void)
> > +static void overflow_size_helpers_test(struct kunit *test)
> > {
> > /* Make sure struct_size() can be used in a constant expression. */
> > u8 ce_array[struct_size((struct __test_flex_array *)0, data, 55)];
> > struct __test_flex_array *obj;
> > int count = 0;
> > - int err = 0;
> > int var;
> > volatile int unconst = 0;
>
> Where does this variable declaration (and its use below) come from?
> It's not in the version of the "overflow: Implement size_t saturating
> arithmetic helpers" patch in linux-next:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=6312fc63aee9d2fef2f9c4d42e57e5b46828f0dc
>
> It all works if I add it in manually, but it does make applying the
> change as-is something of a pain.
Ah, thanks for noticing that! It looks like I failed to send an update
for that patch. I'll resend this portion of my tree.
-Kees
--
Kees Cook
next prev parent reply other threads:[~2022-02-27 17:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-24 5:48 [PATCH v2] lib: overflow: Convert to Kunit Kees Cook
2022-02-24 21:32 ` Gustavo A. R. Silva
2022-02-25 7:57 ` David Gow
2022-02-27 17:33 ` Kees Cook [this message]
2022-03-23 15:19 ` Guenter Roeck
2022-03-23 19:44 ` Daniel Latypov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202202270929.5FD6A89B@keescook \
--to=keescook@chromium.org \
--cc=arnd@arndb.de \
--cc=davidgow@google.com \
--cc=dlatypov@google.com \
--cc=gustavoars@kernel.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=vitor@massaru.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).