| Message ID | 1291414464-6518-2-git-send-email-john.johansen@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Brad Figg |
| Headers | show |
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@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@canonical.com>
On 12/06/2010 11:49 AM, Tim Gardner wrote: > 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@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@canonical.com> > Acked-by: Brad Figg <brad.figg@canonical.com> Applied to Lucid master-next.
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++) {
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@canonical.com> --- security/apparmor/match.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)