On Wed, Aug 19, 2026 at 2:38 AM Maxime Bélair via AppArmor
<[email protected]> wrote:
>
> unpack_strs_table() stores each entry into the table only after
> validating it, so an entry the validation rejects is never freed. Up to
> 64KB leaks per rejected entry.
>
> Store the pointer before validating it.
>
> Fixes: c140dcd1246b ("apparmor: make str table more generic and be able to
> have multiple entries")
> Signed-off-by: Maxime Bélair <[email protected]>
> ---
> security/apparmor/policy_unpack.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/policy_unpack.c
> b/security/apparmor/policy_unpack.c
> index f1fc48e72d0e..99bc4f946d15 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -558,6 +558,9 @@ static int unpack_strs_table(struct aa_ext *e, const char
> *name, bool multi,
> /* aa_unpack_strdup verifies that the last character
> is
> * null termination byte.
> */
There is a pre-existing problem that aa_unpack_strdup might place a
NULL pointer into str: while the size guard in process_strs_entry
prevents the NULL pointer from actually being dereferenced, this is
still somewhat fragile, and a comment might be warranted here as a
separate patch.
> + /* publish before validating; the error path frees it
> */
> + table[i].strs = str;
> + table[i].size = size2;
> c = process_strs_entry(str, size2, multi);
> if (c <= 0) {
> AA_DEBUG(DEBUG_UNPACK, "process_strs %d i %d
> pos %ld",
> @@ -570,9 +573,7 @@ static int unpack_strs_table(struct aa_ext *e, const char
> *name, bool multi,
> /* fail - all other cases with embedded \0 */
> goto fail;
> }
> - table[i].strs = str;
> table[i].count = c;
> - table[i].size = size2;
> }
> if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
> goto fail;
> --
> 2.51.0
>
>
The patch itself LGTM, so:
Reviewed-by: Ryan Lee <[email protected]>
