From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2] domctl: tighten XEN_DOMCTL_*_permission
Date: Fri, 15 Aug 2014 11:36:42 +0100
Message-ID: <53EDE2BA.7020307@citrix.com>
References: <5368FAFD020000780000F610@mail.emea.novell.com>
	<CAH_mUMM7g05dvSQVU+C7SS-Le-r5C_R_7qRFu45O9Qid6NmB_g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Andrew.Cooper3@citrix.com>) id 1XIEsJ-0007wl-Tc
	for xen-devel@lists.xenproject.org; Fri, 15 Aug 2014 10:36:48 +0000
In-Reply-To: <CAH_mUMM7g05dvSQVU+C7SS-Le-r5C_R_7qRFu45O9Qid6NmB_g@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>, Jan Beulich <JBeulich@suse.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>, xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>
List-Id: xen-devel@lists.xenproject.org

On 15/08/14 11:00, Andrii Tseglytskyi wrote:
> Hi,
>
> I see possible issue with this patch. Can someone clarify - did I get
> everything correctly?
>
> On Tue, May 6, 2014 at 4:08 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> @@ -790,7 +790,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>>
>>          if ( pirq >= d->nr_pirqs )
>>              ret = -EINVAL;
>> -        else if ( xsm_irq_permission(XSM_HOOK, d, pirq, allow) )
>> +        else if ( !pirq_access_permitted(current->domain, pirq) ||
> pirq_access_permitted() checks a range. Range can be added only with
> pirq_permit_access() function call. The only place where
> pirq_permit_access() is called - is following
>  *else if* branch. But it will be never called -
> pirq_access_permitted() will return 0 if range does not exist. As
> result - it is impossible to add irq, even if XSM allows this.
> The same is true for iomem_access_permitted() function call.

I questioned the same issue when this patch went in.

The argument is that, even with XSM, a domain may only permit access to
pirqs for which it also has permissions.

This prevents a buggy domain builder accidentally conferring pirq access
for a dom0 resource, without dom0 first having conferred access to the
domain builder.

~Andrew