Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross <jgross@xxxxxxxx>]

On 24/08/17 17:16, Andrew Cooper wrote:
> On 24/08/17 16:01, Juergen Gross wrote:
>> On 24/08/17 16:50, Andrew Cooper wrote:
>>> This patch was originally a workaround for XSA-226.  Since then, transitive
>>> grants are believed to be functioning properly, and the defaults have 
>>> changed
>>> appropriately.
>>>
>>> However, for those people who chose to use the workaround (especially from 
>>> an
>>> attack surface mitigation point of view), retain the ability for the host
>>> administrator to choose.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
>>> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: Tim Deegan <tim@xxxxxxx>
>>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> ---
>>>  docs/misc/xen-command-line.markdown | 13 +++++++++++
>>>  xen/common/grant_table.c            | 44 
>>> +++++++++++++++++++++++++++++++++++--
>>>  2 files changed, 55 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/misc/xen-command-line.markdown 
>>> b/docs/misc/xen-command-line.markdown
>>> index 4002eab..78c7b51 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>>  
>>>  Specify which console gdbstub should use. See **console**.
>>>  
>>> +### gnttab
>>> +> `= List of [ max_ver:<integer>, transitive ]`
>>> +
>>> +> Default: `gnttab=max_ver:2,transitive`
>>> +
>>> +Control various aspects of the grant table behaviour available to guests.
>>> +
>>> +* `max_ver` Select the maximum grant table version to offer to guests.  
>>> Valid
>>> +version are 1 and 2.
>>> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
>>> the
>>> +use of grant table v2 without transitive grants is an ABI breakage from the
>>> +guests point of view.
>> So shouldn't there be a way for the guest to query the support of
>> transient grants?
> 
> Ideally yes, but how do you suggest doing this in a compatible way?

An ELF note in the guest kernel indicating it is aware of that
possibility in order to allow v2 grants for that guest without transient
grants?

> All Xen downstreams which haven't backported the eventual transitive
> fixes will have this clobber in place, without any query-ability.

So the only really compatible way would be to disallow v2 grants. OTOH
I guess there aren't so many guests using v2 right now...

> The reason this workaround was so effective, and why several large users
> still wish to clobber them, is because nothing uses transitive grants.

Right. And only very few guests use v2 grants.

> 
>>
>>> +
>>>  ### gnttab\_max\_frames
>>>  > `= <integer>`
>>>  
>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>>> index 36895aa..f9c313d 100644
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", 
>>> max_nr_grant_frames);
>>>  unsigned int __read_mostly max_grant_frames;
>>>  integer_param("gnttab_max_frames", max_grant_frames);
>>>  
>>> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
>>> +static bool __read_mostly opt_transitive_grants = true;
>>> +
>>> +static void __init parse_gnttab(char *s)
>> Do you mind using:
>>
>> static int __init parse_gnttab(const char *s)
>>
>> in order to comply with my runtime parameter series?
> 
> If the result will still compile.  What should the semantics be?  (I've
> had a quick look through your series, but I can't see the semantics
> stated anywhere obvious)

The parsing routine must not change the parameter string and should
return an error (e.g. -EINVAL) in case of an illegal parameter.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel