On 8/19/26 20:52, Ryan Lee via AppArmor wrote:
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]>
I pulled this into the apparmor tree
--- 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.0The patch itself LGTM, so: Reviewed-by: Ryan Lee <[email protected]>
