Re: [PATCH] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash

[Juergen Gross <jgross@xxxxxxxx>]

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