On 16.02.2026 14:43, Julian Vetter wrote:
> The page was allocated with MEMF_no_refcount. The MEMF_no_refcount flag
> means the page is allocated without a regular reference, but it still
> has the allocation reference. If get_page_and_type() fails, we still
> need to release the allocation reference. Otherwise the page would leak.
> domain_crash() doesn't free individual pages; it just marks the domain
> for destruction. The domain teardown will eventually free domain heap
> pages, but only those it can find. A page with a dangling alloc ref
> would prevent the page from being fully freed during domain cleanup.
> 
> Signed-off-by: Julian Vetter <[email protected]>

Yes, it would be nice if this could be done, but have you read the big
comment in put_page_alloc_ref()? We have no choice here, ...

> --- a/xen/common/ioreq.c
> +++ b/xen/common/ioreq.c
> @@ -287,6 +287,7 @@ static int ioreq_server_alloc_mfn(struct ioreq_server *s, 
> bool buf)
>           * The domain can't possibly know about this page yet, so failure
>           * here is a clear indication of something fishy going on.
>           */
> +        put_page_alloc_ref(page);
>          domain_crash(s->emulator);
>          return -ENODATA;
>      }

... we need to leak the page. (Perhaps the comment could have done with
saying so explicitly.) Hence why the domain is being crashed, so the
leak cannot easily accumulate. Otherwise simply returning an error here
would be quite fine.

Jan

Reply via email to