Re: [PATCH] apparmor: avoid potential double free when remapping DFA tables

[John Johansen <john.johansen@canonical.com>]

On 10/7/26 20:15, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> When unpacking a DFA, aadfaunpack() stores a table in dfa->tables
> before attempting to remap a 16-bit table to 32-bit.
> 
> However, remapdata16todata32() frees the original table when
> allocation of the new table fails. This leaves the corresponding
> dfa->tables entry pointing to freed memory. If the unpacking then
> fails, dfafree() attempts to free the same table again.
> 
> Only publish the table to dfa->tables after any required remapping
> has completed successfully, so that dfa->tables never contains a
> pointer to a table that has already been freed.
> 
> Fixes: 9208c05f9fdf ("apparmor: add support for 2^24 states to the dfa state machine.")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Acked-by: John Johansen <john.johansen@canonical.com>

I have pulled this into the apparmor tree

> ---
>   security/apparmor/match.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index 7713484..ca30a42 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -397,7 +397,6 @@ struct aa_dfa *aa_dfa_unpack(const void *blob, size_t size, int flags)
>   		/* check for duplicate table entry */
>   		if (dfa->tables[table->td_id])
>   			goto fail;
> -		dfa->tables[table->td_id] = table;
>   		data += table_size(table->td_lolen, table->td_flags);
>   		size -= table_size(table->td_lolen, table->td_flags);
>   
> @@ -414,9 +413,9 @@ struct aa_dfa *aa_dfa_unpack(const void *blob, size_t size, int flags)
>   				if (!table)
>   					goto fail;
>   			}
> -			dfa->tables[table->td_id] = table;
>   			break;
>   		}
> +		dfa->tables[table->td_id] = table;
>   		table = NULL;
>   	}
>   	error = verify_table_headers(dfa->tables, flags);