Re: [PATCH 0/2] apparmor unaligned memory fixes

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

Hi Helge,


On Thu, 2025-11-27 at 10:25 +0100, John Paul Adrian Glaubitz wrote:
> Hi Helge,
> 
> On Wed, 2025-11-26 at 21:15 +0100, Helge Deller wrote:
> > So, here is a (untested) v3:
> > 
> > 
> > [PATCH v3] apparmor: Optimize table creation from possibly unaligned memory
> > 
> > Source blob may come from userspace and might be unaligned.
> > Try to optize the copying process by avoiding unaligned memory accesses.
> > 
> > Signed-off-by: Helge Deller <deller@gmx.de>
> > 
> > diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> > index 1fbe82f5021b..19e72b3e8f49 100644
> > --- a/security/apparmor/include/match.h
> > +++ b/security/apparmor/include/match.h
> > @@ -104,16 +104,18 @@ struct aa_dfa {
> >  	struct table_header *tables[YYTD_ID_TSIZE];
> >  };
> >  
> > -#define byte_to_byte(X) (X)
> > -
> >  #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX)	\
> >  	do { \
> >  		typeof(LEN) __i; \
> >  		TTYPE *__t = (TTYPE *) TABLE; \
> >  		BTYPE *__b = (BTYPE *) BLOB; \
> > -		for (__i = 0; __i < LEN; __i++) { \
> > -			__t[__i] = NTOHX(__b[__i]); \
> > -		} \
> > +		BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> > +		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) \
> > +			memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> > +		else /* copy & convert convert from big-endian */ \
> > +			for (__i = 0; __i < LEN; __i++) { \
> > +				__t[__i] = NTOHX(&__b[__i]); \
> > +			} \
> >  	} while (0)
> >  
> >  static inline size_t table_size(size_t len, size_t el_size)
> > diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> > index c5a91600842a..1e32c8ba14ae 100644
> > --- a/security/apparmor/match.c
> > +++ b/security/apparmor/match.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/vmalloc.h>
> >  #include <linux/err.h>
> >  #include <linux/kref.h>
> > +#include <linux/unaligned.h>
> >  
> >  #include "include/lib.h"
> >  #include "include/match.h"
> > @@ -66,14 +67,13 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> >  		table->td_flags = th.td_flags;
> >  		table->td_lolen = th.td_lolen;
> >  		if (th.td_flags == YYTD_DATA8)
> > -			UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> > -				     u8, u8, byte_to_byte);
> > +			memcpy(table->td_data, blob, th.td_lolen);
> >  		else if (th.td_flags == YYTD_DATA16)
> >  			UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> > -				     u16, __be16, be16_to_cpu);
> > +				     u16, __be16, get_unaligned_be16);
> >  		else if (th.td_flags == YYTD_DATA32)
> >  			UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> > -				     u32, __be32, be32_to_cpu);
> > +				     u32, __be32, get_unaligned_be32);
> >  		else
> >  			goto fail;
> >  		/* if table was vmalloced make sure the page tables are synced
> 
> This one does not apply:
> 
> glaubitz@node54:/data/home/glaubitz/linux> git am ../20251125_app_armor_unalign_2nd.mbx
> Applying: apparmor unaligned memory fixes
> error: patch failed: security/apparmor/match.c:15
> error: security/apparmor/match.c: patch does not apply
> Patch failed at 0001 apparmor unaligned memory fixes
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config set advice.mergeConflict false"
> glaubitz@node54:/data/home/glaubitz/linux>

The patch alone applies, i.e without your previous patch, but it does not fix the problem:

[   73.961582] Kernel unaligned access at TPC[8dabdc] aa_dfa_unpack+0x3c/0x6e0
[   74.053195] Kernel unaligned access at TPC[8dabec] aa_dfa_unpack+0x4c/0x6e0
[   74.144814] Kernel unaligned access at TPC[8dacd0] aa_dfa_unpack+0x130/0x6e0
[   74.237538] Kernel unaligned access at TPC[8dacd0] aa_dfa_unpack+0x130/0x6e0
[   74.330296] Kernel unaligned access at TPC[8dacd0] aa_dfa_unpack+0x130/0x6e0

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913