From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH 12/20] xen: avoid calling
 rcu_lock_*target_domain when an XSM hook exists
Date: Tue, 11 Sep 2012 09:26:38 -0400
Message-ID: <504F3C0E.8020901@tycho.nsa.gov>
References: <1347306553-20980-1-git-send-email-dgdegra@tycho.nsa.gov>
	<1347306553-20980-13-git-send-email-dgdegra@tycho.nsa.gov>
	<504F0603020000780009A6CC@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <504F0603020000780009A6CC@nat28.tlf.novell.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: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 09/11/2012 03:36 AM, Jan Beulich wrote:
>>>> On 10.09.12 at 21:49, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -195,30 +195,6 @@ double_gt_unlock(struct grant_table *lgt, struct 
>> grant_table *rgt)
>>          spin_unlock(&rgt->lock);
>>  }
>>  
>> -static struct domain *gt_lock_target_domain_by_id(domid_t dom)
>> -{
>> -    struct domain *d;
>> -    int rc = GNTST_general_error;
>> -
>> -    switch ( rcu_lock_target_domain_by_id(dom, &d) )
>> -    {
>> -    case 0:
>> -        return d;
>> -
>> -    case -ESRCH:
>> -        gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom);
>> -        rc = GNTST_bad_domain;
>> -        break;
>> -
>> -    case -EPERM:
>> -        rc = GNTST_permission_denied;
>> -        break;
>> -    }
>> -
>> -    ASSERT(rc < 0 && -rc <= MAX_ERRNO);
>> -    return ERR_PTR(rc);
>> -}
>> -
> 
> Removing that function again is fine as long as you retain the
> distinguished error codes, which ...
> 
>>  static inline int
>>  __get_maptrack_handle(
>>      struct grant_table *t)
>> @@ -1304,11 +1280,12 @@ gnttab_setup_table(
>>          goto out1;
>>      }
>>  
>> -    d = gt_lock_target_domain_by_id(op.dom);
>> -    if ( IS_ERR(d) )
>> +    d = rcu_lock_domain_by_any_id(op.dom);
>> +    if ( d == NULL )
>>      {
>> -        op.status = PTR_ERR(d);
>> -        goto out1;
>> +        gdprintk(XENLOG_INFO, "Bad domid %d.\n", op.dom);
>> +        op.status = GNTST_bad_domain;
> 
> ... you don't.

Actually, I do. The only distinguishing error code here is
GNTST_permission_denied, which is now only triggered by the XSM
hook.

>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -570,7 +570,8 @@ long do_memory_op(unsigned long cmd, 
>> XEN_GUEST_HANDLE(void) arg)
>>               && (reservation.mem_flags & XENMEMF_populate_on_demand) )
>>              args.memflags |= MEMF_populate_on_demand;
>>  
>> -        if ( unlikely(rcu_lock_target_domain_by_id(reservation.domid, &d)) )
>> +        d = rcu_lock_domain_by_any_id(reservation.domid);
>> +        if ( d == NULL )
>>              return start_extent;
>>          args.domain = d;
>>  
>> @@ -619,9 +620,9 @@ long do_memory_op(unsigned long cmd, 
>> XEN_GUEST_HANDLE(void) arg)
>>          if ( copy_from_guest(&domid, arg, 1) )
>>              return -EFAULT;
>>  
>> -        rc = rcu_lock_target_domain_by_id(domid, &d);
>> -        if ( rc )
>> -            return rc;
>> +        d = rcu_lock_domain_by_any_id(domid);
>> +        if ( d == NULL )
>> +            return -ESRCH;
> 
> And quite similarly here and further down you're losing -EPERM.
> 
> Jan
> 
Same logic: rcu_lock_domain_by_any_id will succeed where -EPERM was
returned by rcu_lock_target_domain_by_id; the check is moved to the
XSM hook.

-- 
Daniel De Graaf
National Security Agency