linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* linux-next: manual merge of the kunit-next tree with the apparmor tree
@ 2022-04-05  2:55 Stephen Rothwell
  2022-07-04 23:13 ` Stephen Rothwell
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Rothwell @ 2022-04-05  2:55 UTC (permalink / raw)
  To: Shuah Khan, Brendan Higgins, John Johansen
  Cc: David Gow, Linux Kernel Mailing List, Linux Next Mailing List,
	Ricardo Ribalda

[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]

Hi all,

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

  security/apparmor/policy_unpack_test.c

between commit:

  d86d1652ab13 ("apparmor: test: Remove some casts which are no-longer required")

from the apparmor tree and commit:

  5f91bd9f1e7a ("apparmor: test: Use NULL macros")

from the kunit-next tree.

I fixed it up (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.

-- 
Cheers,
Stephen Rothwell

diff --cc security/apparmor/policy_unpack_test.c
index 399dce3781aa,5c18d2f19862..000000000000
--- a/security/apparmor/policy_unpack_test.c
+++ b/security/apparmor/policy_unpack_test.c
@@@ -408,8 -408,8 +408,8 @@@ static void policy_unpack_test_unpack_u
  
  	size = unpack_u16_chunk(puf->e, &chunk);
  
 -	KUNIT_EXPECT_EQ(test, size, (size_t)0);
 +	KUNIT_EXPECT_EQ(test, size, 0);
- 	KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
+ 	KUNIT_EXPECT_NULL(test, chunk);
  	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->end - 1);
  }
  
@@@ -430,8 -430,8 +430,8 @@@ static void policy_unpack_test_unpack_u
  
  	size = unpack_u16_chunk(puf->e, &chunk);
  
 -	KUNIT_EXPECT_EQ(test, size, (size_t)0);
 +	KUNIT_EXPECT_EQ(test, size, 0);
- 	KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
+ 	KUNIT_EXPECT_NULL(test, chunk);
  	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_U16_OFFSET);
  }
  

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-04-05  2:55 linux-next: manual merge of the kunit-next tree with the apparmor tree Stephen Rothwell
@ 2022-07-04 23:13 ` Stephen Rothwell
  2022-07-05  8:57   ` David Gow
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Rothwell @ 2022-07-04 23:13 UTC (permalink / raw)
  To: John Johansen
  Cc: Shuah Khan, Brendan Higgins, David Gow,
	Linux Kernel Mailing List, Linux Next Mailing List,
	Ricardo Ribalda

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

Hi all,

On Tue, 5 Apr 2022 12:55:40 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the kunit-next tree got a conflict in:
> 
>   security/apparmor/policy_unpack_test.c
> 
> between commit:
> 
>   d86d1652ab13 ("apparmor: test: Remove some casts which are no-longer required")
> 
> from the apparmor tree and commit:
> 
>   5f91bd9f1e7a ("apparmor: test: Use NULL macros")
> 
> from the kunit-next tree.
> 
> I fixed it up (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.
> 
> 
> diff --cc security/apparmor/policy_unpack_test.c
> index 399dce3781aa,5c18d2f19862..000000000000
> --- a/security/apparmor/policy_unpack_test.c
> +++ b/security/apparmor/policy_unpack_test.c
> @@@ -408,8 -408,8 +408,8 @@@ static void policy_unpack_test_unpack_u
>   
>   	size = unpack_u16_chunk(puf->e, &chunk);
>   
>  -	KUNIT_EXPECT_EQ(test, size, (size_t)0);
>  +	KUNIT_EXPECT_EQ(test, size, 0);
> - 	KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
> + 	KUNIT_EXPECT_NULL(test, chunk);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->end - 1);
>   }
>   
> @@@ -430,8 -430,8 +430,8 @@@ static void policy_unpack_test_unpack_u
>   
>   	size = unpack_u16_chunk(puf->e, &chunk);
>   
>  -	KUNIT_EXPECT_EQ(test, size, (size_t)0);
>  +	KUNIT_EXPECT_EQ(test, size, 0);
> - 	KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
> + 	KUNIT_EXPECT_NULL(test, chunk);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_U16_OFFSET);
>   }
>   

This is now a conflict between the apparmor tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-07-04 23:13 ` Stephen Rothwell
@ 2022-07-05  8:57   ` David Gow
  2022-07-05 18:22     ` John Johansen
  0 siblings, 1 reply; 20+ messages in thread
