[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Fri, 2013-01-11 at 17:37 +0000, Daniel De Graaf wrote: > On 01/11/2013 11:46 AM, Ian Campbell wrote: > > On Fri, 2013-01-11 at 16:36 +0000, Keir Fraser wrote: > >> On 11/01/2013 16:24, "Lars Rasmusson" <lra@xxxxxxx> wrote: > >> > >>>> On 11/01/2013 13:32, "lra@xxxxxxx" <lra@xxxxxxx> wrote: > >>>> > >>>>> From: Lars Rasmusson <Lars.Rasmusson@xxxxxxx> > >>>>> > >>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx> > >>>> > >>>> If this is a build fix after my checkins this morning then: > >>>> Acked-by: Keir Fraser <keir@xxxxxxx> > >>> > >>> Yes, the XEN_TARGET_ARCH=arm32 make breaks when compiling memory.c > >>> > >>> In xen/include/xsm/dummy.h where many of the functions are used, some are > >>> declared only for X86, so I picked the same #ifdef CONFIG_X86 > >>> as the header file uses. > >>> > >>> As Ian said, it's not pretty, but since ARM doesn't have xsm (yet?) I > >>> think > >>> adding a dummy xsm_remove_from_physmap for arm also is ugly. > >>> > >>> Is there some other way to write memory.c so that it doesn't need > >>> xsm_remove...? (I mean, it does't need xsm_add....) > >> > >> The XSM infrastructure is not architecture dependent. It's probably a > >> mistake that xsm_remove_from_physmap() is ifdef CONFIG_X86. > > > > Agreed. > > > > 8<--------------------- > > > >>From fb57be285e956cadea51dc48a28fba77d752044d Mon Sep 17 00:00:00 2001 > > From: Ian Campbell <ian.campbell@xxxxxxxxxx> > > Date: Fri, 11 Jan 2013 16:44:14 +0000 > > Subject: [PATCH] xen arm: add XSM hooks to arch_memory_op > > > > Treat XENMEM_add_to_physmap_range the same as > > XENMEM_add_to_physmap. > > > > Reported-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx> > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > > Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> > > --- > > xen/arch/arm/mm.c | 13 +++++++++++++ > > xen/include/xsm/dummy.h | 24 ++++++++++++------------ > > 2 files changed, 25 insertions(+), 12 deletions(-) > > I agree with the movement (neither of these hooks are x86-specific), but > this patch also needs to move the hooks in xen/include/xsm/xsm.h and > xen/xsm/flask/hooks.c out of the same #ifdefs. Thanks, and dummy.c. Turns out this is far from the only barrier to enabling FLASK on ARM, flask_op.c has a few x86isms not included in #ifdef, e.g. MSIs I also needed a few extra #includes on ARM for things which are picked up indirectly on x86. The patch is below, its a bit ugly and totally untested. I could pick out the bits to do the rigth thing on ARM without FLASK on if that would be preferred. > On a mostly unrelated note, Ian: do you think it would be useful to add > build tests with XSM enabled to catch these types of issues during the > automated testing? It just requires "echo XSM_ENABLE=y > .config" > before the build, and is apparently best done for both x86 and arm. In general I think build tests with a variety of different compile time options, xsm being one of them, would be very nice to have. e.g. I'd also like to get at least a build test of ARM into the system prior to suitable hardware arriving later in the year. Perhaps a flight of build tests could be added? Ian. 8<---------------- >From 469d20054b96b8bc749891c366659532b67d6031 Mon Sep 17 00:00:00 2001 From: Ian Campbell <ian.campbell@xxxxxxxxxx> Date: Fri, 11 Jan 2013 16:44:14 +0000 Subject: [PATCH] xen arm: add XSM hooks to arch_memory_op Treat XENMEM_add_to_physmap_range the same as XENMEM_add_to_physmap. Also conditionalise more x86-isms and add required headers such that it compiles on ARM. Totally untested. Reported-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> --- xen/arch/arm/mm.c | 13 +++++++++++++ xen/include/xsm/dummy.h | 25 +++++++++++++------------ xen/include/xsm/xsm.h | 12 ++++++------ xen/xsm/dummy.c | 5 +++-- xen/xsm/flask/avc.c | 1 + xen/xsm/flask/flask_op.c | 4 +++- xen/xsm/flask/hooks.c | 44 ++++++++++++++++++++++++++++++-------------- xen/xsm/xsm_policy.c | 1 + 8 files changed, 70 insertions(+), 35 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index d97b3ea..311ec97 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -35,6 +35,7 @@ #include <asm/current.h> #include <public/memory.h> #include <xen/sched.h> +#include <xsm/xsm.h> struct domain *dom_xen, *dom_io, *dom_cow; @@ -651,6 +652,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( rc != 0 ) return rc; + if ( xsm_add_to_physmap(XSM_TARGET, current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + rc = xenmem_add_to_physmap_one(d, xatp.space, DOMID_INVALID, xatp.idx, xatp.gpfn); @@ -671,6 +678,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( rc != 0 ) return rc; + if ( xsm_add_to_physmap(XSM_TARGET, current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + rc = xenmem_add_to_physmap_range(d, &xatpr); rcu_unlock_domain(d); diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 1ca82b0..f40b196 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -16,6 +16,7 @@ */ #include <xen/sched.h> +#include <xen/errno.h> #include <xsm/xsm.h> /* Cannot use BUILD_BUG_ON here because the expressions we check are not @@ -443,6 +444,18 @@ static XSM_INLINE int xsm_pci_config_permission(XSM_DEFAULT_ARG struct domain *d return xsm_default_action(action, current->domain, d); } +static XSM_INLINE int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2) +{ + XSM_ASSERT_ACTION(XSM_TARGET); + return xsm_default_action(action, d1, d2); +} + +static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2) +{ + XSM_ASSERT_ACTION(XSM_TARGET); + return xsm_default_action(action, d1, d2); +} + #ifdef CONFIG_X86 static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op) { @@ -544,18 +557,6 @@ static XSM_INLINE int xsm_update_va_mapping(XSM_DEFAULT_ARG struct domain *d, st return xsm_default_action(action, d, f); } -static XSM_INLINE int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2) -{ - XSM_ASSERT_ACTION(XSM_TARGET); - return xsm_default_action(action, d1, d2); -} - -static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2) -{ - XSM_ASSERT_ACTION(XSM_TARGET); - return xsm_default_action(action, d1, d2); -} - static XSM_INLINE int xsm_bind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct xen_domctl_bind_pt_irq *bind) { XSM_ASSERT_ACTION(XSM_HOOK); diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 8947372..5048344 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -90,6 +90,7 @@ struct xsm_operations { 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 *d1, struct domain *d2, struct page_info *page); + int (*add_to_physmap) (struct domain *d1, struct domain *d2); int (*remove_from_physmap) (struct domain *d1, struct domain *d2); int (*console_io) (struct domain *d, int cmd); @@ -149,7 +150,6 @@ struct xsm_operations { struct domain *f, uint32_t flags); int (*mmuext_op) (struct domain *d, struct domain *f); 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 (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); int (*unbind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); int (*ioport_permission) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow); @@ -335,6 +335,11 @@ static inline int xsm_memory_pin_page(xsm_default_t def, struct domain *d1, stru return xsm_ops->memory_pin_page(d1, d2, page); } +static inline int xsm_add_to_physmap(xsm_default_t def, struct domain *d1, struct domain *d2) +{ + return xsm_ops->add_to_physmap(d1, d2); +} + static inline int xsm_remove_from_physmap(xsm_default_t def, struct domain *d1, struct domain *d2) { return xsm_ops->remove_from_physmap(d1, d2); @@ -558,11 +563,6 @@ static inline int xsm_update_va_mapping(xsm_default_t def, struct domain *d, str return xsm_ops->update_va_mapping(d, f, pte); } -static inline int xsm_add_to_physmap(xsm_default_t def, struct domain *d1, struct domain *d2) -{ - return xsm_ops->add_to_physmap(d1, d2); -} - static inline int xsm_bind_pt_irq(xsm_default_t def, struct domain *d, struct xen_domctl_bind_pt_irq *bind) { diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 529a724..5031e16 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -101,6 +101,9 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, do_xsm_op); + set_to_dummy_if_null(ops, add_to_physmap); + set_to_dummy_if_null(ops, remove_from_physmap); + #ifdef CONFIG_X86 set_to_dummy_if_null(ops, shadow_control); set_to_dummy_if_null(ops, hvm_param); @@ -118,8 +121,6 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, mmu_update); set_to_dummy_if_null(ops, mmuext_op); set_to_dummy_if_null(ops, update_va_mapping); - set_to_dummy_if_null(ops, add_to_physmap); - set_to_dummy_if_null(ops, remove_from_physmap); set_to_dummy_if_null(ops, bind_pt_irq); set_to_dummy_if_null(ops, unbind_pt_irq); set_to_dummy_if_null(ops, ioport_permission); diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c index 7fede00..f994fd9 100644 --- a/xen/xsm/flask/avc.c +++ b/xen/xsm/flask/avc.c @@ -28,6 +28,7 @@ #include <xen/rcupdate.h> #include <asm/atomic.h> #include <asm/current.h> +#include <public/event_channel.h> #include <public/xsm/flask_op.h> #include "avc.h" diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c index 4426ab9..f9b16f2 100644 --- a/xen/xsm/flask/flask_op.c +++ b/xen/xsm/flask/flask_op.c @@ -10,9 +10,11 @@ #include <xen/errno.h> #include <xen/event.h> +#include <xen/init.h> #include <xsm/xsm.h> #include <xen/guest_access.h> +#include <public/event_channel.h> #include <public/xsm/flask_op.h> #include <avc.h> @@ -71,7 +73,7 @@ static int domain_has_security(struct domain *d, u32 perms) perms, NULL); } -static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf, uint32_t size) +static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf, uint32_t size) { char *tmp = xmalloc_bytes(size + 1); if ( !tmp ) diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index ba67502..c39f129 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -19,11 +19,15 @@ #include <xen/errno.h> #include <xen/guest_access.h> #include <xen/xenoprof.h> +#ifdef CONFIG_X86 #include <asm/msi.h> +#endif +#include <asm/irq.h> #include <public/xen.h> #include <public/physdev.h> #include <public/platform.h> +#include <public/event_channel.h> #include <public/xsm/flask_op.h> #include <avc.h> @@ -100,7 +104,9 @@ static int domain_has_xen(struct domain *d, u32 perms) static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad) { +#ifdef CONFIG_X86 struct irq_desc *desc = irq_to_desc(irq); +#endif if ( irq >= nr_irqs || irq < 0 ) return -EINVAL; if ( irq < nr_static_irqs ) { @@ -110,6 +116,7 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad) } return security_irq_sid(irq, sid); } +#ifdef CONFIG_X86 if ( desc->msi_desc ) { struct pci_dev *dev = desc->msi_desc->dev; u32 sbdf = (dev->seg << 16) | (dev->bus << 8) | dev->devfn; @@ -119,6 +126,7 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad) } return security_device_sid(sbdf, sid); } +#endif if (ad) { AVC_AUDIT_DATA_INIT(ad, IRQ); ad->irq = irq; @@ -822,7 +830,9 @@ static int flask_map_domain_pirq (struct domain *d, int irq, void *data) { u32 sid, dsid; int rc = -EPERM; +#ifdef CONFIG_X86 struct msi_info *msi = data; +#endif struct avc_audit_data ad; rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD); @@ -830,12 +840,17 @@ static int flask_map_domain_pirq (struct domain *d, int irq, void *data) if ( rc ) return rc; - if ( irq >= nr_static_irqs && msi ) { +#ifdef CONFIG_X86 + if ( irq >= nr_static_irqs && msi ) + { u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn; AVC_AUDIT_DATA_INIT(&ad, DEV); ad.device = machine_bdf; rc = security_device_sid(machine_bdf, &sid); - } else { + } + else +#endif + { rc = get_irq_sid(irq, &sid, &ad); } if ( rc ) @@ -1055,6 +1070,16 @@ static inline int flask_tmem_control(void) return domain_has_xen(current->domain, XEN__TMEM_CONTROL); } +static int flask_add_to_physmap(struct domain *d1, struct domain *d2) +{ + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); +} + +static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) +{ + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); +} + #ifdef CONFIG_X86 static int flask_shadow_control(struct domain *d, uint32_t op) { @@ -1325,16 +1350,6 @@ static int flask_update_va_mapping(struct domain *d, struct domain *f, return domain_has_perm(d, f, SECCLASS_MMU, map_perms); } -static int flask_add_to_physmap(struct domain *d1, struct domain *d2) -{ - return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); -} - -static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) -{ - return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); -} - static int flask_get_device_group(uint32_t machine_bdf) { u32 rsid; @@ -1501,6 +1516,9 @@ static struct xsm_operations flask_ops = { .do_xsm_op = do_flask_op, + .add_to_physmap = flask_add_to_physmap, + .remove_from_physmap = flask_remove_from_physmap, + #ifdef CONFIG_X86 .shadow_control = flask_shadow_control, .hvm_param = flask_hvm_param, @@ -1518,8 +1536,6 @@ static struct xsm_operations flask_ops = { .mmu_update = flask_mmu_update, .mmuext_op = flask_mmuext_op, .update_va_mapping = flask_update_va_mapping, - .add_to_physmap = flask_add_to_physmap, - .remove_from_physmap = flask_remove_from_physmap, .get_device_group = flask_get_device_group, .test_assign_device = flask_test_assign_device, .assign_device = flask_assign_device, diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c index a419cf4..c1a3fc9 100644 --- a/xen/xsm/xsm_policy.c +++ b/xen/xsm/xsm_policy.c @@ -20,6 +20,7 @@ #include <xsm/xsm.h> #include <xen/multiboot.h> +#include <xen/init.h> #include <asm/bitops.h> char *__initdata policy_buffer = NULL; -- 1.7.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |