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

John Paul Adrian Glaubitz
2025-11-26T07:28:05Z

Hi Helge,

On Tue, 2025-11-25 at 16:11 +0100, Helge Deller wrote:
> Regarding this:
>
> > Kernel side, we are going to need to add some extra verification checks, it should
> > be catching this, as unaligned as part of the unpack. Userspace side, we will have
> > to verify my guess and fix the loader.
>
> I wonder if loading those tables are really time critical?
> If not, maybe just making the kernel aware that the tables might be unaligned
> can help, e.g. with the following (untested) patch.
> Adrian, maybe you want to test?

Yes, I'll test that one.

> ------------------------
>
> [PATCH] Allow apparmor to handle unaligned dfa tables
>
> The dfa tables can originate from kernel or userspace and 8-byte alignment
> isn't always guaranteed and as such may trigger unaligned memory accesses
> on various architectures.
> Work around it by using the get_unaligned_xx() helpers.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index c5a91600842a..26e82ba879d4 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"
> @@ -42,11 +43,11 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> /* loaded td_id's start at 1, subtract 1 now to avoid doing
> * it every time we use td_id as an index
> */
> - th.td_id = be16_to_cpu(*(__be16 *) (blob)) - 1;
> + th.td_id = get_unaligned_be16(blob) - 1;
> if (th.td_id > YYTD_ID_MAX)
> goto out;
> - th.td_flags = be16_to_cpu(*(__be16 *) (blob + 2));
> - th.td_lolen = be32_to_cpu(*(__be32 *) (blob + 8));
> + th.td_flags = get_unaligned_be16(blob + 2);
> + th.td_lolen = get_unaligned_be32(blob + 8);
> blob += sizeof(struct table_header);
>
> if (!(th.td_flags == YYTD_DATA16 || th.td_flags == YYTD_DATA32 ||
> @@ -313,14 +314,14 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
> if (size < sizeof(struct table_set_header))
> goto fail;
>
> - if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
> + if (get_unaligned_be32(data) != YYTH_MAGIC)
> goto fail;
>
> - hsize = ntohl(*(__be32 *) (data + 4));
> + hsize = get_unaligned_be32(data + 4);
> if (size < hsize)
> goto fail;
>
> - dfa->flags = ntohs(*(__be16 *) (data + 12));
> + dfa->flags = get_unaligned_be16(data + 12);
> if (dfa->flags & ~(YYTH_FLAGS))
> goto fail;
>
> @@ -329,7 +330,7 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
> * if (dfa->flags & YYTH_FLAGS_OOB_TRANS) {
> * if (hsize < 16 + 4)
> * goto fail;
> - * dfa->max_oob = ntol(*(__be32 *) (data + 16));
> + * dfa->max_oob = get_unaligned_be32(data + 16);
> * if (dfa->max <= MAX_OOB_SUPPORTED) {
> * pr_err("AppArmor DFA OOB greater than supported\n");
> * goto fail;

Adrian

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