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

Re: [PATCH] xen: fix XEN_DOMCTL_gdbsx_guestmemio crash


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 19 Apr 2022 10:42:32 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=p/woneOYshgtmIMld/+jQHXDH1F7e042Iv3EGlBwRic=; b=RxkT5ANujIG+6d2ED8a3/m+8cE6la28XSvhGOzZy9HpJ+Do8Xp+ymFoshghIjTy75aMACRQ/btBrFxMuhtn2Et9j2ZCF6nzjUTZcockSz7XjoTUc8jjzafVFhqDSPele+GEX6a84J0xHUruJ5MP9R+uK0pwc0vd1hzzOLDVBtapNaX6/Z0nkjtiEP90ruR4dhEOUVIngKNqJXFKF+38Nv5c+5J4dhWjPDzucoS+mOsVpsvSnrV5zIsnWun0z35Y70up4LpBGjAAawuwpCKDXl4WaskdXnhgU4/6Siq78MeEN/HmE/zlKSJXYPwT2soouaoLI68/y2ob8ikym+54aMA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SlSgiuVEA4Xa9RFzZZhal39lKlAGnKfexY5y7ODxShQaYKGzTP7zsC2HMALa3c7wGIqpBQQ3GsKl/goG47FgTWSP/YnxwBrsepLYhnm+E4+EzsQyV6psrEuSapvyMy6x/QddjoS2CBa5fcno+8KgAjMkSuRAXqTHp4/FQSP4brdLalKx8CZ2IHgdEMmnBSELIVKskkVErHUPoMrrKnKgSueSvH5UW8itnDSybpfmTse9ILViqKVRbmA2fCnsVyicQZ8oi+JJWD0JzW7RYM9omO1zuSjxHsAoaJwJOPvaSa1PV/BatO1e+IPF9VwiDtnS+b67p7JojvrFdeF4br/9zg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Cheyenne Wills <cheyenne.wills@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Tue, 19 Apr 2022 08:42:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

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