Re: [Xen-devel] [PATCH v19 10/11] common: add a new mappable resource type: XENMEM_resource_grant_table

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 12 April 2018 16:28
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>;
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org)
> <tim@xxxxxxx>
> Subject: Re: [PATCH v19 10/11] common: add a new mappable resource
> type: XENMEM_resource_grant_table
> 
> >>> On 29.03.18 at 17:36, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -967,6 +968,54 @@ static long xatp_permission_check(struct domain
> *d, unsigned int space)
> >      return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
> >  }
> >
> > +static int acquire_grant_table(struct domain *d, unsigned int id,
> > +                               unsigned long frame,
> > +                               unsigned int nr_frames,
> > +                               xen_pfn_t mfn_list[])
> > +{
> > +    unsigned int i = nr_frames;
> > +
> > +    /*
> > +     * FIXME: It is not currently safe to map grant status frames if they
> > +     *        will be inserted into the caller's P2M, because these
> > +     *        insertions are not yet properly reference counted.
> > +     *        This restriction can be removed when appropriate reference
> > +     *        counting is added.
> > +     */
> > +    if ( paging_mode_translate(current->domain) &&
> > +         (id == XENMEM_resource_grant_table_id_status) )
> > +        return -EOPNOTSUPP;
> 
> I don't understand why this is for status frames only: The refcounting
> problem
> exists in any case (at the very least when the guest goes away but the
> mapping
> domain survives). The ioreq server use is fine because the page gets
> assigned
> to the domain intended to do the mapping.

Ok. I had limited to status frames as they can go away on a version change but, 
yes, it should cover both for a tools domain that's not fully trusted.

> 
> However, besides tightening the check, there can also be a little bit of
> relaxation, I think: At least the hardware domain can do such mappings, as
> we trust it anyway (and it won't - for the foreseeable future - go away).
> 

Right. I'll adjust accordingly.

  Paul

> Jan
> 


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