From: David Gow @ 2022-07-05  8:57 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: John Johansen, Shuah Khan, Brendan Higgins,
	Linux Kernel Mailing List, Linux Next Mailing List,
	Ricardo Ribalda

[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]

On Tue, Jul 5, 2022 at 7:14 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> On Tue, 5 Apr 2022 12:55:40 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Today's linux-next merge of the kunit-next tree got a conflict in:
> >
> >   security/apparmor/policy_unpack_test.c
> >
> > between commit:
> >
> >   d86d1652ab13 ("apparmor: test: Remove some casts which are no-longer required")
> >
> > from the apparmor tree and commit:
> >
> >   5f91bd9f1e7a ("apparmor: test: Use NULL macros")
> >
> > from the kunit-next tree.
> >
> > I fixed it up (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.
> >
> >
> > diff --cc security/apparmor/policy_unpack_test.c
> > index 399dce3781aa,5c18d2f19862..000000000000
> > --- a/security/apparmor/policy_unpack_test.c
> > +++ b/security/apparmor/policy_unpack_test.c
> > @@@ -408,8 -408,8 +408,8 @@@ static void policy_unpack_test_unpack_u
> >
> >       size = unpack_u16_chunk(puf->e, &chunk);
> >
> >  -    KUNIT_EXPECT_EQ(test, size, (size_t)0);
> >  +    KUNIT_EXPECT_EQ(test, size, 0);
> > -     KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
> > +     KUNIT_EXPECT_NULL(test, chunk);
> >       KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->end - 1);
> >   }
> >
> > @@@ -430,8 -430,8 +430,8 @@@ static void policy_unpack_test_unpack_u
> >
> >       size = unpack_u16_chunk(puf->e, &chunk);
> >
> >  -    KUNIT_EXPECT_EQ(test, size, (size_t)0);
> >  +    KUNIT_EXPECT_EQ(test, size, 0);
> > -     KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
> > +     KUNIT_EXPECT_NULL(test, chunk);
> >       KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_U16_OFFSET);
> >   }
> >
>
> This is now a conflict between the apparmor tree and Linus' tree.
>

Hmm... this patch -- d86d1652ab13 ("apparmor: test: Remove some casts
which are no-longer required") -- has been sitting in the
apparmor-next branch since December, but there haven't been any
AppArmor pull requests since then.

If it's easier, I'm happy to redo this and send it in via the KUnit
tree (assuming it gets removed from apparmor-next). Otherwise, I guess
this'll just have to wait for the next AppArmor PR.

-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-07-05  8:57   ` David Gow
@ 2022-07-05 18:22     ` John Johansen
  0 siblings, 0 replies; 20+ messages in thread
From: John Johansen @ 2022-07-05 18:22 UTC (permalink / raw)
  To: David Gow, Stephen Rothwell
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Ricardo Ribalda

On 7/5/22 01:57, David Gow wrote:
> On Tue, Jul 5, 2022 at 7:14 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Hi all,
>>
>> On Tue, 5 Apr 2022 12:55:40 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>
>>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>>
>>>    security/apparmor/policy_unpack_test.c
>>>
>>> between commit:
>>>
>>>    d86d1652ab13 ("apparmor: test: Remove some casts which are no-longer required")
>>>
>>> from the apparmor tree and commit:
>>>
>>>    5f91bd9f1e7a ("apparmor: test: Use NULL macros")
>>>
>>> from the kunit-next tree.
>>>
>>> I fixed it up (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.
>>>
>>>
>>> diff --cc security/apparmor/policy_unpack_test.c
>>> index 399dce3781aa,5c18d2f19862..000000000000
>>> --- a/security/apparmor/policy_unpack_test.c
>>> +++ b/security/apparmor/policy_unpack_test.c
>>> @@@ -408,8 -408,8 +408,8 @@@ static void policy_unpack_test_unpack_u
>>>
>>>        size = unpack_u16_chunk(puf->e, &chunk);
>>>
>>>   -    KUNIT_EXPECT_EQ(test, size, (size_t)0);
>>>   +    KUNIT_EXPECT_EQ(test, size, 0);
>>> -     KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
>>> +     KUNIT_EXPECT_NULL(test, chunk);
>>>        KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->end - 1);
>>>    }
>>>
>>> @@@ -430,8 -430,8 +430,8 @@@ static void policy_unpack_test_unpack_u
>>>
>>>        size = unpack_u16_chunk(puf->e, &chunk);
>>>
>>>   -    KUNIT_EXPECT_EQ(test, size, (size_t)0);
>>>   +    KUNIT_EXPECT_EQ(test, size, 0);
>>> -     KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
>>> +     KUNIT_EXPECT_NULL(test, chunk);
>>>        KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_U16_OFFSET);
>>>    }
>>>
>>
>> This is now a conflict between the apparmor tree and Linus' tree.
>>
> 
> Hmm... this patch -- d86d1652ab13 ("apparmor: test: Remove some casts
> which are no-longer required") -- has been sitting in the
> apparmor-next branch since December, but there haven't been any
> AppArmor pull requests since then.
> 
yeah sorry we had some testing infrastructure issues against the upstream kernel that
needed to be reworked, and I have been refusing to pull new stuff push a pull request
until we could get that fixed and have passing tests to ensure we weren't breaking
something, and this got caught up in that mess. That has finally happened this whole
lot that has been sitting can finally go up, and I can start merging in some of the
queued up new stuff.

> If it's easier, I'm happy to redo this and send it in via the KUnit
> tree (assuming it gets removed from apparmor-next). Otherwise, I guess
> this'll just have to wait for the next AppArmor PR.
> 
I have dropped it from apparmor-next for the moment. It can go up via KUnit or the
apparmor PR for 5.20.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-13 23:58 ` Stephen Rothwell
@ 2022-12-14 18:38   ` John Johansen
  0 siblings, 0 replies; 20+ messages in thread
From: John Johansen @ 2022-12-14 18:38 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Rae Moar

On 12/13/22 15:58, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 8 Dec 2022 12:46:53 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Today's linux-next merge of the kunit-next tree got a conflict in:
>>
>>    security/apparmor/policy_unpack_test.c
>>
>> between commits:
>>
>>    371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>>    32490541682b ("apparmor: Fix kunit test for out of bounds array")
>>
>> from the apparmor tree and commit:
>>
>>    2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
>>
>> from the kunit-next tree.
>>
>> I fixed it up (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.
>>
>> -- 
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc security/apparmor/policy_unpack_test.c
>> index 7465da42492d,f25cf2a023d5..000000000000
>> --- a/security/apparmor/policy_unpack_test.c
>> +++ b/security/apparmor/policy_unpack_test.c
>> @@@ -144,8 -147,8 +147,8 @@@ static void policy_unpack_test_unpack_a
>>    
>>    	puf->e->pos += TEST_ARRAY_BUF_OFFSET;
>>    
>> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, NULL, &array_size),
>>   -	array_size = aa_unpack_array(puf->e, NULL);
>>   -
>> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, NULL, &array_size),
>>   +			TRI_TRUE);
>>    	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
>>    	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>>    		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
>> @@@ -159,8 -162,8 +162,8 @@@ static void policy_unpack_test_unpack_a
>>    
>>    	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
>>    
>> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
>>   -	array_size = aa_unpack_array(puf->e, name);
>>   -
>> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
>>   +			TRI_TRUE);
>>    	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
>>    	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>>    		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
>> @@@ -175,8 -178,9 +178,8 @@@ static void policy_unpack_test_unpack_a
>>    	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
>>    	puf->e->end = puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16);
>>    
>> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
>>   -	array_size = aa_unpack_array(puf->e, name);
>>   -
>>   -	KUNIT_EXPECT_EQ(test, array_size, 0);
>> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
>>   +			TRI_FALSE);
>>    	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>>    		puf->e->start + TEST_NAMED_ARRAY_BUF_OFFSET);
>>    }
> 
> This is now a conflict between the apparmor tree and Linus' tree.
> 


