From: Rae Moar <rmoar@google.com>
To: brendanhiggins@google.com, davidgow@google.com, dlatypov@google.com
Cc: skhan@linuxfoundation.org, tales.aparecida@gmail.com,
john.johansen@canonical.com, kunit-dev@googlegroups.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
apparmor@lists.ubuntu.com, Rae Moar <rmoar@google.com>
Subject: [PATCH v1 2/2] apparmor: test: make static symbols visible during kunit testing
Date: Wed, 2 Nov 2022 17:59:59 +0000 [thread overview]
Message-ID: <20221102175959.2921063-3-rmoar@google.com> (raw)
In-Reply-To: <20221102175959.2921063-1-rmoar@google.com>
Use macros, VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT, to allow
static symbols to be conditionally set to be visible during KUnit
testing. Remove the need to include testing file in the implementation
file. Provide example of how static symbols can be dealt with in
testing.
Signed-off-by: Rae Moar <rmoar@google.com>
---
security/apparmor/Kconfig | 4 +-
security/apparmor/Makefile | 2 +
security/apparmor/include/policy_unpack.h | 50 ++++++++++++++++
security/apparmor/policy_unpack.c | 72 +++++++----------------
security/apparmor/policy_unpack_test.c | 5 ++
5 files changed, 80 insertions(+), 53 deletions(-)
diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
index cb3496e00d8a..f334e7cccf2d 100644
--- a/security/apparmor/Kconfig
+++ b/security/apparmor/Kconfig
@@ -106,8 +106,8 @@ config SECURITY_APPARMOR_PARANOID_LOAD
Disabling the check will speed up policy loads.
config SECURITY_APPARMOR_KUNIT_TEST
- bool "Build KUnit tests for policy_unpack.c" if !KUNIT_ALL_TESTS
- depends on KUNIT=y && SECURITY_APPARMOR
+ tristate "Build KUnit tests for policy_unpack.c" if !KUNIT_ALL_TESTS
+ depends on KUNIT && SECURITY_APPARMOR
default KUNIT_ALL_TESTS
help
This builds the AppArmor KUnit tests.
diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile
index ff23fcfefe19..6a92428375eb 100644
--- a/security/apparmor/Makefile
+++ b/security/apparmor/Makefile
@@ -8,6 +8,8 @@ apparmor-y := apparmorfs.o audit.o capability.o task.o ipc.o lib.o match.o \
resource.o secid.o file.o policy_ns.o label.o mount.o net.o
apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o
+obj-$(CONFIG_SECURITY_APPARMOR_KUNIT_TEST) += policy_unpack_test.o
+
clean-files := capability_names.h rlim_names.h net_names.h
# Build a lower case string table of address family names
diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
index eb5f7d7f132b..a963687bcc9b 100644
--- a/security/apparmor/include/policy_unpack.h
+++ b/security/apparmor/include/policy_unpack.h
@@ -48,6 +48,43 @@ enum {
AAFS_LOADDATA_NDENTS /* count of entries */
};
+/*
+ * The AppArmor interface treats data as a type byte followed by the
+ * actual data. The interface has the notion of a named entry
+ * which has a name (AA_NAME typecode followed by name string) followed by
+ * the entries typecode and data. Named types allow for optional
+ * elements and extensions to be added and tested for without breaking
+ * backwards compatibility.
+ */
+
+enum aa_code {
+ AA_U8,
+ AA_U16,
+ AA_U32,
+ AA_U64,
+ AA_NAME, /* same as string except it is items name */
+ AA_STRING,
+ AA_BLOB,
+ AA_STRUCT,
+ AA_STRUCTEND,
+ AA_LIST,
+ AA_LISTEND,
+ AA_ARRAY,
+ AA_ARRAYEND,
+};
+
+/*
+ * aa_ext is the read of the buffer containing the serialized profile. The
+ * data is copied into a kernel buffer in apparmorfs and then handed off to
+ * the unpack routines.
+ */
+struct aa_ext {
+ void *start;
+ void *end;
+ void *pos; /* pointer to current position in the buffer */
+ u32 version;
+};
+
/*
* struct aa_loaddata - buffer of policy raw_data set
*
@@ -126,4 +163,17 @@ static inline void aa_put_loaddata(struct aa_loaddata *data)
kref_put(&data->count, aa_loaddata_kref);
}
+#if IS_ENABLED(CONFIG_KUNIT)
+bool inbounds(struct aa_ext *e, size_t size);
+size_t unpack_u16_chunk(struct aa_ext *e, char **chunk);
+bool unpack_X(struct aa_ext *e, enum aa_code code);
+bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
+bool unpack_u32(struct aa_ext *e, u32 *data, const char *name);
+bool unpack_u64(struct aa_ext *e, u64 *data, const char *name);
+size_t unpack_array(struct aa_ext *e, const char *name);
+size_t unpack_blob(struct aa_ext *e, char **blob, const char *name);
+int unpack_str(struct aa_ext *e, const char **string, const char *name);
+int unpack_strdup(struct aa_ext *e, char **string, const char *name);
+#endif
+
#endif /* __POLICY_INTERFACE_H */
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 55d31bac4f35..c23aa70349aa 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -14,6 +14,7 @@
*/
#include <asm/unaligned.h>
+#include <kunit/visibility.h>
#include <linux/ctype.h>
#include <linux/errno.h>
#include <linux/zlib.h>
@@ -37,43 +38,6 @@
#define v7 7
#define v8 8 /* full network masking */
-/*
- * The AppArmor interface treats data as a type byte followed by the
- * actual data. The interface has the notion of a named entry
- * which has a name (AA_NAME typecode followed by name string) followed by
- * the entries typecode and data. Named types allow for optional
- * elements and extensions to be added and tested for without breaking
- * backwards compatibility.
- */
-
-enum aa_code {
- AA_U8,
- AA_U16,
- AA_U32,
- AA_U64,
- AA_NAME, /* same as string except it is items name */
- AA_STRING,
- AA_BLOB,
- AA_STRUCT,
- AA_STRUCTEND,
- AA_LIST,
- AA_LISTEND,
- AA_ARRAY,
- AA_ARRAYEND,
-};
-
-/*
- * aa_ext is the read of the buffer containing the serialized profile. The
- * data is copied into a kernel buffer in apparmorfs and then handed off to
- * the unpack routines.
- */
-struct aa_ext {
- void *start;
- void *end;
- void *pos; /* pointer to current position in the buffer */
- u32 version;
-};
-
/* audit callback for unpack fields */
static void audit_cb(struct audit_buffer *ab, void *va)
{
@@ -199,10 +163,11 @@ struct aa_loaddata *aa_loaddata_alloc(size_t size)
}
/* test if read will be in packed data bounds */
-static bool inbounds(struct aa_ext *e, size_t size)
+VISIBLE_IF_KUNIT bool inbounds(struct aa_ext *e, size_t size)
{
return (size <= e->end - e->pos);
}
+EXPORT_SYMBOL_IF_KUNIT(inbounds);
static void *kvmemdup(const void *src, size_t len)
{
@@ -220,7 +185,7 @@ static void *kvmemdup(const void *src, size_t len)
*
* Returns: the size of chunk found with the read head at the end of the chunk.
*/
-static size_t unpack_u16_chunk(struct aa_ext *e, char **chunk)
+VISIBLE_IF_KUNIT size_t unpack_u16_chunk(struct aa_ext *e, char **chunk)
{
size_t size = 0;
void *pos = e->pos;
@@ -239,9 +204,10 @@ static size_t unpack_u16_chunk(struct aa_ext *e, char **chunk)
e->pos = pos;
return 0;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_u16_chunk);
/* unpack control byte */
-static bool unpack_X(struct aa_ext *e, enum aa_code code)
+VISIBLE_IF_KUNIT bool unpack_X(struct aa_ext *e, enum aa_code code)
{
if (!inbounds(e, 1))
return false;
@@ -250,6 +216,7 @@ static bool unpack_X(struct aa_ext *e, enum aa_code code)
e->pos++;
return true;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_X);
/**
* unpack_nameX - check is the next element is of type X with a name of @name
@@ -267,7 +234,7 @@ static bool unpack_X(struct aa_ext *e, enum aa_code code)
*
* Returns: false if either match fails, the read head does not move
*/
-static bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
+VISIBLE_IF_KUNIT bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
{
/*
* May need to reset pos if name or type doesn't match
@@ -296,6 +263,7 @@ static bool unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name)
e->pos = pos;
return false;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_nameX);
static bool unpack_u8(struct aa_ext *e, u8 *data, const char *name)
{
@@ -315,7 +283,7 @@ static bool unpack_u8(struct aa_ext *e, u8 *data, const char *name)
return false;
}
-static bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
+VISIBLE_IF_KUNIT bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
{
void *pos = e->pos;
@@ -332,8 +300,9 @@ static bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
e->pos = pos;
return false;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_u32);
-static bool unpack_u64(struct aa_ext *e, u64 *data, const char *name)
+VISIBLE_IF_KUNIT bool unpack_u64(struct aa_ext *e, u64 *data, const char *name)
{
void *pos = e->pos;
@@ -350,8 +319,9 @@ static bool unpack_u64(struct aa_ext *e, u64 *data, const char *name)
e->pos = pos;
return false;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_u64);
-static size_t unpack_array(struct aa_ext *e, const char *name)
+VISIBLE_IF_KUNIT size_t unpack_array(struct aa_ext *e, const char *name)
{
void *pos = e->pos;
@@ -368,8 +338,9 @@ static size_t unpack_array(struct aa_ext *e, const char *name)
e->pos = pos;
return 0;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_array);
-static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
+VISIBLE_IF_KUNIT size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
{
void *pos = e->pos;
@@ -390,8 +361,9 @@ static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
e->pos = pos;
return 0;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_blob);
-static int unpack_str(struct aa_ext *e, const char **string, const char *name)
+VISIBLE_IF_KUNIT int unpack_str(struct aa_ext *e, const char **string, const char *name)
{
char *src_str;
size_t size = 0;
@@ -413,8 +385,9 @@ static int unpack_str(struct aa_ext *e, const char **string, const char *name)
e->pos = pos;
return 0;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_str);
-static int unpack_strdup(struct aa_ext *e, char **string, const char *name)
+VISIBLE_IF_KUNIT int unpack_strdup(struct aa_ext *e, char **string, const char *name)
{
const char *tmp;
void *pos = e->pos;
@@ -432,6 +405,7 @@ static int unpack_strdup(struct aa_ext *e, char **string, const char *name)
return res;
}
+EXPORT_SYMBOL_IF_KUNIT(unpack_strdup);
/**
@@ -1251,7 +1225,3 @@ 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 */
diff --git a/security/apparmor/policy_unpack_test.c b/security/apparmor/policy_unpack_test.c
index 0a969b2e03db..3474fe2cd922 100644
--- a/security/apparmor/policy_unpack_test.c
+++ b/security/apparmor/policy_unpack_test.c
@@ -4,6 +4,7 @@
*/
#include <kunit/test.h>
+#include <kunit/visibility.h>
#include "include/policy.h"
#include "include/policy_unpack.h"
@@ -43,6 +44,8 @@
#define TEST_ARRAY_BUF_OFFSET \
(TEST_NAMED_ARRAY_BUF_OFFSET + 3 + strlen(TEST_ARRAY_NAME) + 1)
+MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
+
struct policy_unpack_fixture {
struct aa_ext *e;
size_t e_size;
@@ -605,3 +608,5 @@ static struct kunit_suite apparmor_policy_unpack_test_module = {
};
kunit_test_suite(apparmor_policy_unpack_test_module);
+
+MODULE_LICENSE("GPL");
--
2.38.1.273.g43a17bfeac-goog
next prev parent reply other threads:[~2022-11-02 18:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-02 17:59 [PATCH v1 0/2] kunit: add macro to allow conditionally exposing static symbols to tests Rae Moar
2022-11-02 17:59 ` [PATCH v1 1/2] " Rae Moar
2022-11-22 6:13 ` John Johansen
2022-11-23 9:20 ` David Gow
2022-11-02 17:59 ` Rae Moar [this message]
2022-11-22 6:20 ` [PATCH v1 2/2] apparmor: test: make static symbols visible during kunit testing John Johansen
2022-11-23 9:19 ` David Gow
2022-11-24 4:49 ` John Johansen
2022-12-05 17:23 ` Rae Moar
2022-11-22 6:13 ` [PATCH v1 0/2] kunit: add macro to allow conditionally exposing static symbols to tests John Johansen
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=20221102175959.2921063-3-rmoar@google.com \
--to=rmoar@google.com \
--cc=apparmor@lists.ubuntu.com \
--cc=brendanhiggins@google.com \
--cc=davidgow@google.com \
--cc=dlatypov@google.com \
--cc=john.johansen@canonical.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=tales.aparecida@gmail.com \
/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