| Message ID | 1271142580-26555-9-git-send-email-john.johansen@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Andy Whitcroft |
| Headers | show |
On Tue, Apr 13, 2010 at 12:09:37AM -0700, john.johansen@canonical.com wrote: > From: John Johansen <john.johansen@canonical.com> > > OriginalAuthor: John Johansen <john.johansen@canonical.com> > OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$ > commit: 6857acf643ba19eddfc29125fc011a3ce48fe87b > BugLink: http://bugs.launchpad.net/bugs/529288 > > vmalloc doesn't guarentee that the tlbs of all cpus will be flushed > when it completes. Instead the tlbs gets flushed lazily, however for > AppArmor this is a problem as the dfa becomes live to all cpus as > soon as the profile replacedby value is written (this is even before > locking of the lists are removed). > > It is possible for another cpu to be in a state where it has an old > tlb mapping for the vmalloc address (this will be caused by putting > a reference on an old profile while replacing to the current), > so that it references to the wrong memory location when doing dfa > lookups. > > Replacement is not a common operation so make sure all memory > aliases are removed before the dfa goes live. > > Signed-off-by: John Johansen <john.johansen@canonical.com> > --- > security/apparmor/match.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/security/apparmor/match.c b/security/apparmor/match.c > index 5a55959..afc2dd2 100644 > --- a/security/apparmor/match.c > +++ b/security/apparmor/match.c > @@ -50,6 +50,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize) > { > struct table_header *table = NULL; > struct table_header th; > + int unmap_alias = 0; > size_t tsize; > > if (bsize < sizeof(struct table_header)) > @@ -73,8 +74,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize) > > /* freed by free_table */ > table = kmalloc(tsize, GFP_KERNEL | __GFP_NOWARN); > - if (!table) > + if (!table) { > + unmap_alias = 1; > table = vmalloc(tsize); It is possible this could be below more optimally: table = vmalloc(tsize); if (table) unmap_alias = 1 > + } > if (table) { > *table = th; > if (th.td_flags == YYTD_DATA8) > @@ -91,6 +94,8 @@ static struct table_header *unpack_table(char *blob, size_t bsize) > } > > out: > + if (unmap_alias) > + vm_unmap_aliases(); > return table; > fail: > free_table(table); Even without the above it seems sane and necessary: Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 5a55959..afc2dd2 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -50,6 +50,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize) { struct table_header *table = NULL; struct table_header th; + int unmap_alias = 0; size_t tsize; if (bsize < sizeof(struct table_header)) @@ -73,8 +74,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize) /* freed by free_table */ table = kmalloc(tsize, GFP_KERNEL | __GFP_NOWARN); - if (!table) + if (!table) { + unmap_alias = 1; table = vmalloc(tsize); + } if (table) { *table = th; if (th.td_flags == YYTD_DATA8) @@ -91,6 +94,8 @@ static struct table_header *unpack_table(char *blob, size_t bsize) } out: + if (unmap_alias) + vm_unmap_aliases(); return table; fail: free_table(table);