sorry for the delay on this, build and regression testing took way
longer than they should have.

apparmor merge request is now sent



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-14  0:00 ` Stephen Rothwell
  2022-12-14  0:55   ` John Johansen
@ 2022-12-14 18:38   ` John Johansen
  1 sibling, 0 replies; 20+ messages in thread
From: John Johansen @ 2022-12-14 18:38 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Rae Moar

On 12/13/22 16:00, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 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 <sfr@canb.auug.org.au>
>> 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 <sfr@canb.auug.org.au>
>> ---
>>   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).
> 

sorry for the delay on this, build and regression testing took way
longer than they should have.

apparmor merge request is now sent


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-14  0:00 ` Stephen Rothwell
@ 2022-12-14  0:55   ` John Johansen
  2022-12-14 18:38   ` John Johansen
  1 sibling, 0 replies; 20+ messages in thread
From: John Johansen @ 2022-12-14  0:55 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Rae Moar

On 12/13/22 16:00, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 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 <sfr@canb.auug.org.au>
>> 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 <sfr@canb.auug.org.au>
>> ---
>>   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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-08  2:53 Stephen Rothwell
  2022-12-08 20:10 ` John Johansen
  2022-12-13  3:22 ` Stephen Rothwell
@ 2022-12-14  0:00 ` Stephen Rothwell
  2022-12-14  0:55   ` John Johansen
  2022-12-14 18:38   ` John Johansen
  2 siblings, 2 replies; 20+ messages in thread
From: Stephen Rothwell @ 2022-12-14  0:00 UTC (permalink / raw)
  To: John Johansen
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Rae Moar

[-- Attachment #1: Type: text/plain, Size: 27125 bytes --]

Hi all,

On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 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 <sfr@canb.auug.org.au>
> 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 <sfr@canb.auug.org.au>
> ---
>  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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-08  1:46 Stephen Rothwell
@ 2022-12-13 23:58 ` Stephen Rothwell
  2022-12-14 18:38   ` John Johansen
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Rothwell @ 2022-12-13 23:58 UTC (permalink / raw)
  To: John Johansen
  Cc: Shuah Khan, Brendan Higgins, Linux Kernel Mailing List,
	Linux Next Mailing List, Rae Moar

[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]

Hi all,

On Thu, 8 Dec 2022 12:46:53 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the kunit-next tree got a conflict in:
> 
>   security/apparmor/policy_unpack_test.c
> 
> between commits:
> 
>   371e50a0b19f ("apparmor: make unpack_array return a trianary value")
>   32490541682b ("apparmor: Fix kunit test for out of bounds array")
> 
> from the apparmor tree and commit:
> 
>   2c92044683f5 ("apparmor: test: make static symbols visible during kunit testing")
> 
> from the kunit-next tree.
> 
> I fixed it up (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.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc security/apparmor/policy_unpack_test.c
> index 7465da42492d,f25cf2a023d5..000000000000
> --- a/security/apparmor/policy_unpack_test.c
> +++ b/security/apparmor/policy_unpack_test.c
> @@@ -144,8 -147,8 +147,8 @@@ static void policy_unpack_test_unpack_a
>   
>   	puf->e->pos += TEST_ARRAY_BUF_OFFSET;
>   
> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, NULL, &array_size),
>  -	array_size = aa_unpack_array(puf->e, NULL);
>  -
> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, NULL, &array_size),
>  +			TRI_TRUE);
>   	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>   		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
> @@@ -159,8 -162,8 +162,8 @@@ static void policy_unpack_test_unpack_a
>   
>   	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
>   
> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
>  -	array_size = aa_unpack_array(puf->e, name);
>  -
> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
>  +			TRI_TRUE);
>   	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>   		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
> @@@ -175,8 -178,9 +178,8 @@@ static void policy_unpack_test_unpack_a
>   	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
>   	puf->e->end = puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16);
>   
> - 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
>  -	array_size = aa_unpack_array(puf->e, name);
>  -
>  -	KUNIT_EXPECT_EQ(test, array_size, 0);
> ++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
>  +			TRI_FALSE);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
>   		puf->e->start + TEST_NAMED_ARRAY_BUF_OFFSET);
>   }

This is now a conflict between the apparmor tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-08  2:53 Stephen Rothwell
  2022-12-08 20:10 ` John Johansen
