lra@sics.se
2013-Jan-11 13:32 UTC
[PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
From: Lars Rasmusson <Lars.Rasmusson@sics.se>
Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
---
xen/common/memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index c8c1ef2..8230565 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
if ( rc != 0 )
return rc;
+#ifdef CONFIG_X86
if ( xsm_remove_from_physmap(current->domain, d) )
{
rcu_unlock_domain(d);
return -EPERM;
}
-
+#endif
domain_lock(d);
page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
--
1.7.9.5
Ian Campbell
2013-Jan-11 13:40 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Fri, 2013-01-11 at 13:32 +0000, lra@sics.se wrote:> From: Lars Rasmusson <Lars.Rasmusson@sics.se> > > Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>Thanks but I think the right approach here would be to add the necessary stuff to ARM rather than skip the permissions checks on ARM. Ian.
Keir Fraser
2013-Jan-11 13:47 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:> From: Lars Rasmusson <Lars.Rasmusson@sics.se> > > Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>If this is a build fix after my checkins this morning then: Acked-by: Keir Fraser <keir@xen.org>> --- > xen/common/memory.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/common/memory.c b/xen/common/memory.c > index c8c1ef2..8230565 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd, > XEN_GUEST_HANDLE_PARAM(void) arg) > if ( rc != 0 ) > return rc; > > +#ifdef CONFIG_X86 > if ( xsm_remove_from_physmap(current->domain, d) ) > { > rcu_unlock_domain(d); > return -EPERM; > } > - > +#endif > domain_lock(d); > > page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
Lars Rasmusson
2013-Jan-11 16:24 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Jan 11, 2013, at 14:47 , Keir Fraser <keir@xen.org> wrote:> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: > >> From: Lars Rasmusson <Lars.Rasmusson@sics.se> >> >> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> > > If this is a build fix after my checkins this morning then: > Acked-by: Keir Fraser <keir@xen.org>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....) /Lars> >> --- >> xen/common/memory.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/xen/common/memory.c b/xen/common/memory.c >> index c8c1ef2..8230565 100644 >> --- a/xen/common/memory.c >> +++ b/xen/common/memory.c >> @@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd, >> XEN_GUEST_HANDLE_PARAM(void) arg) >> if ( rc != 0 ) >> return rc; >> >> +#ifdef CONFIG_X86 >> if ( xsm_remove_from_physmap(current->domain, d) ) >> { >> rcu_unlock_domain(d); >> return -EPERM; >> } >> - >> +#endif >> domain_lock(d); >> >> page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC); > >
Keir Fraser
2013-Jan-11 16:36 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On 11/01/2013 16:24, "Lars Rasmusson" <lra@sics.se> wrote:>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: >> >>> From: Lars Rasmusson <Lars.Rasmusson@sics.se> >>> >>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> >> >> If this is a build fix after my checkins this morning then: >> Acked-by: Keir Fraser <keir@xen.org> > > 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. -- Keir
Ian Campbell
2013-Jan-11 16:46 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Fri, 2013-01-11 at 16:36 +0000, Keir Fraser wrote:> On 11/01/2013 16:24, "Lars Rasmusson" <lra@sics.se> wrote: > > >> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: > >> > >>> From: Lars Rasmusson <Lars.Rasmusson@sics.se> > >>> > >>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> > >> > >> If this is a build fix after my checkins this morning then: > >> Acked-by: Keir Fraser <keir@xen.org> > > > > 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@citrix.com> 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@sics.se> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- xen/arch/arm/mm.c | 13 +++++++++++++ xen/include/xsm/dummy.h | 24 ++++++++++++------------ 2 files changed, 25 insertions(+), 12 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..870bd67 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -443,6 +443,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 +556,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); -- 1.7.9.1
Daniel De Graaf
2013-Jan-11 17:37 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
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@sics.se> wrote: >> >>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: >>>> >>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se> >>>>> >>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> >>>> >>>> If this is a build fix after my checkins this morning then: >>>> Acked-by: Keir Fraser <keir@xen.org> >>> >>> 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@citrix.com> > 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@sics.se> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > --- > 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. 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.
Ian Campbell
2013-Jan-15 11:18 UTC
Re: [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@sics.se> wrote: > >> > >>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: > >>>> > >>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se> > >>>>> > >>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> > >>>> > >>>> If this is a build fix after my checkins this morning then: > >>>> Acked-by: Keir Fraser <keir@xen.org> > >>> > >>> 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@citrix.com> > > 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@sics.se> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > --- > > 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@citrix.com> 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@sics.se> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- 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
Stefano Stabellini
2013-Jan-15 11:52 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Tue, 15 Jan 2013, Ian Campbell wrote:> From 469d20054b96b8bc749891c366659532b67d6031 Mon Sep 17 00:00:00 2001 > From: Ian Campbell <ian.campbell@citrix.com> > 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@sics.se> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>the patch looks mostly to me> 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 )this change is correct> 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 )this part is a bit ugly, can we refactor the msi checks into separate, arch specific, functions?
Ian Campbell
2013-Jan-15 13:42 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Tue, 2013-01-15 at 11:52 +0000, Stefano Stabellini wrote:> > @@ -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 ) > > this part is a bit ugly, can we refactor the msi checks into separate, > arch specific, functions?Yes that would be better. I''ll give Daniel a chance to respond to the rest before I respin with this change. Ian.
Daniel De Graaf
2013-Jan-15 14:35 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On 01/15/2013 06:18 AM, Ian Campbell wrote:> 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@sics.se> wrote: >>>> >>>>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote: >>>>>> >>>>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se> >>>>>>> >>>>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se> >>>>>> >>>>>> If this is a build fix after my checkins this morning then: >>>>>> Acked-by: Keir Fraser <keir@xen.org> >>>>> >>>>> 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@citrix.com> >>> 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@sics.se> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> >>> --- >>> 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@citrix.com> > 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@sics.se> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > --- > 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; > + } > +Using the return value of the xsm hook instead of -EPERM is preferred; this turns an -ENOMEM inside FLASK into an -EPERM (and same below).> 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; >The rest of the changes look correct. The #ifdefs are a bit ugly, but refactoring the MSI code into an arch-specific function should fix that.
Ian Campbell
2013-Jan-17 17:05 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On Tue, 2013-01-15 at 14:35 +0000, Daniel De Graaf wrote:> The rest of the changes look correct. The #ifdefs are a bit ugly, but > refactoring the MSI code into an arch-specific function should fix > that.I''ve just sent out a new patch which takes are of only the build failure on ARM with XSM disabled. For fixing the flask build on ARM: what does security_device_sid return? If I want to refactor this: #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; if (ad) { AVC_AUDIT_DATA_INIT(ad, DEV); ad->device = sbdf; } return security_device_sid(sbdf, sid); } #endif into an arch specific function I need to be able to return something in the !desc->msi_desc case. Can a sid be any integer or could I return e.g. 0 in this case? Ian.
Daniel De Graaf
2013-Jan-17 17:13 UTC
Re: [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM
On 01/17/2013 12:05 PM, Ian Campbell wrote:> On Tue, 2013-01-15 at 14:35 +0000, Daniel De Graaf wrote: >> The rest of the changes look correct. The #ifdefs are a bit ugly, but >> refactoring the MSI code into an arch-specific function should fix >> that. > > I''ve just sent out a new patch which takes are of only the build failure > on ARM with XSM disabled. > > For fixing the flask build on ARM: what does security_device_sid return? > If I want to refactor this: > #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; > if (ad) { > AVC_AUDIT_DATA_INIT(ad, DEV); > ad->device = sbdf; > } > return security_device_sid(sbdf, sid); > } > #endif > into an arch specific function I need to be able to return something in > the !desc->msi_desc case. Can a sid be any integer or could I return > e.g. 0 in this case? > > Ian. >There are some reserved sids that might be useful here (SECINITSID_UNLABELED if this case shouldn''t be encountered). Zero might be the most suitable value, since zero is not a valid sid and can be used to indicate "unable to resolve". Zero will be treated as the unlabeled SID if passed to avc_has_perm. security_device_sid returns either SECINITSID_DEVICE or the assigned SID of the requested PCI device (as indexed by its sbdf number, and set either in XSM policy or by flask-label-pci). -- Daniel De Graaf National Security Agency