* [PATCH] xsm: add missing permissions discovered in testing
@ 2016-11-04 15:35 Daniel De Graaf
2016-11-04 15:44 ` Andrew Cooper
2016-11-07 7:22 ` Wei Liu
0 siblings, 2 replies; 4+ messages in thread
From: Daniel De Graaf @ 2016-11-04 15:35 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Daniel De Graaf
Add two missing allow rules:
1. Device model domain construction uses getvcpucontext, discovered by
Andrew Cooper in an (apparently) unrelated bisection.
2. When a domain is destroyed with a device passthrough active, the
calls to remove_{irq,ioport,iomem} can be made by the hypervisor itself
(which results in an XSM check with the source xen_t). It does not make
sense to deny these permissions; no domain should be using xen_t, and
forbidding the hypervisor from performing cleanup is not useful.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
tools/flask/policy/modules/xen.if | 2 +-
tools/flask/policy/modules/xen.te | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
index d83f031..eb646f5 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -49,7 +49,7 @@ define(`create_domain_common', `
allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
getdomaininfo hypercall setvcpucontext getscheduler
getvcpuinfo getaddrsize getaffinity setaffinity
- settime setdomainhandle };
+ settime setdomainhandle getvcpucontext };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
psr_cmt_op psr_cat_op soft_reset };
diff --git a/tools/flask/policy/modules/xen.te b/tools/flask/policy/modules/xen.te
index b52edc2..0cff2df 100644
--- a/tools/flask/policy/modules/xen.te
+++ b/tools/flask/policy/modules/xen.te
@@ -49,6 +49,10 @@ type ioport_t, resource_type;
type iomem_t, resource_type;
type device_t, resource_type;
+# Domain destruction can result in some access checks for actions performed by
+# the hypervisor. These should always be allowed.
+allow xen_t resource_type : resource { remove_irq remove_ioport remove_iomem };
+
################################################################################
#
# Policy constraints
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] xsm: add missing permissions discovered in testing
2016-11-04 15:35 [PATCH] xsm: add missing permissions discovered in testing Daniel De Graaf
@ 2016-11-04 15:44 ` Andrew Cooper
2016-11-07 7:22 ` Wei Liu
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2016-11-04 15:44 UTC (permalink / raw)
To: Daniel De Graaf, xen-devel
On 04/11/16 15:35, Daniel De Graaf wrote:
> Add two missing allow rules:
>
> 1. Device model domain construction uses getvcpucontext, discovered by
> Andrew Cooper in an (apparently) unrelated bisection.
Merely observation of the logs while chasing an unrelated issue.
~Andrew
>
> 2. When a domain is destroyed with a device passthrough active, the
> calls to remove_{irq,ioport,iomem} can be made by the hypervisor itself
> (which results in an XSM check with the source xen_t). It does not make
> sense to deny these permissions; no domain should be using xen_t, and
> forbidding the hypervisor from performing cleanup is not useful.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> tools/flask/policy/modules/xen.if | 2 +-
> tools/flask/policy/modules/xen.te | 4 ++++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
> index d83f031..eb646f5 100644
> --- a/tools/flask/policy/modules/xen.if
> +++ b/tools/flask/policy/modules/xen.if
> @@ -49,7 +49,7 @@ define(`create_domain_common', `
> allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
> getdomaininfo hypercall setvcpucontext getscheduler
> getvcpuinfo getaddrsize getaffinity setaffinity
> - settime setdomainhandle };
> + settime setdomainhandle getvcpucontext };
> allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
> set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
> psr_cmt_op psr_cat_op soft_reset };
> diff --git a/tools/flask/policy/modules/xen.te b/tools/flask/policy/modules/xen.te
> index b52edc2..0cff2df 100644
> --- a/tools/flask/policy/modules/xen.te
> +++ b/tools/flask/policy/modules/xen.te
> @@ -49,6 +49,10 @@ type ioport_t, resource_type;
> type iomem_t, resource_type;
> type device_t, resource_type;
>
> +# Domain destruction can result in some access checks for actions performed by
> +# the hypervisor. These should always be allowed.
> +allow xen_t resource_type : resource { remove_irq remove_ioport remove_iomem };
> +
> ################################################################################
> #
> # Policy constraints
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] xsm: add missing permissions discovered in testing
2016-11-04 15:35 [PATCH] xsm: add missing permissions discovered in testing Daniel De Graaf
2016-11-04 15:44 ` Andrew Cooper
@ 2016-11-07 7:22 ` Wei Liu
2016-11-07 10:58 ` Wei Liu
1 sibling, 1 reply; 4+ messages in thread
From: Wei Liu @ 2016-11-07 7:22 UTC (permalink / raw)
To: Daniel De Graaf; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Fri, Nov 04, 2016 at 11:35:20AM -0400, Daniel De Graaf wrote:
> Add two missing allow rules:
>
> 1. Device model domain construction uses getvcpucontext, discovered by
> Andrew Cooper in an (apparently) unrelated bisection.
>
> 2. When a domain is destroyed with a device passthrough active, the
> calls to remove_{irq,ioport,iomem} can be made by the hypervisor itself
> (which results in an XSM check with the source xen_t). It does not make
> sense to deny these permissions; no domain should be using xen_t, and
> forbidding the hypervisor from performing cleanup is not useful.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
I will pick this up for 4.8.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] xsm: add missing permissions discovered in testing
2016-11-07 7:22 ` Wei Liu
@ 2016-11-07 10:58 ` Wei Liu
0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2016-11-07 10:58 UTC (permalink / raw)
To: Daniel De Graaf; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Mon, Nov 07, 2016 at 07:22:40AM +0000, Wei Liu wrote:
> On Fri, Nov 04, 2016 at 11:35:20AM -0400, Daniel De Graaf wrote:
> > Add two missing allow rules:
> >
> > 1. Device model domain construction uses getvcpucontext, discovered by
> > Andrew Cooper in an (apparently) unrelated bisection.
> >
> > 2. When a domain is destroyed with a device passthrough active, the
> > calls to remove_{irq,ioport,iomem} can be made by the hypervisor itself
> > (which results in an XSM check with the source xen_t). It does not make
> > sense to deny these permissions; no domain should be using xen_t, and
> > forbidding the hypervisor from performing cleanup is not useful.
> >
> > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> I will pick this up for 4.8.
Updated commit message and applied.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-07 10:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-04 15:35 [PATCH] xsm: add missing permissions discovered in testing Daniel De Graaf
2016-11-04 15:44 ` Andrew Cooper
2016-11-07 7:22 ` Wei Liu
2016-11-07 10:58 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).