Messages in this thread | | | Date | Wed, 30 Oct 2019 11:59:00 -0700 | From | Kees Cook <> | Subject | Re: [PATCH linux-kselftest/test v1] apparmor: add AppArmor KUnit tests for policy unpack |
| |
On Thu, Oct 17, 2019 at 05:33:56PM -0700, Iurii Zaikin wrote: > On Thu, Oct 17, 2019 at 5:19 PM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > +config SECURITY_APPARMOR_TEST > > + bool "Build KUnit tests for policy_unpack.c" > > + default n
New options already already default n, this can be left off.
> > + depends on KUNIT && SECURITY_APPARMOR > > + help > > > select SECURITY_APPARMOR ?
"select" doesn't enforce dependencies, so just a "depends ..." is correct.
> > + KUNIT_EXPECT_EQ(test, size, TEST_BLOB_DATA_SIZE); > > + KUNIT_EXPECT_TRUE(test, > > + memcmp(blob, TEST_BLOB_DATA, TEST_BLOB_DATA_SIZE) == 0); > I think this must be KUNIT_ASSERT_EQ(test, size, TEST_BLOB_DATA_SIZE);, > otherwise there could be a buffer overflow in memcmp. All tests that > follow such pattern
Agreed.
> are suspect. Also, not sure about your stylistic preference for > KUNIT_EXPECT_TRUE(test, > memcmp(blob, TEST_BLOB_DATA, TEST_BLOB_DATA_SIZE) == 0); > vs > KUNIT_EXPECT_EQ(test, > 0, > memcmp(blob, TEST_BLOB_DATA, TEST_BLOB_DATA_SIZE));
I like == 0.
-- Kees Cook
| |