Re: [PATCH linux-kselftest/test v2] apparmor: add AppArmor KUnit tests for policy unpack

On Tue, Nov 05, 2019 at 04:43:29PM -0800, Brendan Higgins wrote:

From: Mike Salvatore mike.salvatore@canonical.com

Add KUnit tests to test AppArmor unpacking of userspace policies. AppArmor uses a serialized binary format for loading policies. To find policy format documentation see Documentation/admin-guide/LSM/apparmor.rst.

In order to write the tests against the policy unpacking code, some static functions needed to be exposed for testing purposes. One of the goals of this patch is to establish a pattern for which testing these kinds of functions should be done in the future.

Signed-off-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Mike Salvatore mike.salvatore@canonical.com

---

security/apparmor/Kconfig | 16 + security/apparmor/policy_unpack.c | 4 + security/apparmor/policy_unpack_test.c | 607 +++++++++++++++++++++++++ 3 files changed, 627 insertions(+) create mode 100644 security/apparmor/policy_unpack_test.c

diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig index d8b1a360a6368..78a33ccac2574 100644 --- a/security/apparmor/Kconfig +++ b/security/apparmor/Kconfig @@ -66,3 +66,19 @@ config SECURITY_APPARMOR_DEBUG_MESSAGES Set the default value of the apparmor.debug kernel parameter. When enabled, various debug messages will be logged to the kernel message buffer.

+config SECURITY_APPARMOR_KUNIT_TEST

• bool "Build KUnit tests for policy_unpack.c"
• depends on KUNIT && SECURITY_APPARMOR
• help

•  This builds the AppArmor KUnit tests.
  

•  KUnit tests run during boot and output the results to the debug log
  

•  in TAP format (http://testanything.org/). Only useful for kernel devs
  

•  running KUnit test harness and are not for inclusion into a
  

•  production build.
  

•  For more information on KUnit and unit tests in general please refer
  

•  to the KUnit documentation in Documentation/dev-tools/kunit/.
  

•  If unsure, say N.
  

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 8cfc9493eefc7..37c1dd3178fc0 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -1120,3 +1120,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh, return error; }

+#ifdef CONFIG_SECURITY_APPARMOR_KUNIT_TEST +#include "policy_unpack_test.c" +#endif /* CONFIG_SECURITY_APPARMOR_KUNIT_TEST */

To make this even LESS intrusive, the ifdefs could live in ..._test.c.

Also, while I *think* the kernel build system will correctly track this dependency, can you double-check that changes to ..._test.c correctly trigger a recompile of policy_unpack.c?