Re: [PATCH] xen/gnttab: _cache_flush refuse an owner without a grant table

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/11/2026 1:06 am, Stefano Stabellini wrote:
> With CONFIG_STATIC_SHM, a shared memory region without a "role" is owned
> by dom_io. A cache flush on such a page then reaches grant_read_lock()
> with dom_io's NULL grant table. Reinstate the check, and give back the
> page reference taken by page_get_owner_and_reference().
>
> Fixes: bec8e3c1fb91 ("xen/gnttab: Drop owner->grant_table check in 
> _cache_flush()")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> ---
>  xen/common/grant_table.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 2285c4aee8..999b10abcf 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3633,9 +3633,11 @@ static int _cache_flush(gnttab_cache_flush_t *cflush, 
> grant_ref_t *cur_ref)
>  
>      page = mfn_to_page(mfn);
>      owner = page_get_owner_and_reference(page);
> -    if ( !owner )
> +    if ( !owner || !owner->grant_table )
>      {
>          rcu_unlock_domain(d);
> +        if ( owner )
> +            put_page(page);
>          return -EPERM;
>      }
>  

Seriously?

We debated this at length in the security team, even the fact that
XSA-520 only applied to Xen 4.21 and older.

Now you're saying that claim in both the XSA-520 advisory and patch are
wrong, and therefore the owner path was not in fact dead code and a
MISRA violation.  This is the only reason why the patch ended up in this
form...

Nothing in CI noticed, not even the STATIC_SHM tests.


What I would like to do is a straight revert of bec8e3c1fb91, stating
that the claim is false, then apply the XSA-520 hunk with a different
commit message.   That way at least the logic ends up the same on all
branches of Xen (in this patch, you're reversed with rcu_unlock_domain()
vs the XSA-520 fix).

Thoughts?

~Andrew