Re: [PATCH] Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

[Jürgen Groß <jgross@xxxxxxxx>]

On 06.10.26 10:28, Jan Beulich wrote:
> On 06.10.2026 10:12, Juergen Gross wrote:
> > On 16.02.26 09:55, Jan Beulich wrote:
> > > Both decrease-reservation and populate-physmap can succeed partially. For
> > > the former this may be pretty unlikely, but clearly the latter can hit
> > > both out-of-memory conditions in the hypervisor or denial because of the
> > > allocation exceeding the domain's allowance (there's no interaction with
> > > the balloon driver here either).
> > >
> > > In gnttab_dma_free_pages() we simply can't give back to the system what
> > > hasn't been re-filled with backing memory. For gnttab_dma_alloc_pages()
> > > both parts of the overall region need dealing with differently.
> > >
> > > While no present caller of gnttab_dma_free_pages() checks its return
> > > value, also don't use -EFAULT there: It really is an out-of-memory
> > > condition. (In gnttab_dma_alloc_pages() -EFAULT doesn't look quite right
> > > either, but I couldn't think of a clearly better error code there.)
> > >
> > > Fixes: 9bdc7304f536 ("xen/grant-table: Allow allocating buffers suitable for 
> > > DMA")
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > I'm likely screwing things up, as I can't understand how this is intended
> > > to work: args->dev_bus_addr shouldn't really be in pseudo-physical address
> > > space, or else the address isn't suitable for handing to a device in order
> > > to DMA to/from it. Hence using __phys_to_pfn() on it to then hand the
> > > result to pfn_to_page() feels bogus. Was all of this perhaps only ever
> > > intended for (tested with) translated domains? With the uses of
> > > xenmem_reservation_va_mapping_*() only being masquerade?
> > I finally forced myself to look into this maze.
> Thanks much.
>
> > gnttab_dma_alloc_pages() will only be called via
> > gntdev_ioctl_dmabuf_exp_from_refs(), which has:
> >
> >         if (xen_pv_domain()) {
> >                 pr_debug("Cannot provide dma-buf in a PV domain\n");
> >                 return -EINVAL;
> >         }
> >
> > at its top.
> Well, gnttab_dma_alloc_pages() is an exported function, so it can in
> principle be called from anywhere, including out-of-tree modules.
Right, that is why I want to write patch rejecting any calls of
gnttab_dma_alloc_pages() in PV guests.

> > I'll setup a patch making this clear.
> >
> > Are you fine with me taking your patch and cleaning it up regarding the
> > use of xenmem_reservation_va_mapping_update()?
> Yes, sure.
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature