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@xxxxxxxxxx> 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
