Commit 18fc3b79 authored by Charlie Jacobsen's avatar Charlie Jacobsen Committed by Vikram Narayanan

Fixes security bug: LCD could modify host's struct.

This is a trivial commit, but I want to get this fixed before
it bites us in the future.

The problem: the host kernel uses a struct to represent an
installed kernel module. But this struct is literally embedded
in the module's program bits. We're mapping the entire module
inside the LCD. Thus, the LCD can do whatever it wants to the
struct, and this could affect the host.

Yes, all of the other parts of the module are technically exposed
to the host. But it's important we isolate this struct because this
is something the host will actually touch.

The solution: I duplicate the page that contains the struct, so
that the LCD has a separate copy of the struct.

Open issues: we still need to come up with a solution for an LCD
to pass its struct module as an argument, via rpc. I'm going to
hack it for now with PMFS.
parent bd924ac1
......@@ -34,6 +34,7 @@ int klcd_page_alloc(cptr_t *slot_out, gpa_t gpa);
int klcd_pages_alloc(cptr_t *slots_out, hpa_t *hp_base_out,
hva_t *hv_base_out, unsigned order);
int klcd_gfp(cptr_t *slot_out, gpa_t *gpa_out, gva_t *gva_out);
int klcd_free_page(cptr_t page);
int klcd_create_sync_endpoint(cptr_t *slot_out);
int klcd_send(cptr_t endpoint);
int klcd_recv(cptr_t endpoint);
......@@ -207,6 +208,15 @@ static inline int lcd_gfp(cptr_t *slot_out, gpa_t *gpa_out, gva_t *gva_out)
return klcd_gfp(slot_out, gpa_out, gva_out);
}
/**
* Free a page allocated with lcd_gfp.
*/
static inline int lcd_free_page(cptr_t page)
{
return klcd_free_page(page);
}
/* IPC -------------------------------------------------- */
/**
......
......@@ -132,14 +132,70 @@ fail:
return;
}
/* Helper used by get_module_pages. */
static int page_contains(hva_t page_hva, void *protected_ptr)
{
return (hva_val(page_hva) ==
(((unsigned long)(protected_ptr)) & PAGE_MASK));
}
/* Helper used by get_module_pages. */
static int dup_page(hva_t hva_src, cptr_t *page_cptr)
{
gpa_t gpa;
gva_t gva;
hva_t hva_dest;
int ret;
/*
* Allocate a new page
*/
ret = lcd_gfp(page_cptr, &gpa, &gva);
if (ret)
goto fail1;
/*
* For the host, gva == hva
*/
hva_dest = __hva(gva_val(gva));
/*
* Copy over data
*/
memcpy(hva2va(hva_dest), hva2va(hva_src), PAGE_SIZE);
return 0;
fail1:
return ret;
}
/**
* Adds the kernel module's pages to the caller's cspace,
* using klcd_add_page. Creates a list of metadata that
* represent the pages that make up the module.
*
* There is one exception: when protected_data is non-null, we check
* to see which module page it belongs to. For *this page*,
* we make a *copy*.
*
* Motivation: The struct module for a kernel module is allocated
* in the module's core. We don't want the host and the LCD
* to use the same copy. So, we make a copy of the page that
* contains the struct module. The host and LCD will now use
* their own copies.
*
* Fortunately, the page we alloc to do the copy is under
* capability access control, and when the caller deletes the
* last capability, the page will be freed.
*/
static int get_module_pages(hva_t hva, unsigned long size,
struct list_head *mpage_list)
struct list_head *mpage_list,
void *protected_data)
{
int ret;
unsigned long mapped;
struct page *p;
cptr_t pg_cptr;
struct lcd_module_page *mp;
int did_dup = 0;
mapped = 0;
while (mapped < size) {
......@@ -148,11 +204,29 @@ static int get_module_pages(hva_t hva, unsigned long size,
*/
p = vmalloc_to_page(hva2va(hva));
/*
* Add page to klcd
* Check if need to make a copy of the page
*/
ret = klcd_add_page(p, &pg_cptr);
if (ret)
goto fail1;
if (protected_data && page_contains(hva, protected_data)) {
/*
* Yes: the protected data is in this page.
*
* Make a copy
*/
ret = dup_page(hva, &pg_cptr);
if (ret)
goto fail1;
did_dup = 1;
} else {
/*
* No: the protected data is not in this page.
*
* Just add the page to caller's cspace
*/
ret = klcd_add_page(p, &pg_cptr);
if (ret)
goto fail1;
did_dup = 0;
}
/*
* Record in list of pages
*/
......@@ -171,11 +245,19 @@ static int get_module_pages(hva_t hva, unsigned long size,
*/
mapped += PAGE_SIZE;
hva = hva_add(hva, PAGE_SIZE);
did_dup = 0;
}
return 0;
fail2:
klcd_rm_page(pg_cptr);
/* Differentiating the two doesn't matter right now, but it
* may in the future. (In both cases, we just delete the capability
* and free the slot in the cptr cache.) */
if (did_dup)
lcd_free_page(pg_cptr);
else
klcd_rm_page(pg_cptr);
fail1:
return ret; /* caller will free lcd_module_page's, etc. */
}
......@@ -294,12 +376,14 @@ int klcd_load_module(char *mname, cptr_t mloader_endpoint,
* Get init and core pages
*/
ret = get_module_pages(va2hva(m->module_init),
m->init_size, &(*mi)->mpages_list);
m->init_size, &(*mi)->mpages_list,
NULL);
if (ret) {
goto fail2;
}
ret = get_module_pages(va2hva(m->module_core),
m->core_size, &(*mi)->mpages_list);
m->core_size, &(*mi)->mpages_list,
m);
if (ret) {
goto fail3;
}
......
......@@ -146,9 +146,24 @@ fail1:
return ret;
}
int klcd_free_page(cptr_t page)
{
/*
* Delete from cspace
*/
lcd_cap_delete(page);
/*
* Free the slot
*/
lcd_free_cptr(page);
return 0;
}
/* EXPORTS -------------------------------------------------- */
EXPORT_SYMBOL(klcd_add_page);
EXPORT_SYMBOL(klcd_rm_page);
EXPORT_SYMBOL(klcd_page_alloc);
EXPORT_SYMBOL(klcd_gfp);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment