The first and third patches are simple cleanup; #1 does fix null function pointer dereferences if you compile with XSM but don''t use it. The second patch addresses the issue partially fixed by changeset 25605:9950f2dc2ee6; XSM hooks were incorrectly assuming a number of things about the presence and validity of struct page_info fields while validating domU-provided MFN values. This patch discards these checks and uses the pg_owner domain to validate access to other domain''s pages. This series supersedes the two patches sent on 2012-07-31 that applied the incomplete fix to flask_mmu_machphys_update; patch #1 here is identical to that post''s #2. [PATCH 1/3] xsm: Add missing dummy hooks [PATCH 2/3] xsm/flask: remove page-to-domain lookups from XSM hooks [PATCH 3/3] flask/policy: add accesses used by newer dom0s
A few XSM hooks have been defined without implementation in dummy.c; these will cause a null function pointer deference if called. Also implement the efi_call hook, which was incorrectly added without any implementations. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- xen/xsm/dummy.c | 30 ++++++++++++++++++++++++++++++ xen/xsm/flask/hooks.c | 6 ++++++ 2 files changed, 36 insertions(+) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 7027ee7..5d35342 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -295,6 +295,21 @@ static char *dummy_show_security_evtchn (struct domain *d, const struct evtchn * return NULL; } +static int dummy_get_pod_target(struct domain *d) +{ + return 0; +} + +static int dummy_set_pod_target(struct domain *d) +{ + return 0; +} + +static int dummy_get_device_group (uint32_t machine_bdf) +{ + return 0; +} + static int dummy_test_assign_device (uint32_t machine_bdf) { return 0; @@ -503,6 +518,11 @@ static int dummy_firmware_info (void) return 0; } +static int dummy_efi_call(void) +{ + return 0; +} + static int dummy_acpi_sleep (void) { return 0; @@ -565,6 +585,11 @@ static int dummy_bind_pt_irq (struct domain *d, struct xen_domctl_bind_pt_irq *b return 0; } +static int dummy_unbind_pt_irq (struct domain *d) +{ + return 0; +} + static int dummy_pin_mem_cacheattr (struct domain *d) { return 0; @@ -652,6 +677,8 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, alloc_security_evtchn); set_to_dummy_if_null(ops, free_security_evtchn); set_to_dummy_if_null(ops, show_security_evtchn); + set_to_dummy_if_null(ops, get_pod_target); + set_to_dummy_if_null(ops, set_pod_target); set_to_dummy_if_null(ops, memory_adjust_reservation); set_to_dummy_if_null(ops, memory_stat_reservation); @@ -670,6 +697,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, iomem_permission); set_to_dummy_if_null(ops, pci_config_permission); + set_to_dummy_if_null(ops, get_device_group); set_to_dummy_if_null(ops, test_assign_device); set_to_dummy_if_null(ops, assign_device); set_to_dummy_if_null(ops, deassign_device); @@ -711,6 +739,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, physinfo); set_to_dummy_if_null(ops, platform_quirk); set_to_dummy_if_null(ops, firmware_info); + set_to_dummy_if_null(ops, efi_call); set_to_dummy_if_null(ops, acpi_sleep); set_to_dummy_if_null(ops, change_freq); set_to_dummy_if_null(ops, getidletime); @@ -723,6 +752,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, remove_from_physmap); set_to_dummy_if_null(ops, sendtrigger); set_to_dummy_if_null(ops, bind_pt_irq); + set_to_dummy_if_null(ops, unbind_pt_irq); set_to_dummy_if_null(ops, pin_mem_cacheattr); set_to_dummy_if_null(ops, ext_vcpucontext); set_to_dummy_if_null(ops, vcpuextstate); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 23b84f3..de79d66 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1280,6 +1280,11 @@ static int flask_firmware_info(void) return domain_has_xen(current->domain, XEN__FIRMWARE); } +static int flask_efi_call(void) +{ + return domain_has_xen(current->domain, XEN__FIRMWARE); +} + static int flask_acpi_sleep(void) { return domain_has_xen(current->domain, XEN__SLEEP); @@ -1663,6 +1668,7 @@ static struct xsm_operations flask_ops = { .physinfo = flask_physinfo, .platform_quirk = flask_platform_quirk, .firmware_info = flask_firmware_info, + .efi_call = flask_efi_call, .acpi_sleep = flask_acpi_sleep, .change_freq = flask_change_freq, .getidletime = flask_getidletime, -- 1.7.11.2
Daniel De Graaf
2012-Aug-17 16:23 UTC
[PATCH 2/3] xsm/flask: remove page-to-domain lookups from XSM hooks
Doing a reverse lookup from MFN to its owning domain is redundant with the internal checks Xen does on pages. Change the checks to operate directly on the domain owning the pages for normal memory; MMIO areas are still checked with security_iomem_sid. This fixes a hypervisor crash when a domU attempts to map an MFN that is free in Xen''s heap: the XSM hook is called before the validity check, and page_get_owner returns garbage when called on these pages. While explicitly checking for such pages using page_get_owner_and_reference is a possible solution, this ends up duplicating parts of get_page_from_l1e. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- xen/arch/x86/domctl.c | 23 +++--- xen/arch/x86/mm.c | 4 +- xen/include/xsm/xsm.h | 23 +++--- xen/xsm/dummy.c | 6 +- xen/xsm/flask/hooks.c | 189 +++++++++++++------------------------------------- 5 files changed, 80 insertions(+), 165 deletions(-) diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 135ea6e..97a13fb 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -114,7 +114,7 @@ long arch_do_domctl( page = mfn_to_page(mfn); - ret = xsm_getpageframeinfo(page); + ret = xsm_getpageframeinfo(d); if ( ret ) { rcu_unlock_domain(d); @@ -170,6 +170,13 @@ long arch_do_domctl( if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) ) break; + ret = xsm_getpageframeinfo(d); + if ( ret ) + { + rcu_unlock_domain(d); + break; + } + if ( unlikely(num > 1024) || unlikely(num != domctl->u.getpageframeinfo3.num) ) { @@ -209,8 +216,6 @@ long arch_do_domctl( if ( unlikely(!page) || unlikely(is_xen_heap_page(page)) ) type = XEN_DOMCTL_PFINFO_XTAB; - else if ( xsm_getpageframeinfo(page) != 0 ) - ; else { switch( page->u.inuse.type_info & PGT_type_mask ) @@ -267,6 +272,13 @@ long arch_do_domctl( if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) ) break; + ret = xsm_getpageframeinfo(d); + if ( ret ) + { + rcu_unlock_domain(d); + break; + } + if ( unlikely(num > 1024) ) { ret = -E2BIG; @@ -310,11 +322,6 @@ long arch_do_domctl( if ( unlikely(!page) || unlikely(is_xen_heap_page(page)) ) arr32[j] |= XEN_DOMCTL_PFINFO_XTAB; - else if ( xsm_getpageframeinfo(page) != 0 ) - { - put_page(page); - continue; - } else { unsigned long type = 0; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 84820f1..d02fe97 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -3073,7 +3073,7 @@ long do_mmuext_op( break; } - if ( (rc = xsm_memory_pin_page(d, page)) != 0 ) + if ( (rc = xsm_memory_pin_page(d, pg_owner, page)) != 0 ) { put_page_and_type(page); okay = 0; @@ -3643,7 +3643,7 @@ long do_mmu_update( mfn = req.ptr >> PAGE_SHIFT; gpfn = req.val; - rc = xsm_mmu_machphys_update(d, mfn); + rc = xsm_mmu_machphys_update(d, pg_owner, mfn); if ( rc ) break; diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index bef79df..593cdbd 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -105,7 +105,7 @@ struct xsm_operations { int (*set_pod_target) (struct domain *d); int (*memory_adjust_reservation) (struct domain *d1, struct domain *d2); int (*memory_stat_reservation) (struct domain *d1, struct domain *d2); - int (*memory_pin_page) (struct domain *d, struct page_info *page); + int (*memory_pin_page) (struct domain *d1, struct domain *d2, struct page_info *page); int (*remove_from_physmap) (struct domain *d1, struct domain *d2); int (*console_io) (struct domain *d, int cmd); @@ -143,7 +143,7 @@ struct xsm_operations { #ifdef CONFIG_X86 int (*shadow_control) (struct domain *d, uint32_t op); - int (*getpageframeinfo) (struct page_info *page); + int (*getpageframeinfo) (struct domain *d); int (*getmemlist) (struct domain *d); int (*hypercall_init) (struct domain *d); int (*hvmcontext) (struct domain *d, uint32_t op); @@ -171,9 +171,8 @@ struct xsm_operations { int (*domain_memory_map) (struct domain *d); int (*mmu_normal_update) (struct domain *d, struct domain *t, struct domain *f, intpte_t fpte); - int (*mmu_machphys_update) (struct domain *d, unsigned long mfn); - int (*update_va_mapping) (struct domain *d, struct domain *f, - l1_pgentry_t pte); + int (*mmu_machphys_update) (struct domain *d1, struct domain *d2, unsigned long mfn); + int (*update_va_mapping) (struct domain *d, struct domain *f, l1_pgentry_t pte); int (*add_to_physmap) (struct domain *d1, struct domain *d2); int (*sendtrigger) (struct domain *d); int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); @@ -455,9 +454,10 @@ static inline int xsm_memory_stat_reservation (struct domain *d1, return xsm_call(memory_stat_reservation(d1, d2)); } -static inline int xsm_memory_pin_page(struct domain *d, struct page_info *page) +static inline int xsm_memory_pin_page(struct domain *d1, struct domain *d2, + struct page_info *page) { - return xsm_call(memory_pin_page(d, page)); + return xsm_call(memory_pin_page(d1, d2, page)); } static inline int xsm_remove_from_physmap(struct domain *d1, struct domain *d2) @@ -617,9 +617,9 @@ static inline int xsm_shadow_control (struct domain *d, uint32_t op) return xsm_call(shadow_control(d, op)); } -static inline int xsm_getpageframeinfo (struct page_info *page) +static inline int xsm_getpageframeinfo (struct domain *d) { - return xsm_call(getpageframeinfo(page)); + return xsm_call(getpageframeinfo(d)); } static inline int xsm_getmemlist (struct domain *d) @@ -753,9 +753,10 @@ static inline int xsm_mmu_normal_update (struct domain *d, struct domain *t, return xsm_call(mmu_normal_update(d, t, f, fpte)); } -static inline int xsm_mmu_machphys_update (struct domain *d, unsigned long mfn) +static inline int xsm_mmu_machphys_update (struct domain *d1, struct domain *d2, + unsigned long mfn) { - return xsm_call(mmu_machphys_update(d, mfn)); + return xsm_call(mmu_machphys_update(d1, d2, mfn)); } static inline int xsm_update_va_mapping(struct domain *d, struct domain *f, diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 5d35342..4836fc0 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -243,7 +243,7 @@ static int dummy_schedop_shutdown (struct domain *d1, struct domain *d2) return 0; } -static int dummy_memory_pin_page(struct domain *d, struct page_info *page) +static int dummy_memory_pin_page(struct domain *d1, struct domain *d2, struct page_info *page) { return 0; } @@ -418,7 +418,7 @@ static int dummy_shadow_control (struct domain *d, uint32_t op) return 0; } -static int dummy_getpageframeinfo (struct page_info *page) +static int dummy_getpageframeinfo (struct domain *d) { return 0; } @@ -554,7 +554,7 @@ static int dummy_mmu_normal_update (struct domain *d, struct domain *t, return 0; } -static int dummy_mmu_machphys_update (struct domain *d, unsigned long mfn) +static int dummy_mmu_machphys_update (struct domain *d, struct domain *f, unsigned long mfn) { return 0; } diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index de79d66..8c853de 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -108,15 +108,21 @@ static int flask_domain_alloc_security(struct domain *d) memset(dsec, 0, sizeof(struct domain_security_struct)); - if ( is_idle_domain(d) ) + dsec->create_sid = SECSID_NULL; + switch ( d->domain_id ) { + case DOMID_IDLE: dsec->sid = SECINITSID_XEN; dsec->create_sid = SECINITSID_DOM0; - } - else - { + break; + case DOMID_XEN: + dsec->sid = SECINITSID_DOMXEN; + break; + case DOMID_IO: + dsec->sid = SECINITSID_DOMIO; + break; + default: dsec->sid = SECINITSID_UNLABELED; - dsec->create_sid = SECSID_NULL; } d->ssid = dsec; @@ -361,64 +367,6 @@ static int flask_grant_query_size(struct domain *d1, struct domain *d2) return domain_has_perm(d1, d2, SECCLASS_GRANT, GRANT__QUERY); } -static int get_page_sid(struct page_info *page, u32 *sid) -{ - int rc = 0; - struct domain *d; - struct domain_security_struct *dsec; - unsigned long mfn; - - d = page_get_owner(page); - - if ( d == NULL ) - { - mfn = page_to_mfn(page); - rc = security_iomem_sid(mfn, sid); - return rc; - } - - switch ( d->domain_id ) - { - case DOMID_IO: - /*A tracked IO page?*/ - *sid = SECINITSID_DOMIO; - break; - - case DOMID_XEN: - /*A page from Xen''s private heap?*/ - *sid = SECINITSID_DOMXEN; - break; - - default: - /*Pages are implicitly labeled by domain ownership!*/ - dsec = d->ssid; - *sid = dsec ? dsec->sid : SECINITSID_UNLABELED; - break; - } - - return rc; -} - -static int get_mfn_sid(unsigned long mfn, u32 *sid) -{ - int rc = 0; - struct page_info *page; - - if ( mfn_valid(mfn) ) - { - /*mfn is valid if this is a page that Xen is tracking!*/ - page = mfn_to_page(mfn); - rc = get_page_sid(page, sid); - } - else - { - /*Possibly an untracked IO page?*/ - rc = security_iomem_sid(mfn, sid); - } - - return rc; -} - static int flask_get_pod_target(struct domain *d) { return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, DOMAIN__GETPODTARGET); @@ -439,18 +387,10 @@ static int flask_memory_stat_reservation(struct domain *d1, struct domain *d2) return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__STAT); } -static int flask_memory_pin_page(struct domain *d, struct page_info *page) +static int flask_memory_pin_page(struct domain *d1, struct domain *d2, + struct page_info *page) { - int rc = 0; - u32 sid; - struct domain_security_struct *dsec; - dsec = d->ssid; - - rc = get_page_sid(page, &sid); - if ( rc ) - return rc; - - return avc_has_perm(dsec->sid, sid, SECCLASS_MMU, MMU__PINPAGE, NULL); + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PINPAGE); } static int flask_console_io(struct domain *d, int cmd) @@ -1095,19 +1035,9 @@ static int flask_ioport_permission(struct domain *d, uint32_t start, uint32_t en return security_iterate_ioport_sids(start, end, _ioport_has_perm, &data); } -static int flask_getpageframeinfo(struct page_info *page) +static int flask_getpageframeinfo(struct domain *d) { - int rc = 0; - u32 tsid; - struct domain_security_struct *dsec; - - dsec = current->domain->ssid; - - rc = get_page_sid(page, &tsid); - if ( rc ) - return rc; - - return avc_has_perm(dsec->sid, tsid, SECCLASS_MMU, MMU__PAGEINFO, NULL); + return domain_has_perm(current->domain, d, SECCLASS_MMU, MMU__PAGEINFO); } static int flask_getmemlist(struct domain *d) @@ -1314,88 +1244,65 @@ static int flask_domain_memory_map(struct domain *d) return domain_has_perm(current->domain, d, SECCLASS_MMU, MMU__MEMORYMAP); } -static int flask_mmu_normal_update(struct domain *d, struct domain *t, - struct domain *f, intpte_t fpte) +static int domain_memory_perm(struct domain *d, struct domain *f, l1_pgentry_t pte) { int rc = 0; u32 map_perms = MMU__MAP_READ; unsigned long fgfn, fmfn; - struct domain_security_struct *dsec; - u32 fsid; - struct avc_audit_data ad; p2m_type_t p2mt; - if (d != t) - rc = domain_has_perm(d, t, SECCLASS_MMU, MMU__REMOTE_REMAP); - if ( rc ) - return rc; - - if ( !(l1e_get_flags(l1e_from_intpte(fpte)) & _PAGE_PRESENT) ) + if ( !(l1e_get_flags(pte) & _PAGE_PRESENT) ) return 0; - dsec = d->ssid; - - if ( l1e_get_flags(l1e_from_intpte(fpte)) & _PAGE_RW ) + if ( l1e_get_flags(pte) & _PAGE_RW ) map_perms |= MMU__MAP_WRITE; - AVC_AUDIT_DATA_INIT(&ad, MEMORY); - fgfn = l1e_get_pfn(l1e_from_intpte(fpte)); + fgfn = l1e_get_pfn(pte); fmfn = mfn_x(get_gfn_query(f, fgfn, &p2mt)); - - ad.sdom = d; - ad.tdom = f; - ad.memory.pte = fpte; - ad.memory.mfn = fmfn; - - rc = get_mfn_sid(fmfn, &fsid); - put_gfn(f, fgfn); - if ( rc ) - return rc; + if ( f->domain_id == DOMID_IO || !mfn_valid(fmfn) ) + { + struct avc_audit_data ad; + struct domain_security_struct *dsec = d->ssid; + u32 fsid; + AVC_AUDIT_DATA_INIT(&ad, MEMORY); + ad.sdom = d; + ad.tdom = f; + ad.memory.pte = pte.l1; + ad.memory.mfn = fmfn; + rc = security_iomem_sid(fmfn, &fsid); + if ( rc ) + return rc; + return avc_has_perm(dsec->sid, fsid, SECCLASS_MMU, map_perms, &ad); + } - return avc_has_perm(dsec->sid, fsid, SECCLASS_MMU, map_perms, &ad); + return domain_has_perm(d, f, SECCLASS_MMU, map_perms); } -static int flask_mmu_machphys_update(struct domain *d, unsigned long mfn) +static int flask_mmu_normal_update(struct domain *d, struct domain *t, + struct domain *f, intpte_t fpte) { int rc = 0; - u32 psid; - struct domain_security_struct *dsec; - dsec = d->ssid; - rc = get_mfn_sid(mfn, &psid); + if (d != t) + rc = domain_has_perm(d, t, SECCLASS_MMU, MMU__REMOTE_REMAP); if ( rc ) return rc; - return avc_has_perm(dsec->sid, psid, SECCLASS_MMU, MMU__UPDATEMP, NULL); + return domain_memory_perm(d, f, l1e_from_intpte(fpte)); +} + +static int flask_mmu_machphys_update(struct domain *d1, struct domain *d2, + unsigned long mfn) +{ + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__UPDATEMP); } static int flask_update_va_mapping(struct domain *d, struct domain *f, l1_pgentry_t pte) { - int rc = 0; - u32 psid; - u32 map_perms = MMU__MAP_READ; - struct page_info *page = NULL; - struct domain_security_struct *dsec; - - if ( !(l1e_get_flags(pte) & _PAGE_PRESENT) ) - return 0; - - if ( l1e_get_flags(pte) & _PAGE_RW ) - map_perms |= MMU__MAP_WRITE; - - dsec = d->ssid; - - page = get_page_from_gfn(f, l1e_get_pfn(pte), NULL, P2M_ALLOC); - rc = get_mfn_sid(page ? page_to_mfn(page) : INVALID_MFN, &psid); - if ( page ) - put_page(page); - if ( rc ) - return rc; - - return avc_has_perm(dsec->sid, psid, SECCLASS_MMU, map_perms, NULL); + return domain_memory_perm(d, f, pte); } static int flask_add_to_physmap(struct domain *d1, struct domain *d2) -- 1.7.11.2
Daniel De Graaf
2012-Aug-17 16:23 UTC
[PATCH 3/3] flask/policy: add accesses used by newer dom0s
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- tools/flask/policy/policy/modules/xen/xen.if | 2 +- tools/flask/policy/policy/modules/xen/xen.te | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if index 87ef165..3f58909 100644 --- a/tools/flask/policy/policy/modules/xen/xen.if +++ b/tools/flask/policy/policy/modules/xen/xen.if @@ -100,7 +100,7 @@ define(`use_device'', ` # admin_device(domain, device) # Allow a device to be used and delegated by a domain define(`admin_device'', ` - allow $1 $2:resource { setup stat_device add_device add_irq add_iomem add_ioport remove_device remove_irq remove_iomem remove_ioport }; + allow $1 $2:resource { setup stat_device add_device add_irq add_iomem add_ioport remove_device remove_irq remove_iomem remove_ioport plug unplug }; allow $1 $2:hvm bind_irq; use_device($1, $2) '') diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te index 29885c4..e175d4b 100644 --- a/tools/flask/policy/policy/modules/xen/xen.te +++ b/tools/flask/policy/policy/modules/xen/xen.te @@ -55,8 +55,8 @@ type device_t, resource_type; allow xen_t dom0_t:domain { create }; allow dom0_t xen_t:xen { kexec readapic writeapic mtrr_read mtrr_add mtrr_del - scheduler physinfo heap quirk readconsole writeconsole settime - microcode cpupool_op sched_op }; + scheduler physinfo heap quirk readconsole writeconsole settime getcpuinfo + microcode cpupool_op sched_op pm_op }; allow dom0_t xen_t:mmu { memorymap }; allow dom0_t security_t:security { check_context compute_av compute_create compute_member load_policy compute_relabel compute_user setenforce -- 1.7.11.2