On 16.04.2022 15:31, Juergen Gross wrote:
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -308,7 +308,9 @@ long cf_check 
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>          if ( op->domain == DOMID_INVALID )
>          {
>      case XEN_DOMCTL_createdomain:
> +#ifdef CONFIG_GDBSX
>      case XEN_DOMCTL_gdbsx_guestmemio:
> +#endif
>              d = NULL;
>              break;
>          }

Wouldn't we be better off simply deleting this case label? dbg_rw_mem()
resolves the domid anyway (exactly as done a few lines down from here),
so I don't see why we couldn't pass a struct domain * there instead of
a domid_t.

This would also reduce the risk of further similar "overrides" appearing
here (taking existing instances as "excuse"), and breaking things again
in a similar way.

And finally I think iommu_do_domctl() needs making resilient against d
coming in as NULL. This isn't just to cover the issue here, but perhaps
more importantly because XEN_DOMCTL_test_assign_device can legitimately
end up having NULL passed here (when the caller passed DOMID_INVALID).
We've simply been lucky that libxl doesn't use this variant of calling
this domctl. I guess when d is NULL we ought to check the global flag
there rather than the per-domain one.

Jan


Reply via email to