From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH v2] domctl: tighten XEN_DOMCTL_*_permission Date: Tue, 06 May 2014 14:08:45 +0100 Message-ID: <5368FAFD020000780000F610@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part407276CD.1__=" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Whf73-0008Nb-LW for xen-devel@lists.xenproject.org; Tue, 06 May 2014 13:08:49 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel Cc: Ian Campbell , Keir Fraser , Ian Jackson , Tim Deegan List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part407276CD.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline With proper permission (and, for the I/O port case, wrap-around) checks added (note that for the I/O port case a count of zero is now being disallowed, in line with I/O memory handling): XEN_DOMCTL_irq_permission: XEN_DOMCTL_ioport_permission: Of both IRQs and I/O ports there is only a reasonably small amount, so there's no excess resource consumption involved here. Additionally they both have a specialized XSM hook associated. XEN_DOMCTL_iomem_permission: While this also has a specialized XSM hook associated (just like XEN_DOMCTL_{irq,ioport}_permission), it's not clear whether it's reasonable to expect XSM to restrict the number of ranges associated with a domain via this hook (which is the main resource consumption item here). Signed-off-by: Jan Beulich --- v2: Fix off-by-one in XEN_DOMCTL_ioport_permission bounds checking. --- a/docs/misc/xsm-flask.txt +++ b/docs/misc/xsm-flask.txt @@ -72,9 +72,7 @@ __HYPERVISOR_domctl (xen/include/public/ * XEN_DOMCTL_getvcpucontext * XEN_DOMCTL_max_vcpus * XEN_DOMCTL_scheduler_op - * XEN_DOMCTL_irq_permission * XEN_DOMCTL_iomem_permission - * XEN_DOMCTL_ioport_permission * XEN_DOMCTL_gethvmcontext * XEN_DOMCTL_sethvmcontext * XEN_DOMCTL_set_address_size --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -46,6 +46,8 @@ static int gdbsx_guest_mem_io( return (iop->remain ? -EFAULT : 0); } =20 +#define MAX_IOPORTS 0x10000 + long arch_do_domctl( struct xen_domctl *domctl, struct domain *d, XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) @@ -72,13 +74,11 @@ long arch_do_domctl( unsigned int np =3D domctl->u.ioport_permission.nr_ports; int allow =3D domctl->u.ioport_permission.allow_access; =20 - ret =3D -EINVAL; - if ( (fp + np) > 65536 ) - break; - - if ( np =3D=3D 0 ) - ret =3D 0; - else if ( xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, = allow) ) + if ( (fp + np) <=3D fp || (fp + np) > MAX_IOPORTS ) + ret =3D -EINVAL; + else if ( !ioports_access_permitted(current->domain, + fp, fp + np - 1) || + xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, = allow) ) ret =3D -EPERM; else if ( allow ) ret =3D ioports_permit_access(d, fp, fp + np - 1); @@ -719,7 +719,6 @@ long arch_do_domctl( =20 case XEN_DOMCTL_ioport_mapping: { -#define MAX_IOPORTS 0x10000 struct hvm_iommu *hd; unsigned int fgp =3D domctl->u.ioport_mapping.first_gport; unsigned int fmp =3D domctl->u.ioport_mapping.first_mport; --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -790,7 +790,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe =20 if ( pirq >=3D d->nr_pirqs ) ret =3D -EINVAL; - else if ( xsm_irq_permission(XSM_HOOK, d, pirq, allow) ) + else if ( !pirq_access_permitted(current->domain, pirq) || + xsm_irq_permission(XSM_HOOK, d, pirq, allow) ) ret =3D -EPERM; else if ( allow ) ret =3D pirq_permit_access(d, pirq); @@ -809,7 +810,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */ break; =20 - if ( xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, = allow) ) + if ( !iomem_access_permitted(current->domain, + mfn, mfn + nr_mfns - 1) || + xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, = allow) ) ret =3D -EPERM; else if ( allow ) ret =3D iomem_permit_access(d, mfn, mfn + nr_mfns - 1); --=__Part407276CD.1__= Content-Type: text/plain; name="domctl-permit-access.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="domctl-permit-access.patch" domctl: tighten XEN_DOMCTL_*_permission=0A=0AWith proper permission (and, = for the I/O port case, wrap-around) checks=0Aadded (note that for the I/O = port case a count of zero is now being=0Adisallowed, in line with I/O = memory handling):=0A=0AXEN_DOMCTL_irq_permission:=0AXEN_DOMCTL_ioport_permi= ssion:=0A=0A Of both IRQs and I/O ports there is only a reasonably small = amount, so=0A there's no excess resource consumption involved here. = Additionally=0A they both have a specialized XSM hook associated.=0A=0AXEN_= DOMCTL_iomem_permission:=0A=0A While this also has a specialized XSM hook = associated (just like=0A XEN_DOMCTL_{irq,ioport}_permission), it's not = clear whether it's=0A reasonable to expect XSM to restrict the number of = ranges associated=0A with a domain via this hook (which is the main = resource consumption=0A item here).=0A=0ASigned-off-by: Jan Beulich = =0A---=0Av2: Fix off-by-one in XEN_DOMCTL_ioport_permiss= ion bounds checking.=0A=0A--- a/docs/misc/xsm-flask.txt=0A+++ b/docs/misc/x= sm-flask.txt=0A@@ -72,9 +72,7 @@ __HYPERVISOR_domctl (xen/include/public/= =0A * XEN_DOMCTL_getvcpucontext=0A * XEN_DOMCTL_max_vcpus=0A * = XEN_DOMCTL_scheduler_op=0A- * XEN_DOMCTL_irq_permission=0A * XEN_DOMCTL_io= mem_permission=0A- * XEN_DOMCTL_ioport_permission=0A * XEN_DOMCTL_gethvmco= ntext=0A * XEN_DOMCTL_sethvmcontext=0A * XEN_DOMCTL_set_address_size=0A--= - a/xen/arch/x86/domctl.c=0A+++ b/xen/arch/x86/domctl.c=0A@@ -46,6 +46,8 = @@ static int gdbsx_guest_mem_io(=0A return (iop->remain ? -EFAULT : = 0);=0A }=0A =0A+#define MAX_IOPORTS 0x10000=0A+=0A long arch_do_domctl(=0A = struct xen_domctl *domctl, struct domain *d,=0A XEN_GUEST_HANDLE_PA= RAM(xen_domctl_t) u_domctl)=0A@@ -72,13 +74,11 @@ long arch_do_domctl(=0A = unsigned int np =3D domctl->u.ioport_permission.nr_ports;=0A = int allow =3D domctl->u.ioport_permission.allow_access;=0A =0A- = ret =3D -EINVAL;=0A- if ( (fp + np) > 65536 )=0A- = break;=0A-=0A- if ( np =3D=3D 0 )=0A- ret =3D 0;=0A- = else if ( xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, allow) = )=0A+ if ( (fp + np) <=3D fp || (fp + np) > MAX_IOPORTS )=0A+ = ret =3D -EINVAL;=0A+ else if ( !ioports_access_permitted(curren= t->domain,=0A+ fp, fp + np - 1) = ||=0A+ xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - = 1, allow) )=0A ret =3D -EPERM;=0A else if ( allow )=0A = ret =3D ioports_permit_access(d, fp, fp + np - 1);=0A@@ -719,7 = +719,6 @@ long arch_do_domctl(=0A =0A case XEN_DOMCTL_ioport_mapping:= =0A {=0A-#define MAX_IOPORTS 0x10000=0A struct hvm_iommu = *hd;=0A unsigned int fgp =3D domctl->u.ioport_mapping.first_gport;= =0A unsigned int fmp =3D domctl->u.ioport_mapping.first_mport;=0A--= - a/xen/common/domctl.c=0A+++ b/xen/common/domctl.c=0A@@ -790,7 +790,8 @@ = long do_domctl(XEN_GUEST_HANDLE_PARAM(xe=0A =0A if ( pirq >=3D = d->nr_pirqs )=0A ret =3D -EINVAL;=0A- else if ( = xsm_irq_permission(XSM_HOOK, d, pirq, allow) )=0A+ else if ( = !pirq_access_permitted(current->domain, pirq) ||=0A+ = xsm_irq_permission(XSM_HOOK, d, pirq, allow) )=0A ret =3D = -EPERM;=0A else if ( allow )=0A ret =3D pirq_permit_acc= ess(d, pirq);=0A@@ -809,7 +810,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(x= e=0A if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */=0A = break;=0A =0A- if ( xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + = nr_mfns - 1, allow) )=0A+ if ( !iomem_access_permitted(current->doma= in,=0A+ mfn, mfn + nr_mfns - 1) ||=0A+ = xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, = allow) )=0A ret =3D -EPERM;=0A else if ( allow )=0A = ret =3D iomem_permit_access(d, mfn, mfn + nr_mfns - 1);=0A --=__Part407276CD.1__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__Part407276CD.1__=--