LinuxLists.cc - linux-next: manual merge of the kunit-next tree with the apparmor tree

2022-12-08 04:41:53

Subject: linux-next: manual merge of the kunit-next tree with the apparmor tree

Hi all,

Today's linux-next merge of the kunit-next tree got a conflict in:

security/apparmor/policy_unpack.c

between commits:

371e50a0b19f ("apparmor: make unpack_array return a trianary value")
73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
217af7e2f4de ("apparmor: refactor profile rules and attachments")
(and probably others)

from the apparmor tree and commit:

2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")

from the kunit-next tree.

This is somewhat of a mess ... pity there is not a shared branch (or
better routing if the patches).

I fixed it up (hopefully - see below) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

I also had to add this patch:

From: Stephen Rothwell <[email protected]>
Date: Thu, 8 Dec 2022 13:47:43 +1100
Subject: [PATCH] fixup for "apparmor: make unpack_array return a trianary value"

Signed-off-by: Stephen Rothwell <[email protected]>
---
security/apparmor/include/policy_unpack.h | 8 +++++++-
security/apparmor/policy_unpack.c | 5 -----
2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
index 940da8a33e0c..8fdf8f703bd0 100644
--- a/security/apparmor/include/policy_unpack.h
+++ b/security/apparmor/include/policy_unpack.h
@@ -172,7 +172,13 @@ bool aa_unpack_X(struct aa_ext *e, enum aa_code code);
bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
-size_t aa_unpack_array(struct aa_ext *e, const char *name);
+
+#define tri int
+#define TRI_TRUE 1
+#define TRI_NONE 0
+#define TRI_FALSE -1
+
+tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name);
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 6513545dad5e..173d832fc4ee 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -30,11 +30,6 @@
#include "include/policy_unpack.h"
#include "include/policy_compat.h"

-#define tri int
-#define TRI_TRUE 1
-#define TRI_NONE 0
-#define TRI_FALSE -1
-
/* audit callback for unpack fields */
static void audit_cb(struct audit_buffer *ab, void *va)
{
--
2.35.1

--
Cheers,
Stephen Rothwell

diff --cc security/apparmor/policy_unpack.c
index 1bf8cfb8700a,12e535fdfa8b..000000000000
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@@ -14,9 -14,10 +14,10 @@@
*/

#include <asm/unaligned.h>
+ #include <kunit/visibility.h>
#include <linux/ctype.h>
#include <linux/errno.h>
-#include <linux/zlib.h>
+#include <linux/zstd.h>

#include "include/apparmor.h"
#include "include/audit.h"
@@@ -27,50 -27,16 +28,12 @@@
#include "include/path.h"
#include "include/policy.h"
#include "include/policy_unpack.h"
+#include "include/policy_compat.h"

-
- /*
- * 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;
- };
-#define K_ABI_MASK 0x3ff
-#define FORCE_COMPLAIN_FLAG 0x800
-#define VERSION_LT(X, Y) (((X) & K_ABI_MASK) < ((Y) & K_ABI_MASK))
-#define VERSION_GT(X, Y) (((X) & K_ABI_MASK) > ((Y) & K_ABI_MASK))
--
-#define v5 5 /* base version */
-#define v6 6 /* per entry policydb mediation check */
-#define v7 7
-#define v8 8 /* full network masking */
+#define tri int
+#define TRI_TRUE 1
+#define TRI_NONE 0
+#define TRI_FALSE -1

/* audit callback for unpack fields */
static void audit_cb(struct audit_buffer *ab, void *va)
@@@ -348,26 -319,28 +316,28 @@@ fail
e->pos = pos;
return false;
}
+ EXPORT_SYMBOL_IF_KUNIT(aa_unpack_u64);

- static tri unpack_array(struct aa_ext *e, const char *name, u16 *size)
-VISIBLE_IF_KUNIT size_t aa_unpack_array(struct aa_ext *e, const char *name)
++VISIBLE_IF_KUNIT tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size)
{
void *pos = e->pos;

- if (unpack_nameX(e, AA_ARRAY, name)) {
- if (!inbounds(e, sizeof(u16)))
+ if (aa_unpack_nameX(e, AA_ARRAY, name)) {
- int size;
+ if (!aa_inbounds(e, sizeof(u16)))
goto fail;
- size = (int)le16_to_cpu(get_unaligned((__le16 *) e->pos));
+ *size = le16_to_cpu(get_unaligned((__le16 *) e->pos));
e->pos += sizeof(u16);
- return size;
+ return TRI_TRUE;
}

+ return TRI_NONE;
fail:
e->pos = pos;
- return 0;
+ return TRI_FALSE;
}
+ EXPORT_SYMBOL_IF_KUNIT(aa_unpack_array);

- static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
+ VISIBLE_IF_KUNIT size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name)
{
void *pos = e->pos;

@@@ -470,36 -447,32 +443,36 @@@ static struct aa_dfa *unpack_dfa(struc
/**
* unpack_trans_table - unpack a profile transition table
* @e: serialized data extent information (NOT NULL)
- * @profile: profile to add the accept table to (NOT NULL)
+ * @table: str table to unpack to (NOT NULL)
*
- * Returns: true if table successfully unpacked
+ * Returns: true if table successfully unpacked or not present
*/
-static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile)
+static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
{
void *saved_pos = e->pos;
+ char **table = NULL;

/* exec table is optional */
- if (unpack_nameX(e, AA_STRUCT, "xtable")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "xtable")) {
- int i, size;
-
- size = aa_unpack_array(e, NULL);
- /* currently 4 exec bits and entries 0-3 are reserved iupcx */
- if (size > 16 - 4)
+ u16 size;
+ int i;
+
- if (unpack_array(e, NULL, &size) != TRI_TRUE)
++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
+ /*
+ * Note: index into trans table array is a max
+ * of 2^24, but unpack array can only unpack
+ * an array of 2^16 in size atm so no need
+ * for size check here
+ */
goto fail;
- profile->file.trans.table = kcalloc(size, sizeof(char *),
- GFP_KERNEL);
- if (!profile->file.trans.table)
+ table = kcalloc(size, sizeof(char *), GFP_KERNEL);
+ if (!table)
goto fail;

- profile->file.trans.size = size;
for (i = 0; i < size; i++) {
char *str;
- int c, j, pos, size2 = unpack_strdup(e, &str, NULL);
- /* unpack_strdup verifies that the last character is
+ int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL);
+ /* aa_unpack_strdup verifies that the last character is
* null termination byte.
*/
if (!size2)
@@@ -534,13 -507,10 +507,13 @@@
/* fail - all other cases with embedded \0 */
goto fail;
}
- if (!unpack_nameX(e, AA_ARRAYEND, NULL))
+ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
+
+ strs->table = table;
+ strs->size = size;
}
return true;

@@@ -554,23 -524,21 +527,23 @@@ static bool unpack_xattrs(struct aa_ex
{
void *pos = e->pos;

- if (unpack_nameX(e, AA_STRUCT, "xattrs")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "xattrs")) {
- int i, size;
+ u16 size;
+ int i;

- if (unpack_array(e, NULL, &size) != TRI_TRUE)
- size = aa_unpack_array(e, NULL);
- profile->xattr_count = size;
- profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
- if (!profile->xattrs)
++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
+ goto fail;
+ profile->attach.xattr_count = size;
+ profile->attach.xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
+ if (!profile->attach.xattrs)
goto fail;
for (i = 0; i < size; i++) {
- if (!unpack_strdup(e, &profile->attach.xattrs[i], NULL))
- if (!aa_unpack_strdup(e, &profile->xattrs[i], NULL))
++ if (!aa_unpack_strdup(e, &profile->attach.xattrs[i], NULL))
goto fail;
}
- if (!unpack_nameX(e, AA_ARRAYEND, NULL))
+ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
}

@@@ -581,34 -549,32 +554,34 @@@ fail
return false;
}

-static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile)
+static bool unpack_secmark(struct aa_ext *e, struct aa_ruleset *rules)
{
void *pos = e->pos;
- int i, size;
+ u16 size;
+ int i;

- if (unpack_nameX(e, AA_STRUCT, "secmark")) {
- if (unpack_array(e, NULL, &size) != TRI_TRUE)
+ if (aa_unpack_nameX(e, AA_STRUCT, "secmark")) {
- size = aa_unpack_array(e, NULL);
++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
+ goto fail;

- profile->secmark = kcalloc(size, sizeof(struct aa_secmark),
+ rules->secmark = kcalloc(size, sizeof(struct aa_secmark),
GFP_KERNEL);
- if (!profile->secmark)
+ if (!rules->secmark)
goto fail;

- profile->secmark_count = size;
+ rules->secmark_count = size;

for (i = 0; i < size; i++) {
- if (!unpack_u8(e, &profile->secmark[i].audit, NULL))
+ if (!unpack_u8(e, &rules->secmark[i].audit, NULL))
goto fail;
- if (!unpack_u8(e, &profile->secmark[i].deny, NULL))
+ if (!unpack_u8(e, &rules->secmark[i].deny, NULL))
goto fail;
- if (!unpack_strdup(e, &rules->secmark[i].label, NULL))
- if (!aa_unpack_strdup(e, &profile->secmark[i].label, NULL))
++ if (!aa_unpack_strdup(e, &rules->secmark[i].label, NULL))
goto fail;
}
- if (!unpack_nameX(e, AA_ARRAYEND, NULL))
+ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
}

@@@ -632,27 -598,26 +605,27 @@@ static bool unpack_rlimits(struct aa_ex
void *pos = e->pos;

/* rlimits are optional */
- if (unpack_nameX(e, AA_STRUCT, "rlimits")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "rlimits")) {
- int i, size;
+ u16 size;
+ int i;
u32 tmp = 0;
- if (!unpack_u32(e, &tmp, NULL))
+ if (!aa_unpack_u32(e, &tmp, NULL))
goto fail;
- profile->rlimits.mask = tmp;
+ rules->rlimits.mask = tmp;

- if (unpack_array(e, NULL, &size) != TRI_TRUE ||
- size = aa_unpack_array(e, NULL);
- if (size > RLIM_NLIMITS)
++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE ||
+ size > RLIM_NLIMITS)
goto fail;
for (i = 0; i < size; i++) {
u64 tmp2 = 0;
int a = aa_map_resource(i);
- if (!unpack_u64(e, &tmp2, NULL))
+ if (!aa_unpack_u64(e, &tmp2, NULL))
goto fail;
- profile->rlimits.limits[a].rlim_max = tmp2;
+ rules->rlimits.limits[a].rlim_max = tmp2;
}
- if (!unpack_nameX(e, AA_ARRAYEND, NULL))
+ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
}
return true;
@@@ -662,144 -627,6 +635,144 @@@ fail
return false;
}

+static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm)
+{
+ bool res;
+
+ if (version != 1)
+ return false;
+
- res = unpack_u32(e, &perm->allow, NULL);
- res = res && unpack_u32(e, &perm->allow, NULL);
- res = res && unpack_u32(e, &perm->deny, NULL);
- res = res && unpack_u32(e, &perm->subtree, NULL);
- res = res && unpack_u32(e, &perm->cond, NULL);
- res = res && unpack_u32(e, &perm->kill, NULL);
- res = res && unpack_u32(e, &perm->complain, NULL);
- res = res && unpack_u32(e, &perm->prompt, NULL);
- res = res && unpack_u32(e, &perm->audit, NULL);
- res = res && unpack_u32(e, &perm->quiet, NULL);
- res = res && unpack_u32(e, &perm->hide, NULL);
- res = res && unpack_u32(e, &perm->xindex, NULL);
- res = res && unpack_u32(e, &perm->tag, NULL);
- res = res && unpack_u32(e, &perm->label, NULL);
++ res = aa_unpack_u32(e, &perm->allow, NULL);
++ res = res && aa_unpack_u32(e, &perm->allow, NULL);
++ res = res && aa_unpack_u32(e, &perm->deny, NULL);
++ res = res && aa_unpack_u32(e, &perm->subtree, NULL);
++ res = res && aa_unpack_u32(e, &perm->cond, NULL);
++ res = res && aa_unpack_u32(e, &perm->kill, NULL);
++ res = res && aa_unpack_u32(e, &perm->complain, NULL);
++ res = res && aa_unpack_u32(e, &perm->prompt, NULL);
++ res = res && aa_unpack_u32(e, &perm->audit, NULL);
++ res = res && aa_unpack_u32(e, &perm->quiet, NULL);
++ res = res && aa_unpack_u32(e, &perm->hide, NULL);
++ res = res && aa_unpack_u32(e, &perm->xindex, NULL);
++ res = res && aa_unpack_u32(e, &perm->tag, NULL);
++ res = res && aa_unpack_u32(e, &perm->label, NULL);
+
+ return res;
+}
+
+static ssize_t unpack_perms_table(struct aa_ext *e, struct aa_perms **perms)
+{
+ void *pos = e->pos;
+ u16 size = 0;
+
+ AA_BUG(!perms);
+ /*
+ * policy perms are optional, in which case perms are embedded
+ * in the dfa accept table
+ */
- if (unpack_nameX(e, AA_STRUCT, "perms")) {
++ if (aa_unpack_nameX(e, AA_STRUCT, "perms")) {
+ int i;
+ u32 version;
+
- if (!unpack_u32(e, &version, "version"))
++ if (!aa_unpack_u32(e, &version, "version"))
+ goto fail_reset;
- if (unpack_array(e, NULL, &size) != TRI_TRUE)
++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
+ goto fail_reset;
+ *perms = kcalloc(size, sizeof(struct aa_perms), GFP_KERNEL);
+ if (!*perms)
+ goto fail_reset;
+ for (i = 0; i < size; i++) {
+ if (!unpack_perm(e, version, &(*perms)[i]))
+ goto fail;
+ }
- if (!unpack_nameX(e, AA_ARRAYEND, NULL))
++ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
+ goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
++ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
+ goto fail;
+ } else
+ *perms = NULL;
+
+ return size;
+
+fail:
+ kfree(*perms);
+fail_reset:
+ e->pos = pos;
+ return -EPROTO;
+}
+
+static int unpack_pdb(struct aa_ext *e, struct aa_policydb *policy,
+ bool required_dfa, bool required_trans,
+ const char **info)
+{
+ void *pos = e->pos;
+ int i, flags, error = -EPROTO;
+ ssize_t size;
+
+ size = unpack_perms_table(e, &policy->perms);
+ if (size < 0) {
+ error = size;
+ policy->perms = NULL;
+ *info = "failed to unpack - perms";
+ goto fail;
+ }
+ policy->size = size;
+
+ if (policy->perms) {
+ /* perms table present accept is index */
+ flags = TO_ACCEPT1_FLAG(YYTD_DATA32);
+ } else {
+ /* packed perms in accept1 and accept2 */
+ flags = TO_ACCEPT1_FLAG(YYTD_DATA32) |
+ TO_ACCEPT2_FLAG(YYTD_DATA32);
+ }
+
+ policy->dfa = unpack_dfa(e, flags);
+ if (IS_ERR(policy->dfa)) {
+ error = PTR_ERR(policy->dfa);
+ policy->dfa = NULL;
+ *info = "failed to unpack - dfa";
+ goto fail;
+ } else if (!policy->dfa) {
+ if (required_dfa) {
+ *info = "missing required dfa";
+ goto fail;
+ }
+ goto out;
+ }
+
+ /*
+ * only unpack the following if a dfa is present
+ *
+ * sadly start was given different names for file and policydb
+ * but since it is optional we can try both
+ */
- if (!unpack_u32(e, &policy->start[0], "start"))
++ if (!aa_unpack_u32(e, &policy->start[0], "start"))
+ /* default start state */
+ policy->start[0] = DFA_START;
- if (!unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
++ if (!aa_unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
+ /* default start state for xmatch and file dfa */
+ policy->start[AA_CLASS_FILE] = DFA_START;
+ } /* setup class index */
+ for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
+ policy->start[i] = aa_dfa_next(policy->dfa, policy->start[0],
+ i);
+ }
+ if (!unpack_trans_table(e, &policy->trans) && required_trans) {
+ *info = "failed to unpack profile transition table";
+ goto fail;
+ }
+
+ /* TODO: move compat mapping here, requires dfa merging first */
+ /* TODO: move verify here, it has to be done after compat mappings */
+out:
+ return 0;
+
+fail:
+ e->pos = pos;
+ return error;
+}
+
static u32 strhash(const void *data, u32 len, u32 seed)
{
const char * const *key = data;
@@@ -858,29 -683,26 +831,29 @@@ static struct aa_profile *unpack_profil
}

profile = aa_alloc_profile(name, NULL, GFP_KERNEL);
- if (!profile)
- return ERR_PTR(-ENOMEM);
+ if (!profile) {
+ info = "out of memory";
+ error = -ENOMEM;
+ goto fail;
+ }
+ rules = list_first_entry(&profile->rules, typeof(*rules), list);

/* profile renaming is optional */
- (void) unpack_str(e, &profile->rename, "rename");
+ (void) aa_unpack_str(e, &profile->rename, "rename");

/* attachment string is optional */
- (void) unpack_str(e, &profile->attach.xmatch_str, "attach");
- (void) aa_unpack_str(e, &profile->attach, "attach");
++ (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach");

/* xmatch is optional and may be NULL */
- profile->xmatch = unpack_dfa(e);
- if (IS_ERR(profile->xmatch)) {
- error = PTR_ERR(profile->xmatch);
- profile->xmatch = NULL;
+ error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info);
+ if (error) {
info = "bad xmatch";
goto fail;
}
- /* xmatch_len is not optional if xmatch is set */
- if (profile->xmatch) {
+
+ /* neither xmatch_len not xmatch_perms are optional if xmatch is set */
+ if (profile->attach.xmatch.dfa) {
- if (!unpack_u32(e, &tmp, NULL)) {
+ if (!aa_unpack_u32(e, &tmp, NULL)) {
info = "missing xmatch len";
goto fail;
}
@@@ -943,38 -757,38 +916,38 @@@
profile->path_flags = PATH_MEDIATE_DELETED;

info = "failed to unpack profile capabilities";
- if (!unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.allow.cap[0]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
goto fail;
- if (!unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.audit.cap[0]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
goto fail;
- if (!unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[0]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
goto fail;
- if (!unpack_u32(e, &tmpcap.cap[0], NULL))
+ if (!aa_unpack_u32(e, &tmpcap.cap[0], NULL))
goto fail;

info = "failed to unpack upper profile capabilities";
- if (unpack_nameX(e, AA_STRUCT, "caps64")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "caps64")) {
/* optional upper half of 64 bit caps */
- if (!unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.allow.cap[1]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
goto fail;
- if (!unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.audit.cap[1]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
goto fail;
- if (!unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[1]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
goto fail;
- if (!unpack_u32(e, &(tmpcap.cap[1]), NULL))
+ if (!aa_unpack_u32(e, &(tmpcap.cap[1]), NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
}

info = "failed to unpack extended profile capabilities";
- if (unpack_nameX(e, AA_STRUCT, "capsx")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "capsx")) {
/* optional extended caps mediation mask */
- if (!unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.extended.cap[0]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
goto fail;
- if (!unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
- if (!aa_unpack_u32(e, &(profile->caps.extended.cap[1]), NULL))
++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
goto fail;
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
}

@@@ -993,55 -807,62 +966,55 @@@
goto fail;
}

- if (unpack_nameX(e, AA_STRUCT, "policydb")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "policydb")) {
/* generic policy dfa - optional and may be NULL */
info = "failed to unpack policydb";
- profile->policy.dfa = unpack_dfa(e);
- if (IS_ERR(profile->policy.dfa)) {
- error = PTR_ERR(profile->policy.dfa);
- profile->policy.dfa = NULL;
- goto fail;
- } else if (!profile->policy.dfa) {
- error = -EPROTO;
+ error = unpack_pdb(e, &rules->policy, true, false,
+ &info);
+ if (error)
goto fail;
- }
- if (!aa_unpack_u32(e, &profile->policy.start[0], "start"))
- /* default start state */
- profile->policy.start[0] = DFA_START;
- /* setup class index */
- for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) {
- profile->policy.start[i] =
- aa_dfa_next(profile->policy.dfa,
- profile->policy.start[0],
- i);
- }
+ /* Fixup: drop when we get rid of start array */
+ if (aa_dfa_next(rules->policy.dfa, rules->policy.start[0],
+ AA_CLASS_FILE))
+ rules->policy.start[AA_CLASS_FILE] =
+ aa_dfa_next(rules->policy.dfa,
+ rules->policy.start[0],
+ AA_CLASS_FILE);
- if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
+ error = aa_compat_map_policy(&rules->policy, e->version);
+ if (error) {
+ info = "failed to remap policydb permission table";
+ goto fail;
+ }
} else
- profile->policy.dfa = aa_get_dfa(nulldfa);
+ rules->policy.dfa = aa_get_dfa(nulldfa);

/* get file rules */
- profile->file.dfa = unpack_dfa(e);
- if (IS_ERR(profile->file.dfa)) {
- error = PTR_ERR(profile->file.dfa);
- profile->file.dfa = NULL;
- info = "failed to unpack profile file rules";
+ error = unpack_pdb(e, &rules->file, false, true, &info);
+ if (error) {
goto fail;
- } else if (profile->file.dfa) {
- if (!aa_unpack_u32(e, &profile->file.start, "dfa_start"))
- /* default start state */
- profile->file.start = DFA_START;
- } else if (profile->policy.dfa &&
- profile->policy.start[AA_CLASS_FILE]) {
- profile->file.dfa = aa_get_dfa(profile->policy.dfa);
- profile->file.start = profile->policy.start[AA_CLASS_FILE];
+ } else if (rules->file.dfa) {
+ error = aa_compat_map_file(&rules->file);
+ if (error) {
+ info = "failed to remap file permission table";
+ goto fail;
+ }
+ } else if (rules->policy.dfa &&
+ rules->policy.start[AA_CLASS_FILE]) {
+ rules->file.dfa = aa_get_dfa(rules->policy.dfa);
+ rules->file.start[AA_CLASS_FILE] = rules->policy.start[AA_CLASS_FILE];
} else
- profile->file.dfa = aa_get_dfa(nulldfa);
-
- if (!unpack_trans_table(e, profile)) {
- info = "failed to unpack profile transition table";
- goto fail;
- }
+ rules->file.dfa = aa_get_dfa(nulldfa);

+ error = -EPROTO;
- if (unpack_nameX(e, AA_STRUCT, "data")) {
+ if (aa_unpack_nameX(e, AA_STRUCT, "data")) {
info = "out of memory";
profile->data = kzalloc(sizeof(*profile->data), GFP_KERNEL);
- if (!profile->data)
+ if (!profile->data) {
+ error = -ENOMEM;
goto fail;
-
+ }
params.nelem_hint = 3;
params.key_len = sizeof(void *);
params.key_offset = offsetof(struct aa_data, key);

Attachments:

(No filename) (499.00 B)
OpenPGP digital signature

2022-12-08 20:59:26

by John Johansen

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/7/22 18:53, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the kunit-next tree got a conflict in:
>
> security/apparmor/policy_unpack.c
>
> between commits:
>
> 371e50a0b19f ("apparmor: make unpack_array return a trianary value")
> 73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
> 217af7e2f4de ("apparmor: refactor profile rules and attachments")
> (and probably others)
>
> from the apparmor tree and commit:
>
> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>
> from the kunit-next tree.
>
> This is somewhat of a mess ... pity there is not a shared branch (or
> better routing if the patches).
>
sorry, there was a miscommunication/misunderstanding, probably all on me, I
thought the kunit stuff that is conflicting here was going to merge next
cycle.

> I fixed it up (hopefully - see below) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging. You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
>
> I also had to add this patch:
>
this needs to be modified to build if kunit is not enabled, basically
the defines need to move up outside the #if IS_ENABLED(CONFIG_KUNIT)

ie.
diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
index 8fdf8f703bd0..dcf7d1bbf96b 100644
--- a/security/apparmor/include/policy_unpack.h
+++ b/security/apparmor/include/policy_unpack.h
@@ -165,6 +165,11 @@ static inline void aa_put_loaddata(struct aa_loaddata *data)
kref_put(&data->count, aa_loaddata_kref);
}

+#define tri int
+#define TRI_TRUE 1
+#define TRI_NONE 0
+#define TRI_FALSE -1
+
#if IS_ENABLED(CONFIG_KUNIT)
bool aa_inbounds(struct aa_ext *e, size_t size);
size_t aa_unpack_u16_chunk(struct aa_ext *e, char **chunk);
@@ -173,11 +178,6 @@ bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);

-#define tri int
-#define TRI_TRUE 1
-#define TRI_NONE 0
-#define TRI_FALSE -1
-
tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);

feel free to apply that to your patch and then add my
Acked-by: John Johansen <[email protected]>

> From: Stephen Rothwell <[email protected]>
> Date: Thu, 8 Dec 2022 13:47:43 +1100
> Subject: [PATCH] fixup for "apparmor: make unpack_array return a trianary value"
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> security/apparmor/include/policy_unpack.h | 8 +++++++-
> security/apparmor/policy_unpack.c | 5 -----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
> index 940da8a33e0c..8fdf8f703bd0 100644
> --- a/security/apparmor/include/policy_unpack.h
> +++ b/security/apparmor/include/policy_unpack.h
> @@ -172,7 +172,13 @@ bool aa_unpack_X(struct aa_ext *e, enum aa_code code);
> bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
> bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
> bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
> -size_t aa_unpack_array(struct aa_ext *e, const char *name);
> +
> +#define tri int
> +#define TRI_TRUE 1
> +#define TRI_NONE 0
> +#define TRI_FALSE -1
> +
> +tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
> size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
> int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
> int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name);
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 6513545dad5e..173d832fc4ee 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -30,11 +30,6 @@
> #include "include/policy_unpack.h"
> #include "include/policy_compat.h"
>
> -#define tri int
> -#define TRI_TRUE 1
> -#define TRI_NONE 0
> -#define TRI_FALSE -1
> -
> /* audit callback for unpack fields */
> static void audit_cb(struct audit_buffer *ab, void *va)
> {

2022-12-12 18:18:22

by Shuah Khan

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

Hi David,

On 12/8/22 13:10, John Johansen wrote:
> On 12/7/22 18:53, Stephen Rothwell wrote:
>> Hi all,
>>
>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>
>>    security/apparmor/policy_unpack.c
>>
>> between commits:
>>
>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>> (and probably others)
>>
>> from the apparmor tree and commit:
>>
>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>
>> from the kunit-next tree.
>>
>> This is somewhat of a mess ... pity there is not a shared branch (or
>> better routing if the patches).
>>
> sorry, there was a miscommunication/misunderstanding, probably all on me, I
> thought the kunit stuff that is conflicting here was going to merge next
> cycle.
>

Sorry for not noticing David isn't on the cc - added David now. What's the
best way to resolve this?

>> I fixed it up (hopefully - see below) and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging. You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.
>>
>> I also had to add this patch:
>>
> this needs to be modified to build if kunit is not enabled, basically
> the defines need to move up outside the #if IS_ENABLED(CONFIG_KUNIT)
>
> ie.
> diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
> index 8fdf8f703bd0..dcf7d1bbf96b 100644
> --- a/security/apparmor/include/policy_unpack.h
> +++ b/security/apparmor/include/policy_unpack.h
> @@ -165,6 +165,11 @@ static inline void aa_put_loaddata(struct aa_loaddata *data)
>          kref_put(&data->count, aa_loaddata_kref);
> }
>
> +#define tri int
> +#define TRI_TRUE 1
> +#define TRI_NONE 0
> +#define TRI_FALSE -1
> +
> #if IS_ENABLED(CONFIG_KUNIT)
> bool aa_inbounds(struct aa_ext *e, size_t size);
> size_t aa_unpack_u16_chunk(struct aa_ext *e, char **chunk);
> @@ -173,11 +178,6 @@ bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
> bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
> bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
>
> -#define tri int
> -#define TRI_TRUE 1
> -#define TRI_NONE 0
> -#define TRI_FALSE -1
> -
> tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
> size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
> int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
>
>
> feel free to apply that to your patch and then add my
> Acked-by: John Johansen <[email protected]>
>
>> From: Stephen Rothwell <[email protected]>
>> Date: Thu, 8 Dec 2022 13:47:43 +1100
>> Subject: [PATCH] fixup for "apparmor: make unpack_array return a trianary value"
>>
>> Signed-off-by: Stephen Rothwell <[email protected]>
>> ---
>> security/apparmor/include/policy_unpack.h | 8 +++++++-
>> security/apparmor/policy_unpack.c         | 5 -----
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
>> index 940da8a33e0c..8fdf8f703bd0 100644
>> --- a/security/apparmor/include/policy_unpack.h
>> +++ b/security/apparmor/include/policy_unpack.h
>> @@ -172,7 +172,13 @@ bool aa_unpack_X(struct aa_ext *e, enum aa_code code);
>> bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
>> bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
>> bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
>> -size_t aa_unpack_array(struct aa_ext *e, const char *name);
>> +
>> +#define tri int
>> +#define TRI_TRUE 1
>> +#define TRI_NONE 0
>> +#define TRI_FALSE -1
>> +
>> +tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
>> size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
>> int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
>> int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name);
>> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
>> index 6513545dad5e..173d832fc4ee 100644
>> --- a/security/apparmor/policy_unpack.c
>> +++ b/security/apparmor/policy_unpack.c
>> @@ -30,11 +30,6 @@
>> #include "include/policy_unpack.h"
>> #include "include/policy_compat.h"
>> -#define tri int
>> -#define TRI_TRUE 1
>> -#define TRI_NONE 0
>> -#define TRI_FALSE -1
>> -
>> /* audit callback for unpack fields */
>> static void audit_cb(struct audit_buffer *ab, void *va)
>> {
>

thanks,
-- Shuah

2022-12-12 18:47:00

by Shuah Khan

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/12/22 10:52, Shuah Khan wrote:
> Hi David,
>
> On 12/8/22 13:10, John Johansen wrote:
>> On 12/7/22 18:53, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>
>>>    security/apparmor/policy_unpack.c
>>>
>>> between commits:
>>>
>>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>>> (and probably others)
>>>
>>> from the apparmor tree and commit:
>>>
>>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>
>>> from the kunit-next tree.
>>>
>>> This is somewhat of a mess ... pity there is not a shared branch (or
>>> better routing if the patches).
>>>
>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
>> thought the kunit stuff that is conflicting here was going to merge next
>> cycle.
>>
>

How about I just drop the following for now and handle this in the next cycle?
I think it might be least confusing option. Let me know. I can just do that
and then send pull request in a day or tow once things settle down in next.

2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")

thanks,
-- Shuah

2022-12-12 19:48:48

by John Johansen

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/12/22 10:03, Shuah Khan wrote:
> On 12/12/22 10:52, Shuah Khan wrote:
>> Hi David,
>>
>> On 12/8/22 13:10, John Johansen wrote:
>>> On 12/7/22 18:53, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>>
>>>>    security/apparmor/policy_unpack.c
>>>>
>>>> between commits:
>>>>
>>>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>>>> (and probably others)
>>>>
>>>> from the apparmor tree and commit:
>>>>
>>>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>>
>>>> from the kunit-next tree.
>>>>
>>>> This is somewhat of a mess ... pity there is not a shared branch (or
>>>> better routing if the patches).
>>>>
>>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
>>> thought the kunit stuff that is conflicting here was going to merge next
>>> cycle.
>>>
>>
>
> How about I just drop the following for now and handle this in the next cycle?

if you want, the other way to handle it is we coordinate our pull requests.
You go first. And then I will submit a little later in the week, with the
references to the merge conflict and a pointer to a branch with it resolved.
This isn't even a particularly tricky merge conflict, it just has the little
subtly around making sure the include symbols are conditional.

This doesn't affect me much as there is already another merge conflict with
the security tree that I need to deal with.

> I think it might be least confusing option. Let me know. I can just do that
> and then send pull request in a day or tow once things settle down in next.
>
> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>

that is the other option. If you go that route I can help you do the rebase/merge
fix.

looking back at this, there wasn't anything explicit about this not going upstream
this cycle, I must have just assumed as the final version came about after rc7. So
my bad.

> thanks,
> -- Shuah

2022-12-12 20:34:23

by Shuah Khan

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/12/22 12:20, John Johansen wrote:
> On 12/12/22 10:03, Shuah Khan wrote:
>> On 12/12/22 10:52, Shuah Khan wrote:
>>> Hi David,
>>>
>>> On 12/8/22 13:10, John Johansen wrote:
>>>> On 12/7/22 18:53, Stephen Rothwell wrote:
>>>>> Hi all,
>>>>>
>>>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>>>
>>>>>    security/apparmor/policy_unpack.c
>>>>>
>>>>> between commits:
>>>>>
>>>>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>>>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>>>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>>>>> (and probably others)
>>>>>
>>>>> from the apparmor tree and commit:
>>>>>
>>>>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>>>
>>>>> from the kunit-next tree.
>>>>>
>>>>> This is somewhat of a mess ... pity there is not a shared branch (or
>>>>> better routing if the patches).
>>>>>
>>>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
>>>> thought the kunit stuff that is conflicting here was going to merge next
>>>> cycle.
>>>>
>>>
>>
>> How about I just drop the following for now and handle this in the next cycle?
>
> if you want, the other way to handle it is we coordinate our pull requests.
> You go first. And then I will submit a little later in the week, with the
> references to the merge conflict and a pointer to a branch with it resolved.
> This isn't even a particularly tricky merge conflict, it just has the little
> subtly around making sure the include symbols are conditional.
>

I assume Linus will not see any problems without your pull requests. In which
case we can do this:

- I send my pull request today
- You can follow with yours with the fixes later on this week

> This doesn't affect me much as there is already another merge conflict with
> the security tree that I need to deal with.
>

>> I think it might be least confusing option. Let me know. I can just do that
>> and then send pull request in a day or tow once things settle down in next.
>>
>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>
>
> that is the other option. If you go that route I can help you do the rebase/merge
> fix.
>

Let's go with your earlier suggestion.

> looking back at this, there wasn't anything explicit about this not going upstream
> this cycle, I must have just assumed as the final version came about after rc7. So
> my bad.
>

Right - I ended up taking this as it looked like a patch if included could
enable other changes to follow without being blocked. Also rc8 was in plan.

thanks,
-- Shuah

2022-12-12 21:01:01

by John Johansen

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/12/22 11:48, Shuah Khan wrote:
> On 12/12/22 12:20, John Johansen wrote:
>> On 12/12/22 10:03, Shuah Khan wrote:
>>> On 12/12/22 10:52, Shuah Khan wrote:
>>>> Hi David,
>>>>
>>>> On 12/8/22 13:10, John Johansen wrote:
>>>>> On 12/7/22 18:53, Stephen Rothwell wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>>>>
>>>>>>    security/apparmor/policy_unpack.c
>>>>>>
>>>>>> between commits:
>>>>>>
>>>>>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>>>>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>>>>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>>>>>> (and probably others)
>>>>>>
>>>>>> from the apparmor tree and commit:
>>>>>>
>>>>>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>>>>
>>>>>> from the kunit-next tree.
>>>>>>
>>>>>> This is somewhat of a mess ... pity there is not a shared branch (or
>>>>>> better routing if the patches).
>>>>>>
>>>>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
>>>>> thought the kunit stuff that is conflicting here was going to merge next
>>>>> cycle.
>>>>>
>>>>
>>>
>>> How about I just drop the following for now and handle this in the next cycle?
>>
>> if you want, the other way to handle it is we coordinate our pull requests.
>> You go first. And then I will submit a little later in the week, with the
>> references to the merge conflict and a pointer to a branch with it resolved.
>> This isn't even a particularly tricky merge conflict, it just has the little
>> subtly around making sure the include symbols are conditional.
>>
>
> I assume Linus will not see any problems without your pull requests. In which
> case we can do this:
>
> - I send my pull request today
> - You can follow with yours with the fixes later on this week
>

okay

>> This doesn't affect me much as there is already another merge conflict with
>> the security tree that I need to deal with.
>>
>
>
>>> I think it might be least confusing option. Let me know. I can just do that
>>> and then send pull request in a day or tow once things settle down in next.
>>>
>>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>
>>
>> that is the other option. If you go that route I can help you do the rebase/merge
>> fix.
>>
>
> Let's go with your earlier suggestion.
>

ack

>> looking back at this, there wasn't anything explicit about this not going upstream
>> this cycle, I must have just assumed as the final version came about after rc7. So
>> my bad.
>>
>
> Right - I ended up taking this as it looked like a patch if included could
> enable other changes to follow without being blocked. Also rc8 was in plan.
>

yeah, my bad

> thanks,
> -- Shuah

2022-12-12 23:47:03

by Shuah Khan

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/12/22 12:53, John Johansen wrote:
> On 12/12/22 11:48, Shuah Khan wrote:
>> On 12/12/22 12:20, John Johansen wrote:
>>> On 12/12/22 10:03, Shuah Khan wrote:
>>>> On 12/12/22 10:52, Shuah Khan wrote:
>>>>> Hi David,
>>>>>
>>>>> On 12/8/22 13:10, John Johansen wrote:
>>>>>> On 12/7/22 18:53, Stephen Rothwell wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>>>>>
>>>>>>>    security/apparmor/policy_unpack.c
>>>>>>>
>>>>>>> between commits:
>>>>>>>
>>>>>>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>>>>>>    73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>>>>>>>    217af7e2f4de ("apparmor: refactor profile rules and attachments")
>>>>>>> (and probably others)
>>>>>>>
>>>>>>> from the apparmor tree and commit:
>>>>>>>
>>>>>>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>>>>>
>>>>>>> from the kunit-next tree.
>>>>>>>
>>>>>>> This is somewhat of a mess ... pity there is not a shared branch (or
>>>>>>> better routing if the patches).
>>>>>>>
>>>>>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
>>>>>> thought the kunit stuff that is conflicting here was going to merge next
>>>>>> cycle.
>>>>>>
>>>>>
>>>>
>>>> How about I just drop the following for now and handle this in the next cycle?
>>>
>>> if you want, the other way to handle it is we coordinate our pull requests.
>>> You go first. And then I will submit a little later in the week, with the
>>> references to the merge conflict and a pointer to a branch with it resolved.
>>> This isn't even a particularly tricky merge conflict, it just has the little
>>> subtly around making sure the include symbols are conditional.
>>>
>>
>> I assume Linus will not see any problems without your pull requests. In which
>> case we can do this:
>>
>> - I send my pull request today
>> - You can follow with yours with the fixes later on this week
>>
>
> okay
>
>>> This doesn't affect me much as there is already another merge conflict with
>>> the security tree that I need to deal with.
>>>
>>
>>
>>>> I think it might be least confusing option. Let me know. I can just do that
>>>> and then send pull request in a day or tow once things settle down in next.
>>>>
>>>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>>>
>>>
>>> that is the other option. If you go that route I can help you do the rebase/merge
>>> fix.
>>>
>>
>> Let's go with your earlier suggestion.
>>
>
> ack
>
>>> looking back at this, there wasn't anything explicit about this not going upstream
>>> this cycle, I must have just assumed as the final version came about after rc7. So
>>> my bad.
>>>
>>
>> Right - I ended up taking this as it looked like a patch if included could
>> enable other changes to follow without being blocked. Also rc8 was in plan.
>>
>
> yeah, my bad
>

No worries. Sent pull request with a note about apparmor and our
coordinated pull requests with you on the cc.

thanks,
-- Shuah

2022-12-13 00:18:28

by David Gow

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On Tue, 13 Dec 2022 at 07:19, Shuah Khan <[email protected]> wrote:
>
> On 12/12/22 12:53, John Johansen wrote:
> > On 12/12/22 11:48, Shuah Khan wrote:
> >> On 12/12/22 12:20, John Johansen wrote:
> >>> On 12/12/22 10:03, Shuah Khan wrote:
> >>>> On 12/12/22 10:52, Shuah Khan wrote:
> >>>>> Hi David,
> >>>>>
> >>>>> On 12/8/22 13:10, John Johansen wrote:
> >>>>>> On 12/7/22 18:53, Stephen Rothwell wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Today's linux-next merge of the kunit-next tree got a conflict in:
> >>>>>>>
> >>>>>>> security/apparmor/policy_unpack.c
> >>>>>>>
> >>>>>>> between commits:
> >>>>>>>
> >>>>>>> 371e50a0b19f ("apparmor: make unpack_array return a trianary value")
> >>>>>>> 73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
> >>>>>>> 217af7e2f4de ("apparmor: refactor profile rules and attachments")
> >>>>>>> (and probably others)
> >>>>>>>
> >>>>>>> from the apparmor tree and commit:
> >>>>>>>
> >>>>>>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
> >>>>>>>
> >>>>>>> from the kunit-next tree.
> >>>>>>>
> >>>>>>> This is somewhat of a mess ... pity there is not a shared branch (or
> >>>>>>> better routing if the patches).
> >>>>>>>
> >>>>>> sorry, there was a miscommunication/misunderstanding, probably all on me, I
> >>>>>> thought the kunit stuff that is conflicting here was going to merge next
> >>>>>> cycle.
> >>>>>>
> >>>>>
> >>>>
> >>>> How about I just drop the following for now and handle this in the next cycle?
> >>>
> >>> if you want, the other way to handle it is we coordinate our pull requests.
> >>> You go first. And then I will submit a little later in the week, with the
> >>> references to the merge conflict and a pointer to a branch with it resolved.
> >>> This isn't even a particularly tricky merge conflict, it just has the little
> >>> subtly around making sure the include symbols are conditional.
> >>>
> >>
> >> I assume Linus will not see any problems without your pull requests. In which
> >> case we can do this:
> >>
> >> - I send my pull request today
> >> - You can follow with yours with the fixes later on this week
> >>
> >
> > okay
> >
> >>> This doesn't affect me much as there is already another merge conflict with
> >>> the security tree that I need to deal with.
> >>>
> >>
> >>
> >>>> I think it might be least confusing option. Let me know. I can just do that
> >>>> and then send pull request in a day or tow once things settle down in next.
> >>>>
> >>>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
> >>>>
> >>>
> >>> that is the other option. If you go that route I can help you do the rebase/merge
> >>> fix.
> >>>
> >>
> >> Let's go with your earlier suggestion.
> >>
> >
> > ack
> >
> >>> looking back at this, there wasn't anything explicit about this not going upstream
> >>> this cycle, I must have just assumed as the final version came about after rc7. So
> >>> my bad.
> >>>
> >>
> >> Right - I ended up taking this as it looked like a patch if included could
> >> enable other changes to follow without being blocked. Also rc8 was in plan.
> >>
> >
> > yeah, my bad
> >
>
> No worries. Sent pull request with a note about apparmor and our
> coordinated pull requests with you on the cc.
>
> thanks,
> -- Shuah
>

Thanks John, Shuah for sorting this out. I confess that I hadn't
noticed the conflict before proposing this for 6.2: in retrospect I
should've checked more carefully given the amount of churn in the
patch.

If we have to drop this patch and split the series, that's not a
problem: it's really just an example. But if the conflict's resolved,
that's even better.

Thanks again!

-- David

2022-12-13 03:49:38

by Stephen Rothwell

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

Attachments:

(No filename) (499.00 B)
OpenPGP digital signature

2022-12-14 00:08:34

by Stephen Rothwell

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

Hi all,

On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <[email protected]> wrote:
>
> Today's linux-next merge of the kunit-next tree got a conflict in:
>
> security/apparmor/policy_unpack.c
>
> between commits:
>
> 371e50a0b19f ("apparmor: make unpack_array return a trianary value")
> 73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
> 217af7e2f4de ("apparmor: refactor profile rules and attachments")
> (and probably others)
>
> from the apparmor tree and commit:
>
> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>
> from the kunit-next tree.
>
> This is somewhat of a mess ... pity there is not a shared branch (or
> better routing if the patches).
>
> I fixed it up (hopefully - see below) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging. You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
>
> I also had to add this patch:
>
> From: Stephen Rothwell <[email protected]>
> Date: Thu, 8 Dec 2022 13:47:43 +1100
> Subject: [PATCH] fixup for "apparmor: make unpack_array return a trianary value"
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> security/apparmor/include/policy_unpack.h | 8 +++++++-
> security/apparmor/policy_unpack.c | 5 -----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
> index 940da8a33e0c..8fdf8f703bd0 100644
> --- a/security/apparmor/include/policy_unpack.h
> +++ b/security/apparmor/include/policy_unpack.h
> @@ -172,7 +172,13 @@ bool aa_unpack_X(struct aa_ext *e, enum aa_code code);
> bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
> bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
> bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
> -size_t aa_unpack_array(struct aa_ext *e, const char *name);
> +
> +#define tri int
> +#define TRI_TRUE 1
> +#define TRI_NONE 0
> +#define TRI_FALSE -1
> +
> +tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
> size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
> int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
> int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name);
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 6513545dad5e..173d832fc4ee 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -30,11 +30,6 @@
> #include "include/policy_unpack.h"
> #include "include/policy_compat.h"
>
> -#define tri int
> -#define TRI_TRUE 1
> -#define TRI_NONE 0
> -#define TRI_FALSE -1
> -
> /* audit callback for unpack fields */
> static void audit_cb(struct audit_buffer *ab, void *va)
> {
> --
> 2.35.1
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc security/apparmor/policy_unpack.c
> index 1bf8cfb8700a,12e535fdfa8b..000000000000
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@@ -14,9 -14,10 +14,10 @@@
> */
>
> #include <asm/unaligned.h>
> + #include <kunit/visibility.h>
> #include <linux/ctype.h>
> #include <linux/errno.h>
> -#include <linux/zlib.h>
> +#include <linux/zstd.h>
>
> #include "include/apparmor.h"
> #include "include/audit.h"
> @@@ -27,50 -27,16 +28,12 @@@
> #include "include/path.h"
> #include "include/policy.h"
> #include "include/policy_unpack.h"
> +#include "include/policy_compat.h"
>
> -
> - /*
> - * 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;
> - };
> -#define K_ABI_MASK 0x3ff
> -#define FORCE_COMPLAIN_FLAG 0x800
> -#define VERSION_LT(X, Y) (((X) & K_ABI_MASK) < ((Y) & K_ABI_MASK))
> -#define VERSION_GT(X, Y) (((X) & K_ABI_MASK) > ((Y) & K_ABI_MASK))
> --
> -#define v5 5 /* base version */
> -#define v6 6 /* per entry policydb mediation check */
> -#define v7 7
> -#define v8 8 /* full network masking */
> +#define tri int
> +#define TRI_TRUE 1
> +#define TRI_NONE 0
> +#define TRI_FALSE -1
>
> /* audit callback for unpack fields */
> static void audit_cb(struct audit_buffer *ab, void *va)
> @@@ -348,26 -319,28 +316,28 @@@ fail
> e->pos = pos;
> return false;
> }
> + EXPORT_SYMBOL_IF_KUNIT(aa_unpack_u64);
>
> - static tri unpack_array(struct aa_ext *e, const char *name, u16 *size)
> -VISIBLE_IF_KUNIT size_t aa_unpack_array(struct aa_ext *e, const char *name)
> ++VISIBLE_IF_KUNIT tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size)
> {
> void *pos = e->pos;
>
> - if (unpack_nameX(e, AA_ARRAY, name)) {
> - if (!inbounds(e, sizeof(u16)))
> + if (aa_unpack_nameX(e, AA_ARRAY, name)) {
> - int size;
> + if (!aa_inbounds(e, sizeof(u16)))
> goto fail;
> - size = (int)le16_to_cpu(get_unaligned((__le16 *) e->pos));
> + *size = le16_to_cpu(get_unaligned((__le16 *) e->pos));
> e->pos += sizeof(u16);
> - return size;
> + return TRI_TRUE;
> }
>
> + return TRI_NONE;
> fail:
> e->pos = pos;
> - return 0;
> + return TRI_FALSE;
> }
> + EXPORT_SYMBOL_IF_KUNIT(aa_unpack_array);
>
> - static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
> + VISIBLE_IF_KUNIT size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name)
> {
> void *pos = e->pos;
>
> @@@ -470,36 -447,32 +443,36 @@@ static struct aa_dfa *unpack_dfa(struc
> /**
> * unpack_trans_table - unpack a profile transition table
> * @e: serialized data extent information (NOT NULL)
> - * @profile: profile to add the accept table to (NOT NULL)
> + * @table: str table to unpack to (NOT NULL)
> *
> - * Returns: true if table successfully unpacked
> + * Returns: true if table successfully unpacked or not present
> */
> -static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile)
> +static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
> {
> void *saved_pos = e->pos;
> + char **table = NULL;
>
> /* exec table is optional */
> - if (unpack_nameX(e, AA_STRUCT, "xtable")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "xtable")) {
> - int i, size;
> -
> - size = aa_unpack_array(e, NULL);
> - /* currently 4 exec bits and entries 0-3 are reserved iupcx */
> - if (size > 16 - 4)
> + u16 size;
> + int i;
> +
> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
> + /*
> + * Note: index into trans table array is a max
> + * of 2^24, but unpack array can only unpack
> + * an array of 2^16 in size atm so no need
> + * for size check here
> + */
> goto fail;
> - profile->file.trans.table = kcalloc(size, sizeof(char *),
> - GFP_KERNEL);
> - if (!profile->file.trans.table)
> + table = kcalloc(size, sizeof(char *), GFP_KERNEL);
> + if (!table)
> goto fail;
>
> - profile->file.trans.size = size;
> for (i = 0; i < size; i++) {
> char *str;
> - int c, j, pos, size2 = unpack_strdup(e, &str, NULL);
> - /* unpack_strdup verifies that the last character is
> + int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL);
> + /* aa_unpack_strdup verifies that the last character is
> * null termination byte.
> */
> if (!size2)
> @@@ -534,13 -507,10 +507,13 @@@
> /* fail - all other cases with embedded \0 */
> goto fail;
> }
> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> +
> + strs->table = table;
> + strs->size = size;
> }
> return true;
>
> @@@ -554,23 -524,21 +527,23 @@@ static bool unpack_xattrs(struct aa_ex
> {
> void *pos = e->pos;
>
> - if (unpack_nameX(e, AA_STRUCT, "xattrs")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "xattrs")) {
> - int i, size;
> + u16 size;
> + int i;
>
> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
> - size = aa_unpack_array(e, NULL);
> - profile->xattr_count = size;
> - profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
> - if (!profile->xattrs)
> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
> + goto fail;
> + profile->attach.xattr_count = size;
> + profile->attach.xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
> + if (!profile->attach.xattrs)
> goto fail;
> for (i = 0; i < size; i++) {
> - if (!unpack_strdup(e, &profile->attach.xattrs[i], NULL))
> - if (!aa_unpack_strdup(e, &profile->xattrs[i], NULL))
> ++ if (!aa_unpack_strdup(e, &profile->attach.xattrs[i], NULL))
> goto fail;
> }
> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> }
>
> @@@ -581,34 -549,32 +554,34 @@@ fail
> return false;
> }
>
> -static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile)
> +static bool unpack_secmark(struct aa_ext *e, struct aa_ruleset *rules)
> {
> void *pos = e->pos;
> - int i, size;
> + u16 size;
> + int i;
>
> - if (unpack_nameX(e, AA_STRUCT, "secmark")) {
> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
> + if (aa_unpack_nameX(e, AA_STRUCT, "secmark")) {
> - size = aa_unpack_array(e, NULL);
> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
> + goto fail;
>
> - profile->secmark = kcalloc(size, sizeof(struct aa_secmark),
> + rules->secmark = kcalloc(size, sizeof(struct aa_secmark),
> GFP_KERNEL);
> - if (!profile->secmark)
> + if (!rules->secmark)
> goto fail;
>
> - profile->secmark_count = size;
> + rules->secmark_count = size;
>
> for (i = 0; i < size; i++) {
> - if (!unpack_u8(e, &profile->secmark[i].audit, NULL))
> + if (!unpack_u8(e, &rules->secmark[i].audit, NULL))
> goto fail;
> - if (!unpack_u8(e, &profile->secmark[i].deny, NULL))
> + if (!unpack_u8(e, &rules->secmark[i].deny, NULL))
> goto fail;
> - if (!unpack_strdup(e, &rules->secmark[i].label, NULL))
> - if (!aa_unpack_strdup(e, &profile->secmark[i].label, NULL))
> ++ if (!aa_unpack_strdup(e, &rules->secmark[i].label, NULL))
> goto fail;
> }
> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> }
>
> @@@ -632,27 -598,26 +605,27 @@@ static bool unpack_rlimits(struct aa_ex
> void *pos = e->pos;
>
> /* rlimits are optional */
> - if (unpack_nameX(e, AA_STRUCT, "rlimits")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "rlimits")) {
> - int i, size;
> + u16 size;
> + int i;
> u32 tmp = 0;
> - if (!unpack_u32(e, &tmp, NULL))
> + if (!aa_unpack_u32(e, &tmp, NULL))
> goto fail;
> - profile->rlimits.mask = tmp;
> + rules->rlimits.mask = tmp;
>
> - if (unpack_array(e, NULL, &size) != TRI_TRUE ||
> - size = aa_unpack_array(e, NULL);
> - if (size > RLIM_NLIMITS)
> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE ||
> + size > RLIM_NLIMITS)
> goto fail;
> for (i = 0; i < size; i++) {
> u64 tmp2 = 0;
> int a = aa_map_resource(i);
> - if (!unpack_u64(e, &tmp2, NULL))
> + if (!aa_unpack_u64(e, &tmp2, NULL))
> goto fail;
> - profile->rlimits.limits[a].rlim_max = tmp2;
> + rules->rlimits.limits[a].rlim_max = tmp2;
> }
> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> }
> return true;
> @@@ -662,144 -627,6 +635,144 @@@ fail
> return false;
> }
>
> +static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm)
> +{
> + bool res;
> +
> + if (version != 1)
> + return false;
> +
> - res = unpack_u32(e, &perm->allow, NULL);
> - res = res && unpack_u32(e, &perm->allow, NULL);
> - res = res && unpack_u32(e, &perm->deny, NULL);
> - res = res && unpack_u32(e, &perm->subtree, NULL);
> - res = res && unpack_u32(e, &perm->cond, NULL);
> - res = res && unpack_u32(e, &perm->kill, NULL);
> - res = res && unpack_u32(e, &perm->complain, NULL);
> - res = res && unpack_u32(e, &perm->prompt, NULL);
> - res = res && unpack_u32(e, &perm->audit, NULL);
> - res = res && unpack_u32(e, &perm->quiet, NULL);
> - res = res && unpack_u32(e, &perm->hide, NULL);
> - res = res && unpack_u32(e, &perm->xindex, NULL);
> - res = res && unpack_u32(e, &perm->tag, NULL);
> - res = res && unpack_u32(e, &perm->label, NULL);
> ++ res = aa_unpack_u32(e, &perm->allow, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->allow, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->deny, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->subtree, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->cond, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->kill, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->complain, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->prompt, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->audit, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->quiet, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->hide, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->xindex, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->tag, NULL);
> ++ res = res && aa_unpack_u32(e, &perm->label, NULL);
> +
> + return res;
> +}
> +
> +static ssize_t unpack_perms_table(struct aa_ext *e, struct aa_perms **perms)
> +{
> + void *pos = e->pos;
> + u16 size = 0;
> +
> + AA_BUG(!perms);
> + /*
> + * policy perms are optional, in which case perms are embedded
> + * in the dfa accept table
> + */
> - if (unpack_nameX(e, AA_STRUCT, "perms")) {
> ++ if (aa_unpack_nameX(e, AA_STRUCT, "perms")) {
> + int i;
> + u32 version;
> +
> - if (!unpack_u32(e, &version, "version"))
> ++ if (!aa_unpack_u32(e, &version, "version"))
> + goto fail_reset;
> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
> + goto fail_reset;
> + *perms = kcalloc(size, sizeof(struct aa_perms), GFP_KERNEL);
> + if (!*perms)
> + goto fail_reset;
> + for (i = 0; i < size; i++) {
> + if (!unpack_perm(e, version, &(*perms)[i]))
> + goto fail;
> + }
> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> ++ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> + goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> ++ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> + goto fail;
> + } else
> + *perms = NULL;
> +
> + return size;
> +
> +fail:
> + kfree(*perms);
> +fail_reset:
> + e->pos = pos;
> + return -EPROTO;
> +}
> +
> +static int unpack_pdb(struct aa_ext *e, struct aa_policydb *policy,
> + bool required_dfa, bool required_trans,
> + const char **info)
> +{
> + void *pos = e->pos;
> + int i, flags, error = -EPROTO;
> + ssize_t size;
> +
> + size = unpack_perms_table(e, &policy->perms);
> + if (size < 0) {
> + error = size;
> + policy->perms = NULL;
> + *info = "failed to unpack - perms";
> + goto fail;
> + }
> + policy->size = size;
> +
> + if (policy->perms) {
> + /* perms table present accept is index */
> + flags = TO_ACCEPT1_FLAG(YYTD_DATA32);
> + } else {
> + /* packed perms in accept1 and accept2 */
> + flags = TO_ACCEPT1_FLAG(YYTD_DATA32) |
> + TO_ACCEPT2_FLAG(YYTD_DATA32);
> + }
> +
> + policy->dfa = unpack_dfa(e, flags);
> + if (IS_ERR(policy->dfa)) {
> + error = PTR_ERR(policy->dfa);
> + policy->dfa = NULL;
> + *info = "failed to unpack - dfa";
> + goto fail;
> + } else if (!policy->dfa) {
> + if (required_dfa) {
> + *info = "missing required dfa";
> + goto fail;
> + }
> + goto out;
> + }
> +
> + /*
> + * only unpack the following if a dfa is present
> + *
> + * sadly start was given different names for file and policydb
> + * but since it is optional we can try both
> + */
> - if (!unpack_u32(e, &policy->start[0], "start"))
> ++ if (!aa_unpack_u32(e, &policy->start[0], "start"))
> + /* default start state */
> + policy->start[0] = DFA_START;
> - if (!unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
> ++ if (!aa_unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
> + /* default start state for xmatch and file dfa */
> + policy->start[AA_CLASS_FILE] = DFA_START;
> + } /* setup class index */
> + for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
> + policy->start[i] = aa_dfa_next(policy->dfa, policy->start[0],
> + i);
> + }
> + if (!unpack_trans_table(e, &policy->trans) && required_trans) {
> + *info = "failed to unpack profile transition table";
> + goto fail;
> + }
> +
> + /* TODO: move compat mapping here, requires dfa merging first */
> + /* TODO: move verify here, it has to be done after compat mappings */
> +out:
> + return 0;
> +
> +fail:
> + e->pos = pos;
> + return error;
> +}
> +
> static u32 strhash(const void *data, u32 len, u32 seed)
> {
> const char * const *key = data;
> @@@ -858,29 -683,26 +831,29 @@@ static struct aa_profile *unpack_profil
> }
>
> profile = aa_alloc_profile(name, NULL, GFP_KERNEL);
> - if (!profile)
> - return ERR_PTR(-ENOMEM);
> + if (!profile) {
> + info = "out of memory";
> + error = -ENOMEM;
> + goto fail;
> + }
> + rules = list_first_entry(&profile->rules, typeof(*rules), list);
>
> /* profile renaming is optional */
> - (void) unpack_str(e, &profile->rename, "rename");
> + (void) aa_unpack_str(e, &profile->rename, "rename");
>
> /* attachment string is optional */
> - (void) unpack_str(e, &profile->attach.xmatch_str, "attach");
> - (void) aa_unpack_str(e, &profile->attach, "attach");
> ++ (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach");
>
> /* xmatch is optional and may be NULL */
> - profile->xmatch = unpack_dfa(e);
> - if (IS_ERR(profile->xmatch)) {
> - error = PTR_ERR(profile->xmatch);
> - profile->xmatch = NULL;
> + error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info);
> + if (error) {
> info = "bad xmatch";
> goto fail;
> }
> - /* xmatch_len is not optional if xmatch is set */
> - if (profile->xmatch) {
> +
> + /* neither xmatch_len not xmatch_perms are optional if xmatch is set */
> + if (profile->attach.xmatch.dfa) {
> - if (!unpack_u32(e, &tmp, NULL)) {
> + if (!aa_unpack_u32(e, &tmp, NULL)) {
> info = "missing xmatch len";
> goto fail;
> }
> @@@ -943,38 -757,38 +916,38 @@@
> profile->path_flags = PATH_MEDIATE_DELETED;
>
> info = "failed to unpack profile capabilities";
> - if (!unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[0]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[0]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[0]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
> goto fail;
> - if (!unpack_u32(e, &tmpcap.cap[0], NULL))
> + if (!aa_unpack_u32(e, &tmpcap.cap[0], NULL))
> goto fail;
>
> info = "failed to unpack upper profile capabilities";
> - if (unpack_nameX(e, AA_STRUCT, "caps64")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "caps64")) {
> /* optional upper half of 64 bit caps */
> - if (!unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[1]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[1]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[1]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(tmpcap.cap[1]), NULL))
> + if (!aa_unpack_u32(e, &(tmpcap.cap[1]), NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> }
>
> info = "failed to unpack extended profile capabilities";
> - if (unpack_nameX(e, AA_STRUCT, "capsx")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "capsx")) {
> /* optional extended caps mediation mask */
> - if (!unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[0]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
> goto fail;
> - if (!unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
> - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[1]), NULL))
> ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
> goto fail;
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> }
>
> @@@ -993,55 -807,62 +966,55 @@@
> goto fail;
> }
>
> - if (unpack_nameX(e, AA_STRUCT, "policydb")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "policydb")) {
> /* generic policy dfa - optional and may be NULL */
> info = "failed to unpack policydb";
> - profile->policy.dfa = unpack_dfa(e);
> - if (IS_ERR(profile->policy.dfa)) {
> - error = PTR_ERR(profile->policy.dfa);
> - profile->policy.dfa = NULL;
> - goto fail;
> - } else if (!profile->policy.dfa) {
> - error = -EPROTO;
> + error = unpack_pdb(e, &rules->policy, true, false,
> + &info);
> + if (error)
> goto fail;
> - }
> - if (!aa_unpack_u32(e, &profile->policy.start[0], "start"))
> - /* default start state */
> - profile->policy.start[0] = DFA_START;
> - /* setup class index */
> - for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) {
> - profile->policy.start[i] =
> - aa_dfa_next(profile->policy.dfa,
> - profile->policy.start[0],
> - i);
> - }
> + /* Fixup: drop when we get rid of start array */
> + if (aa_dfa_next(rules->policy.dfa, rules->policy.start[0],
> + AA_CLASS_FILE))
> + rules->policy.start[AA_CLASS_FILE] =
> + aa_dfa_next(rules->policy.dfa,
> + rules->policy.start[0],
> + AA_CLASS_FILE);
> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
> goto fail;
> + error = aa_compat_map_policy(&rules->policy, e->version);
> + if (error) {
> + info = "failed to remap policydb permission table";
> + goto fail;
> + }
> } else
> - profile->policy.dfa = aa_get_dfa(nulldfa);
> + rules->policy.dfa = aa_get_dfa(nulldfa);
>
> /* get file rules */
> - profile->file.dfa = unpack_dfa(e);
> - if (IS_ERR(profile->file.dfa)) {
> - error = PTR_ERR(profile->file.dfa);
> - profile->file.dfa = NULL;
> - info = "failed to unpack profile file rules";
> + error = unpack_pdb(e, &rules->file, false, true, &info);
> + if (error) {
> goto fail;
> - } else if (profile->file.dfa) {
> - if (!aa_unpack_u32(e, &profile->file.start, "dfa_start"))
> - /* default start state */
> - profile->file.start = DFA_START;
> - } else if (profile->policy.dfa &&
> - profile->policy.start[AA_CLASS_FILE]) {
> - profile->file.dfa = aa_get_dfa(profile->policy.dfa);
> - profile->file.start = profile->policy.start[AA_CLASS_FILE];
> + } else if (rules->file.dfa) {
> + error = aa_compat_map_file(&rules->file);
> + if (error) {
> + info = "failed to remap file permission table";
> + goto fail;
> + }
> + } else if (rules->policy.dfa &&
> + rules->policy.start[AA_CLASS_FILE]) {
> + rules->file.dfa = aa_get_dfa(rules->policy.dfa);
> + rules->file.start[AA_CLASS_FILE] = rules->policy.start[AA_CLASS_FILE];
> } else
> - profile->file.dfa = aa_get_dfa(nulldfa);
> -
> - if (!unpack_trans_table(e, profile)) {
> - info = "failed to unpack profile transition table";
> - goto fail;
> - }
> + rules->file.dfa = aa_get_dfa(nulldfa);
>
> + error = -EPROTO;
> - if (unpack_nameX(e, AA_STRUCT, "data")) {
> + if (aa_unpack_nameX(e, AA_STRUCT, "data")) {
> info = "out of memory";
> profile->data = kzalloc(sizeof(*profile->data), GFP_KERNEL);
> - if (!profile->data)
> + if (!profile->data) {
> + error = -ENOMEM;
> goto fail;
> -
> + }
> params.nelem_hint = 3;
> params.key_len = sizeof(void *);
> params.key_offset = offsetof(struct aa_data, key);

This is now a conflict between the apparmor tree and Linus' tree
(including the updated fix patch).

--
Cheers,
Stephen Rothwell

Attachments:

(No filename) (499.00 B)
OpenPGP digital signature

2022-12-14 01:25:51

by John Johansen

[permalink] [raw]

Subject: Re: linux-next: manual merge of the kunit-next tree with the apparmor tree

On 12/13/22 16:00, Stephen Rothwell wrote:
> Hi all,
>
> On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <[email protected]> wrote:
>>
>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>
>> security/apparmor/policy_unpack.c
>>
>> between commits:
>>
>> 371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>> 73c7e91c8bc9 ("apparmor: Remove unnecessary size check when unpacking trans_table")
>> 217af7e2f4de ("apparmor: refactor profile rules and attachments")
>> (and probably others)
>>
>> from the apparmor tree and commit:
>>
>> 2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>
>> from the kunit-next tree.
>>
>> This is somewhat of a mess ... pity there is not a shared branch (or
>> better routing if the patches).
>>
>> I fixed it up (hopefully - see below) and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging. You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.
>>
>> I also had to add this patch:
>>
>> From: Stephen Rothwell <[email protected]>
>> Date: Thu, 8 Dec 2022 13:47:43 +1100
>> Subject: [PATCH] fixup for "apparmor: make unpack_array return a trianary value"
>>
>> Signed-off-by: Stephen Rothwell <[email protected]>
>> ---
>> security/apparmor/include/policy_unpack.h | 8 +++++++-
>> security/apparmor/policy_unpack.c | 5 -----
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
>> index 940da8a33e0c..8fdf8f703bd0 100644
>> --- a/security/apparmor/include/policy_unpack.h
>> +++ b/security/apparmor/include/policy_unpack.h
>> @@ -172,7 +172,13 @@ bool aa_unpack_X(struct aa_ext *e, enum aa_code code);
>> bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name);
>> bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name);
>> bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name);
>> -size_t aa_unpack_array(struct aa_ext *e, const char *name);
>> +
>> +#define tri int
>> +#define TRI_TRUE 1
>> +#define TRI_NONE 0
>> +#define TRI_FALSE -1
>> +
>> +tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size);
>> size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name);
>> int aa_unpack_str(struct aa_ext *e, const char **string, const char *name);
>> int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name);
>> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
>> index 6513545dad5e..173d832fc4ee 100644
>> --- a/security/apparmor/policy_unpack.c
>> +++ b/security/apparmor/policy_unpack.c
>> @@ -30,11 +30,6 @@
>> #include "include/policy_unpack.h"
>> #include "include/policy_compat.h"
>>
>> -#define tri int
>> -#define TRI_TRUE 1
>> -#define TRI_NONE 0
>> -#define TRI_FALSE -1
>> -
>> /* audit callback for unpack fields */
>> static void audit_cb(struct audit_buffer *ab, void *va)
>> {
>> --
>> 2.35.1
>>
>> --
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc security/apparmor/policy_unpack.c
>> index 1bf8cfb8700a,12e535fdfa8b..000000000000
>> --- a/security/apparmor/policy_unpack.c
>> +++ b/security/apparmor/policy_unpack.c
>> @@@ -14,9 -14,10 +14,10 @@@
>> */
>>
>> #include <asm/unaligned.h>
>> + #include <kunit/visibility.h>
>> #include <linux/ctype.h>
>> #include <linux/errno.h>
>> -#include <linux/zlib.h>
>> +#include <linux/zstd.h>
>>
>> #include "include/apparmor.h"
>> #include "include/audit.h"
>> @@@ -27,50 -27,16 +28,12 @@@
>> #include "include/path.h"
>> #include "include/policy.h"
>> #include "include/policy_unpack.h"
>> +#include "include/policy_compat.h"
>>
>> -
>> - /*
>> - * 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;
>> - };
>> -#define K_ABI_MASK 0x3ff
>> -#define FORCE_COMPLAIN_FLAG 0x800
>> -#define VERSION_LT(X, Y) (((X) & K_ABI_MASK) < ((Y) & K_ABI_MASK))
>> -#define VERSION_GT(X, Y) (((X) & K_ABI_MASK) > ((Y) & K_ABI_MASK))
>> --
>> -#define v5 5 /* base version */
>> -#define v6 6 /* per entry policydb mediation check */
>> -#define v7 7
>> -#define v8 8 /* full network masking */
>> +#define tri int
>> +#define TRI_TRUE 1
>> +#define TRI_NONE 0
>> +#define TRI_FALSE -1
>>
>> /* audit callback for unpack fields */
>> static void audit_cb(struct audit_buffer *ab, void *va)
>> @@@ -348,26 -319,28 +316,28 @@@ fail
>> e->pos = pos;
>> return false;
>> }
>> + EXPORT_SYMBOL_IF_KUNIT(aa_unpack_u64);
>>
>> - static tri unpack_array(struct aa_ext *e, const char *name, u16 *size)
>> -VISIBLE_IF_KUNIT size_t aa_unpack_array(struct aa_ext *e, const char *name)
>> ++VISIBLE_IF_KUNIT tri aa_unpack_array(struct aa_ext *e, const char *name, u16 *size)
>> {
>> void *pos = e->pos;
>>
>> - if (unpack_nameX(e, AA_ARRAY, name)) {
>> - if (!inbounds(e, sizeof(u16)))
>> + if (aa_unpack_nameX(e, AA_ARRAY, name)) {
>> - int size;
>> + if (!aa_inbounds(e, sizeof(u16)))
>> goto fail;
>> - size = (int)le16_to_cpu(get_unaligned((__le16 *) e->pos));
>> + *size = le16_to_cpu(get_unaligned((__le16 *) e->pos));
>> e->pos += sizeof(u16);
>> - return size;
>> + return TRI_TRUE;
>> }
>>
>> + return TRI_NONE;
>> fail:
>> e->pos = pos;
>> - return 0;
>> + return TRI_FALSE;
>> }
>> + EXPORT_SYMBOL_IF_KUNIT(aa_unpack_array);
>>
>> - static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name)
>> + VISIBLE_IF_KUNIT size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name)
>> {
>> void *pos = e->pos;
>>
>> @@@ -470,36 -447,32 +443,36 @@@ static struct aa_dfa *unpack_dfa(struc
>> /**
>> * unpack_trans_table - unpack a profile transition table
>> * @e: serialized data extent information (NOT NULL)
>> - * @profile: profile to add the accept table to (NOT NULL)
>> + * @table: str table to unpack to (NOT NULL)
>> *
>> - * Returns: true if table successfully unpacked
>> + * Returns: true if table successfully unpacked or not present
>> */
>> -static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile)
>> +static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
>> {
>> void *saved_pos = e->pos;
>> + char **table = NULL;
>>
>> /* exec table is optional */
>> - if (unpack_nameX(e, AA_STRUCT, "xtable")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "xtable")) {
>> - int i, size;
>> -
>> - size = aa_unpack_array(e, NULL);
>> - /* currently 4 exec bits and entries 0-3 are reserved iupcx */
>> - if (size > 16 - 4)
>> + u16 size;
>> + int i;
>> +
>> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
>> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
>> + /*
>> + * Note: index into trans table array is a max
>> + * of 2^24, but unpack array can only unpack
>> + * an array of 2^16 in size atm so no need
>> + * for size check here
>> + */
>> goto fail;
>> - profile->file.trans.table = kcalloc(size, sizeof(char *),
>> - GFP_KERNEL);
>> - if (!profile->file.trans.table)
>> + table = kcalloc(size, sizeof(char *), GFP_KERNEL);
>> + if (!table)
>> goto fail;
>>
>> - profile->file.trans.size = size;
>> for (i = 0; i < size; i++) {
>> char *str;
>> - int c, j, pos, size2 = unpack_strdup(e, &str, NULL);
>> - /* unpack_strdup verifies that the last character is
>> + int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL);
>> + /* aa_unpack_strdup verifies that the last character is
>> * null termination byte.
>> */
>> if (!size2)
>> @@@ -534,13 -507,10 +507,13 @@@
>> /* fail - all other cases with embedded \0 */
>> goto fail;
>> }
>> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> +
>> + strs->table = table;
>> + strs->size = size;
>> }
>> return true;
>>
>> @@@ -554,23 -524,21 +527,23 @@@ static bool unpack_xattrs(struct aa_ex
>> {
>> void *pos = e->pos;
>>
>> - if (unpack_nameX(e, AA_STRUCT, "xattrs")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "xattrs")) {
>> - int i, size;
>> + u16 size;
>> + int i;
>>
>> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
>> - size = aa_unpack_array(e, NULL);
>> - profile->xattr_count = size;
>> - profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
>> - if (!profile->xattrs)
>> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
>> + goto fail;
>> + profile->attach.xattr_count = size;
>> + profile->attach.xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
>> + if (!profile->attach.xattrs)
>> goto fail;
>> for (i = 0; i < size; i++) {
>> - if (!unpack_strdup(e, &profile->attach.xattrs[i], NULL))
>> - if (!aa_unpack_strdup(e, &profile->xattrs[i], NULL))
>> ++ if (!aa_unpack_strdup(e, &profile->attach.xattrs[i], NULL))
>> goto fail;
>> }
>> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> }
>>
>> @@@ -581,34 -549,32 +554,34 @@@ fail
>> return false;
>> }
>>
>> -static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile)
>> +static bool unpack_secmark(struct aa_ext *e, struct aa_ruleset *rules)
>> {
>> void *pos = e->pos;
>> - int i, size;
>> + u16 size;
>> + int i;
>>
>> - if (unpack_nameX(e, AA_STRUCT, "secmark")) {
>> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
>> + if (aa_unpack_nameX(e, AA_STRUCT, "secmark")) {
>> - size = aa_unpack_array(e, NULL);
>> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
>> + goto fail;
>>
>> - profile->secmark = kcalloc(size, sizeof(struct aa_secmark),
>> + rules->secmark = kcalloc(size, sizeof(struct aa_secmark),
>> GFP_KERNEL);
>> - if (!profile->secmark)
>> + if (!rules->secmark)
>> goto fail;
>>
>> - profile->secmark_count = size;
>> + rules->secmark_count = size;
>>
>> for (i = 0; i < size; i++) {
>> - if (!unpack_u8(e, &profile->secmark[i].audit, NULL))
>> + if (!unpack_u8(e, &rules->secmark[i].audit, NULL))
>> goto fail;
>> - if (!unpack_u8(e, &profile->secmark[i].deny, NULL))
>> + if (!unpack_u8(e, &rules->secmark[i].deny, NULL))
>> goto fail;
>> - if (!unpack_strdup(e, &rules->secmark[i].label, NULL))
>> - if (!aa_unpack_strdup(e, &profile->secmark[i].label, NULL))
>> ++ if (!aa_unpack_strdup(e, &rules->secmark[i].label, NULL))
>> goto fail;
>> }
>> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> }
>>
>> @@@ -632,27 -598,26 +605,27 @@@ static bool unpack_rlimits(struct aa_ex
>> void *pos = e->pos;
>>
>> /* rlimits are optional */
>> - if (unpack_nameX(e, AA_STRUCT, "rlimits")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "rlimits")) {
>> - int i, size;
>> + u16 size;
>> + int i;
>> u32 tmp = 0;
>> - if (!unpack_u32(e, &tmp, NULL))
>> + if (!aa_unpack_u32(e, &tmp, NULL))
>> goto fail;
>> - profile->rlimits.mask = tmp;
>> + rules->rlimits.mask = tmp;
>>
>> - if (unpack_array(e, NULL, &size) != TRI_TRUE ||
>> - size = aa_unpack_array(e, NULL);
>> - if (size > RLIM_NLIMITS)
>> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE ||
>> + size > RLIM_NLIMITS)
>> goto fail;
>> for (i = 0; i < size; i++) {
>> u64 tmp2 = 0;
>> int a = aa_map_resource(i);
>> - if (!unpack_u64(e, &tmp2, NULL))
>> + if (!aa_unpack_u64(e, &tmp2, NULL))
>> goto fail;
>> - profile->rlimits.limits[a].rlim_max = tmp2;
>> + rules->rlimits.limits[a].rlim_max = tmp2;
>> }
>> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> }
>> return true;
>> @@@ -662,144 -627,6 +635,144 @@@ fail
>> return false;
>> }
>>
>> +static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm)
>> +{
>> + bool res;
>> +
>> + if (version != 1)
>> + return false;
>> +
>> - res = unpack_u32(e, &perm->allow, NULL);
>> - res = res && unpack_u32(e, &perm->allow, NULL);
>> - res = res && unpack_u32(e, &perm->deny, NULL);
>> - res = res && unpack_u32(e, &perm->subtree, NULL);
>> - res = res && unpack_u32(e, &perm->cond, NULL);
>> - res = res && unpack_u32(e, &perm->kill, NULL);
>> - res = res && unpack_u32(e, &perm->complain, NULL);
>> - res = res && unpack_u32(e, &perm->prompt, NULL);
>> - res = res && unpack_u32(e, &perm->audit, NULL);
>> - res = res && unpack_u32(e, &perm->quiet, NULL);
>> - res = res && unpack_u32(e, &perm->hide, NULL);
>> - res = res && unpack_u32(e, &perm->xindex, NULL);
>> - res = res && unpack_u32(e, &perm->tag, NULL);
>> - res = res && unpack_u32(e, &perm->label, NULL);
>> ++ res = aa_unpack_u32(e, &perm->allow, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->allow, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->deny, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->subtree, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->cond, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->kill, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->complain, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->prompt, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->audit, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->quiet, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->hide, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->xindex, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->tag, NULL);
>> ++ res = res && aa_unpack_u32(e, &perm->label, NULL);
>> +
>> + return res;
>> +}
>> +
>> +static ssize_t unpack_perms_table(struct aa_ext *e, struct aa_perms **perms)
>> +{
>> + void *pos = e->pos;
>> + u16 size = 0;
>> +
>> + AA_BUG(!perms);
>> + /*
>> + * policy perms are optional, in which case perms are embedded
>> + * in the dfa accept table
>> + */
>> - if (unpack_nameX(e, AA_STRUCT, "perms")) {
>> ++ if (aa_unpack_nameX(e, AA_STRUCT, "perms")) {
>> + int i;
>> + u32 version;
>> +
>> - if (!unpack_u32(e, &version, "version"))
>> ++ if (!aa_unpack_u32(e, &version, "version"))
>> + goto fail_reset;
>> - if (unpack_array(e, NULL, &size) != TRI_TRUE)
>> ++ if (aa_unpack_array(e, NULL, &size) != TRI_TRUE)
>> + goto fail_reset;
>> + *perms = kcalloc(size, sizeof(struct aa_perms), GFP_KERNEL);
>> + if (!*perms)
>> + goto fail_reset;
>> + for (i = 0; i < size; i++) {
>> + if (!unpack_perm(e, version, &(*perms)[i]))
>> + goto fail;
>> + }
>> - if (!unpack_nameX(e, AA_ARRAYEND, NULL))
>> ++ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
>> + goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> ++ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> + goto fail;
>> + } else
>> + *perms = NULL;
>> +
>> + return size;
>> +
>> +fail:
>> + kfree(*perms);
>> +fail_reset:
>> + e->pos = pos;
>> + return -EPROTO;
>> +}
>> +
>> +static int unpack_pdb(struct aa_ext *e, struct aa_policydb *policy,
>> + bool required_dfa, bool required_trans,
>> + const char **info)
>> +{
>> + void *pos = e->pos;
>> + int i, flags, error = -EPROTO;
>> + ssize_t size;
>> +
>> + size = unpack_perms_table(e, &policy->perms);
>> + if (size < 0) {
>> + error = size;
>> + policy->perms = NULL;
>> + *info = "failed to unpack - perms";
>> + goto fail;
>> + }
>> + policy->size = size;
>> +
>> + if (policy->perms) {
>> + /* perms table present accept is index */
>> + flags = TO_ACCEPT1_FLAG(YYTD_DATA32);
>> + } else {
>> + /* packed perms in accept1 and accept2 */
>> + flags = TO_ACCEPT1_FLAG(YYTD_DATA32) |
>> + TO_ACCEPT2_FLAG(YYTD_DATA32);
>> + }
>> +
>> + policy->dfa = unpack_dfa(e, flags);
>> + if (IS_ERR(policy->dfa)) {
>> + error = PTR_ERR(policy->dfa);
>> + policy->dfa = NULL;
>> + *info = "failed to unpack - dfa";
>> + goto fail;
>> + } else if (!policy->dfa) {
>> + if (required_dfa) {
>> + *info = "missing required dfa";
>> + goto fail;
>> + }
>> + goto out;
>> + }
>> +
>> + /*
>> + * only unpack the following if a dfa is present
>> + *
>> + * sadly start was given different names for file and policydb
>> + * but since it is optional we can try both
>> + */
>> - if (!unpack_u32(e, &policy->start[0], "start"))
>> ++ if (!aa_unpack_u32(e, &policy->start[0], "start"))
>> + /* default start state */
>> + policy->start[0] = DFA_START;
>> - if (!unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
>> ++ if (!aa_unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
>> + /* default start state for xmatch and file dfa */
>> + policy->start[AA_CLASS_FILE] = DFA_START;
>> + } /* setup class index */
>> + for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
>> + policy->start[i] = aa_dfa_next(policy->dfa, policy->start[0],
>> + i);
>> + }
>> + if (!unpack_trans_table(e, &policy->trans) && required_trans) {
>> + *info = "failed to unpack profile transition table";
>> + goto fail;
>> + }
>> +
>> + /* TODO: move compat mapping here, requires dfa merging first */
>> + /* TODO: move verify here, it has to be done after compat mappings */
>> +out:
>> + return 0;
>> +
>> +fail:
>> + e->pos = pos;
>> + return error;
>> +}
>> +
>> static u32 strhash(const void *data, u32 len, u32 seed)
>> {
>> const char * const *key = data;
>> @@@ -858,29 -683,26 +831,29 @@@ static struct aa_profile *unpack_profil
>> }
>>
>> profile = aa_alloc_profile(name, NULL, GFP_KERNEL);
>> - if (!profile)
>> - return ERR_PTR(-ENOMEM);
>> + if (!profile) {
>> + info = "out of memory";
>> + error = -ENOMEM;
>> + goto fail;
>> + }
>> + rules = list_first_entry(&profile->rules, typeof(*rules), list);
>>
>> /* profile renaming is optional */
>> - (void) unpack_str(e, &profile->rename, "rename");
>> + (void) aa_unpack_str(e, &profile->rename, "rename");
>>
>> /* attachment string is optional */
>> - (void) unpack_str(e, &profile->attach.xmatch_str, "attach");
>> - (void) aa_unpack_str(e, &profile->attach, "attach");
>> ++ (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach");
>>
>> /* xmatch is optional and may be NULL */
>> - profile->xmatch = unpack_dfa(e);
>> - if (IS_ERR(profile->xmatch)) {
>> - error = PTR_ERR(profile->xmatch);
>> - profile->xmatch = NULL;
>> + error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info);
>> + if (error) {
>> info = "bad xmatch";
>> goto fail;
>> }
>> - /* xmatch_len is not optional if xmatch is set */
>> - if (profile->xmatch) {
>> +
>> + /* neither xmatch_len not xmatch_perms are optional if xmatch is set */
>> + if (profile->attach.xmatch.dfa) {
>> - if (!unpack_u32(e, &tmp, NULL)) {
>> + if (!aa_unpack_u32(e, &tmp, NULL)) {
>> info = "missing xmatch len";
>> goto fail;
>> }
>> @@@ -943,38 -757,38 +916,38 @@@
>> profile->path_flags = PATH_MEDIATE_DELETED;
>>
>> info = "failed to unpack profile capabilities";
>> - if (!unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[0]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[0]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[0]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[0]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[0]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &tmpcap.cap[0], NULL))
>> + if (!aa_unpack_u32(e, &tmpcap.cap[0], NULL))
>> goto fail;
>>
>> info = "failed to unpack upper profile capabilities";
>> - if (unpack_nameX(e, AA_STRUCT, "caps64")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "caps64")) {
>> /* optional upper half of 64 bit caps */
>> - if (!unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[1]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[1]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[1]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[1]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[1]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(tmpcap.cap[1]), NULL))
>> + if (!aa_unpack_u32(e, &(tmpcap.cap[1]), NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> }
>>
>> info = "failed to unpack extended profile capabilities";
>> - if (unpack_nameX(e, AA_STRUCT, "capsx")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "capsx")) {
>> /* optional extended caps mediation mask */
>> - if (!unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[0]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[0]), NULL))
>> goto fail;
>> - if (!unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
>> - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[1]), NULL))
>> ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[1]), NULL))
>> goto fail;
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> }
>>
>> @@@ -993,55 -807,62 +966,55 @@@
>> goto fail;
>> }
>>
>> - if (unpack_nameX(e, AA_STRUCT, "policydb")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "policydb")) {
>> /* generic policy dfa - optional and may be NULL */
>> info = "failed to unpack policydb";
>> - profile->policy.dfa = unpack_dfa(e);
>> - if (IS_ERR(profile->policy.dfa)) {
>> - error = PTR_ERR(profile->policy.dfa);
>> - profile->policy.dfa = NULL;
>> - goto fail;
>> - } else if (!profile->policy.dfa) {
>> - error = -EPROTO;
>> + error = unpack_pdb(e, &rules->policy, true, false,
>> + &info);
>> + if (error)
>> goto fail;
>> - }
>> - if (!aa_unpack_u32(e, &profile->policy.start[0], "start"))
>> - /* default start state */
>> - profile->policy.start[0] = DFA_START;
>> - /* setup class index */
>> - for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) {
>> - profile->policy.start[i] =
>> - aa_dfa_next(profile->policy.dfa,
>> - profile->policy.start[0],
>> - i);
>> - }
>> + /* Fixup: drop when we get rid of start array */
>> + if (aa_dfa_next(rules->policy.dfa, rules->policy.start[0],
>> + AA_CLASS_FILE))
>> + rules->policy.start[AA_CLASS_FILE] =
>> + aa_dfa_next(rules->policy.dfa,
>> + rules->policy.start[0],
>> + AA_CLASS_FILE);
>> - if (!unpack_nameX(e, AA_STRUCTEND, NULL))
>> + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
>> goto fail;
>> + error = aa_compat_map_policy(&rules->policy, e->version);
>> + if (error) {
>> + info = "failed to remap policydb permission table";
>> + goto fail;
>> + }
>> } else
>> - profile->policy.dfa = aa_get_dfa(nulldfa);
>> + rules->policy.dfa = aa_get_dfa(nulldfa);
>>
>> /* get file rules */
>> - profile->file.dfa = unpack_dfa(e);
>> - if (IS_ERR(profile->file.dfa)) {
>> - error = PTR_ERR(profile->file.dfa);
>> - profile->file.dfa = NULL;
>> - info = "failed to unpack profile file rules";
>> + error = unpack_pdb(e, &rules->file, false, true, &info);
>> + if (error) {
>> goto fail;
>> - } else if (profile->file.dfa) {
>> - if (!aa_unpack_u32(e, &profile->file.start, "dfa_start"))
>> - /* default start state */
>> - profile->file.start = DFA_START;
>> - } else if (profile->policy.dfa &&
>> - profile->policy.start[AA_CLASS_FILE]) {
>> - profile->file.dfa = aa_get_dfa(profile->policy.dfa);
>> - profile->file.start = profile->policy.start[AA_CLASS_FILE];
>> + } else if (rules->file.dfa) {
>> + error = aa_compat_map_file(&rules->file);
>> + if (error) {
>> + info = "failed to remap file permission table";
>> + goto fail;
>> + }
>> + } else if (rules->policy.dfa &&
>> + rules->policy.start[AA_CLASS_FILE]) {
>> + rules->file.dfa = aa_get_dfa(rules->policy.dfa);
>> + rules->file.start[AA_CLASS_FILE] = rules->policy.start[AA_CLASS_FILE];
>> } else
>> - profile->file.dfa = aa_get_dfa(nulldfa);
>> -
>> - if (!unpack_trans_table(e, profile)) {
>> - info = "failed to unpack profile transition table";
>> - goto fail;
>> - }
>> + rules->file.dfa = aa_get_dfa(nulldfa);
>>
>> + error = -EPROTO;
>> - if (unpack_nameX(e, AA_STRUCT, "data")) {
>> + if (aa_unpack_nameX(e, AA_STRUCT, "data")) {
>> info = "out of memory";
>> profile->data = kzalloc(sizeof(*profile->data), GFP_KERNEL);
>> - if (!profile->data)
>> + if (!profile->data) {
>> + error = -ENOMEM;
>> goto fail;
>> -
>> + }
>> params.nelem_hint = 3;
>> params.key_len = sizeof(void *);
>> params.key_offset = offsetof(struct aa_data, key);
>
> This is now a conflict between the apparmor tree and Linus' tree
> (including the updated fix patch).
>
yep thanks,
I am looking at it

2022-12-14 19:03:58

by John Johansen

[permalink] [raw]