[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3] x86/altp2m: Hypercall to set altp2m view visibility


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
  • Date: Thu, 20 Feb 2020 09:59:59 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=bitdefender.com; dmarc=pass action=none header.from=bitdefender.com; dkim=pass header.d=bitdefender.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=iMztn245XkEhzXSFM7UoGhi4z2rEDnI3mGuOXSMDBow=; b=NG4LKdNeeQ82/eWXSJp8aF+F+pncIeJMybss4XVFw+tqoCVA1yaMEaA59GbqCNSn8defsPMrOhAJLuSPzl0ZtlB/+6BnYhua+R2jyylcSUBYuxO7e1lg/0eh5OtznqG3VhyYG6Z+/fu41i11G51QAw94OXufw1QNeBReLW6dQ9xGDFIOHpp6bzMKzlmVx8TNTEMwRc5hj+FuKc/uZahvQMV17oyjPHbvBbz/igZA1ECm9Brj2I8X/UcSsYXrTJ/H5NG/+ifo44iJxZQEC37EZgZNUmuiu5yrxmL7y8sDwzqtLbwwHqnMumavuuz+MqAIRE4xefVQRGCjhMKuvaMxwA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=f/jVrw1a2auDNG8AEjboVmp0cNUoQoqh6QXkyLWJ/lvAXBQUiIlkwWYJFwwoVe6tN3mVRwNfZCUev3Oo+vOnGs6cIWqN35urGFnf5bED4/QdJXwypx+DdSjkS89L1xequyFcRqBEHeDzpqpSYSuFrkql2Kg+pY80njeMqTSSux2aSvXQ5Y8+7WrfIulbvRqhSuPAA51Mx+UDgXIMnaheV6wNIBPY8DsBDjMQzqQp2SYKdbGp4YE453aMMFoTZj8bIuDYKJ7CBJOf8PQSm6XJ9ogRypO/We+1RCEhsmBBsRw/hz60JQqKrvusBsai0Qa6YmKD5PQljQc2nR8BSGhQbg==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=aisaila@xxxxxxxxxxxxxxx;
  • Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Thu, 20 Feb 2020 10:00:36 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHV5wWKuFf2vClElk6nKYx34YpPrKgivhiAgAEc4IA=
  • Thread-topic: [PATCH V3] x86/altp2m: Hypercall to set altp2m view visibility


On 19.02.2020 19:00, Jan Beulich wrote:
> On 19.02.2020 10:18, Alexandru Stefan ISAILA wrote:
>> @@ -4835,6 +4836,23 @@ static int do_altp2m_op(
>>           break;
>>       }
>>   
>> +    case HVMOP_altp2m_set_visibility:
>> +    {
>> +        uint16_t altp2m_idx = a.u.set_visibility.altp2m_idx;
>> +
>> +        if ( a.u.set_visibility.pad )
>> +            rc = -EINVAL;
>> +        else if ( !altp2m_active(d) )
>> +            rc = -EOPNOTSUPP;
>> +        else if ( a.u.set_visibility.visible )
>> +            d->arch.altp2m_working_eptp[altp2m_idx] =
>> +                d->arch.altp2m_eptp[altp2m_idx];
>> +        else
>> +            d->arch.altp2m_working_eptp[altp2m_idx] =
>> +                mfn_x(INVALID_MFN);
> 
> Don't you need to bounds check the index before its use?

Unless we want a index out of bounds from the user. Sorry for not having 
that, I will add a "altp2m_eptp[array_index_nospec(altp2m_idx, 
MAX_EPTP)]" in place for the next version.

> And
> shouldn't you return an error also for in-range ones which
> aren't actually valid?
That is a good thing. Maybe -EINVAL could fit this?

And together with the bounds check it will end up something like this:

if ( !altp2m_idx < 0 || altp2m_idx >= 
min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) || 
altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] == 
mfn_x(INVALID_MFN) )

return -EINVAL;

And looking again, it could be coupled with the a.u.set_visibility.pad.

Thanks,
Alex
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.