@ 2022-12-13  3:22 ` Stephen Rothwell
  2022-12-14  0:00 ` Stephen Rothwell
  2 siblings, 0 replies; 20+ messages in thread
From: Stephen Rothwell @ 2022-12-13  3:22 UTC (permalink / raw)
  To: Shuah Khan, Brendan Higgins, John Johansen
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

[-- Attachment #1: Type: text/plain, Size: 3437 bytes --]

Hi all,

On Thu, 8 Dec 2022 13:53:27 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 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:

This merge fix patch is now:

From: Stephen Rothwell <sfr@canb.auug.org.au>
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 <sfr@canb.auug.org.au>
---
 security/apparmor/include/policy_unpack.h | 7 ++++++-
 security/apparmor/policy_unpack.c         | 5 -----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h
index 940da8a33e0c..67d59b9736de 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);
@@ -172,7 +177,7 @@ 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);
+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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 23:19             ` Shuah Khan
@ 2022-12-12 23:56               ` David Gow
  0 siblings, 0 replies; 20+ messages in thread
From: David Gow @ 2022-12-12 23:56 UTC (permalink / raw)
  To: Shuah Khan
  Cc: John Johansen, Stephen Rothwell, Brendan Higgins,
	Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

On Tue, 13 Dec 2022 at 07:19, Shuah Khan <skhan@linuxfoundation.org> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 19:53           ` John Johansen
@ 2022-12-12 23:19             ` Shuah Khan
  2022-12-12 23:56               ` David Gow
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2022-12-12 23:19 UTC (permalink / raw)
  To: John Johansen, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar, Shuah Khan

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 19:48         ` Shuah Khan
@ 2022-12-12 19:53           ` John Johansen
  2022-12-12 23:19             ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: John Johansen @ 2022-12-12 19:53 UTC (permalink / raw)
  To: Shuah Khan, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 19:20       ` John Johansen
@ 2022-12-12 19:48         ` Shuah Khan
  2022-12-12 19:53           ` John Johansen
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2022-12-12 19:48 UTC (permalink / raw)
  To: John Johansen, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar, Shuah Khan

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 18:03     ` Shuah Khan
@ 2022-12-12 19:20       ` John Johansen
  2022-12-12 19:48         ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: John Johansen @ 2022-12-12 19:20 UTC (permalink / raw)
  To: Shuah Khan, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-12 17:52   ` Shuah Khan
@ 2022-12-12 18:03     ` Shuah Khan
  2022-12-12 19:20       ` John Johansen
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2022-12-12 18:03 UTC (permalink / raw)
  To: John Johansen, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar, Shuah Khan

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-08 20:10 ` John Johansen
@ 2022-12-12 17:52   ` Shuah Khan
  2022-12-12 18:03     ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2022-12-12 17:52 UTC (permalink / raw)
  To: John Johansen, Stephen Rothwell, Brendan Higgins, David Gow
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar, Shuah Khan

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 <john.johansen@canonical.com>
> 
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> 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 <sfr@canb.auug.org.au>
>> ---
>>   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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: linux-next: manual merge of the kunit-next tree with the apparmor tree
  2022-12-08  2:53 Stephen Rothwell
@ 2022-12-08 20:10 ` John Johansen
  2022-12-12 17:52   ` Shuah Khan
  2022-12-13  3:22 ` Stephen Rothwell
  2022-12-14  0:00 ` Stephen Rothwell
  2 siblings, 1 reply; 20+ messages in thread
From: John Johansen @ 2022-12-08 20:10 UTC (permalink / raw)
  To: Stephen Rothwell, Shuah Khan, Brendan Higgins
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

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 <john.johansen@canonical.com>

> From: Stephen Rothwell <sfr@canb.auug.org.au>
> 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 <sfr@canb.auug.org.au>
> ---
>   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)
>   {


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* linux-next: manual merge of the kunit-next tree with the apparmor tree
@ 2022-12-08  2:53 Stephen Rothwell
  2022-12-08 20:10 ` John Johansen
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Stephen Rothwell @ 2022-12-08  2:53 UTC (permalink / raw)
  To: Shuah Khan, Brendan Higgins, John Johansen
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

[-- Attachment #1: Type: text/plain, Size: 25394 bytes --]

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 <sfr@canb.auug.org.au>
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 <sfr@canb.auug.org.au>
---
 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);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* linux-next: manual merge of the kunit-next tree with the apparmor tree
@ 2022-12-08  1:46 Stephen Rothwell
  2022-12-13 23:58 ` Stephen Rothwell
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Rothwell @ 2022-12-08  1:46 UTC (permalink / raw)
  To: Shuah Khan, Brendan Higgins, John Johansen
  Cc: Linux Kernel Mailing List, Linux Next Mailing List, Rae Moar

[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]

Hi all,

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

  security/apparmor/policy_unpack_test.c

between commits:

  371e50a0b19f ("apparmor: make unpack_array return a trianary value")
  32490541682b ("apparmor: Fix kunit test for out of bounds array")

from the apparmor tree and commit:

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

from the kunit-next tree.

I fixed it up (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.

-- 
Cheers,
Stephen Rothwell

diff --cc security/apparmor/policy_unpack_test.c
index 7465da42492d,f25cf2a023d5..000000000000
--- a/security/apparmor/policy_unpack_test.c
+++ b/security/apparmor/policy_unpack_test.c
@@@ -144,8 -147,8 +147,8 @@@ static void policy_unpack_test_unpack_a
  
  	puf->e->pos += TEST_ARRAY_BUF_OFFSET;
  
- 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, NULL, &array_size),
 -	array_size = aa_unpack_array(puf->e, NULL);
 -
++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, NULL, &array_size),
 +			TRI_TRUE);
  	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
  	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
  		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
@@@ -159,8 -162,8 +162,8 @@@ static void policy_unpack_test_unpack_a
  
  	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
  
- 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
 -	array_size = aa_unpack_array(puf->e, name);
 -
++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
 +			TRI_TRUE);
  	KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE);
  	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
  		puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1);
@@@ -175,8 -178,9 +178,8 @@@ static void policy_unpack_test_unpack_a
  	puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET;
  	puf->e->end = puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16);
  
- 	KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size),
 -	array_size = aa_unpack_array(puf->e, name);
 -
 -	KUNIT_EXPECT_EQ(test, array_size, 0);
++	KUNIT_EXPECT_EQ(test, aa_unpack_array(puf->e, name, &array_size),
 +			TRI_FALSE);
  	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
  		puf->e->start + TEST_NAMED_ARRAY_BUF_OFFSET);
  }

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-12-14 18:39 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05  2:55 linux-next: manual merge of the kunit-next tree with the apparmor tree Stephen Rothwell
2022-07-04 23:13 ` Stephen Rothwell
2022-07-05  8:57   ` David Gow
2022-07-05 18:22     ` John Johansen
2022-12-08  1:46 Stephen Rothwell
2022-12-13 23:58 ` Stephen Rothwell
2022-12-14 18:38   ` John Johansen
2022-12-08  2:53 Stephen Rothwell
2022-12-08 20:10 ` John Johansen
2022-12-12 17:52   ` Shuah Khan
2022-12-12 18:03     ` Shuah Khan
2022-12-12 19:20       ` John Johansen
2022-12-12 19:48         ` Shuah Khan
2022-12-12 19:53           ` John Johansen
2022-12-12 23:19             ` Shuah Khan
2022-12-12 23:56               ` David Gow
2022-12-13  3:22 ` Stephen Rothwell
2022-12-14  0:00 ` Stephen Rothwell
2022-12-14  0:55   ` John Johansen
2022-12-14 18:38   ` John Johansen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).