Re: [PATCH v2 6/6] gnttab: allow disabling grant table per-domain

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 22/09/2021 14:38, Juergen Gross wrote:
> On 22.09.21 11:19, Julien Grall wrote:
> > Hi Roger,
> >
> > On 22/09/2021 13:21, Roger Pau Monne wrote:
> > > Allow setting max_grant_version to 0 in order to disable grant table
> > > usage by a domain. This prevents allocating the grant-table structure
> > > inside of Xen and requires guards to be added in several functions in
> > > order to prevent dereferencing the structure.
> > >
> > > Note that a domain without a grant table could still use some of the
> > > grant related hypercalls, it could for example issue a GNTTABOP_copy
> > > of a grant reference from a remote domain into a local frame.
> > >
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > ---
> > >   docs/man/xl.cfg.5.pod.in     |   4 +-
> > >   tools/libs/light/libxl_dom.c |   2 +-
> > >   xen/common/grant_table.c     | 100 +++++++++++++++++++++++++++++++++--
> > >   3 files changed, 98 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> > > index c5a447dfcd..d507540c2c 100644
> > > --- a/docs/man/xl.cfg.5.pod.in
> > > +++ b/docs/man/xl.cfg.5.pod.in
> > > @@ -583,8 +583,8 @@ L<xl.conf(5)>.
> > >   =item B<max_grant_version=NUMBER>
> > >   Specify the maximum grant table version the domain is allowed to 
> > > use. Current
> > > -supported versions are 1 and 2. The default value is settable via
> > > -L<xl.conf(5)>.
> > > +supported versions are 1 and 2. Setting to 0 disables the grant 
> > > table for the
> > > +domain. The default value is settable via L<xl.conf(5)>.
> > Technically, the version only applies to format of the table for 
> > granting page. The mapping itself is version agnostic. So this feels a 
> > bit wrong to use max_grant_version to not allocate d->grant_table.
> > I also can see use-cases where we may want to allow a domain to grant 
> > page but not map grant (for instance, a further hardening of XSA-380). 
> > Therefore, I think we want to keep max_grant_version for the table 
> > itself and manage the mappings separately (possibly by letting the 
> > admin to select the max number of entries in the maptrack).
> You mean the already existing domain config option max_maptrack_frames?
Yes. I didn't realize we already had an option for that :).

Cheers,

--
Julien Grall