[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash



On 19.04.22 10:42, Jan Beulich wrote:
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.

Seems like a good idea.

Will send V2.

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.

I think this should be another patch, though.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.