[PATCH 1/2] AppArmor: fix the upper bound check for the next/check table

Tim Gardner tim.gardner at canonical.com
Mon Dec 6 19:49:14 UTC 2010


On 12/03/2010 03:14 PM, John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/581525
>
> SRU Justification (apparmor)
>
>     impact of the bug is medium for stable releases. There are two parts to
>     this bug: the kernel side OOPSing when a the parser generates invalid
>     tables, and the parser generating correct tables. The lucid kernel should
>     receive the fix sometime in the future, but the userspace should also be
>     fixed.
>
>     The kernel bug was a broken test in verifying the dfa next/check table
>     size (so the userspace bug was not caught when it should have been). This
>     means that it can at times reference beyond the dfa table (by at most 255
>     entries).
>
>     The userspace bug is that the next/check table is not correctly padded
>     with 0 entries, so that it is impossible to reference beyond the end of
>     the table when in the states that use the end of the table for their
>     references.
>
> The next/check table needs to be>= largest base value + 255 (byte range
> being 0..255) to avoid any possible bounds violations.  Fix the test which
> incorrectly was testing that the next/check table + 256<= base values.
>
> Signed-off-by: John Johansen<john.johansen at canonical.com>
> ---
>   security/apparmor/match.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index a3730e2..b456495 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -177,8 +177,10 @@ static int verify_dfa(struct aa_dfa *dfa, int flags)
>   			if (DEFAULT_TABLE(dfa)[i]>= state_count)
>   				goto out;
>   			/* TODO: do check that DEF state recursion terminates */
> -			if (BASE_TABLE(dfa)[i]>= trans_count + 256)
> +			if (BASE_TABLE(dfa)[i] + 255>= trans_count) {
> +				printk("AppArmor DFA next/check upper bounds error\n");
>   				goto out;
> +			}
>   		}
>
>   		for (i = 0; i<  trans_count; i++) {

Matches the upstream code.

Acked-by: Tim Gardner <tim.gardner at canonical.com>

-- 
Tim Gardner tim.gardner at canonical.com




More information about the kernel-team mailing list