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

Re: [PATCH][4.17] x86/shadow: drop (replace) bogus assertions


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 14 Oct 2022 12:30:29 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=/O70qrqLleir06Q+TDztBUljEHDJW+cZ4CXvcP9Rx8Q=; b=HVcJl3ombemQION7NQlSI027RlB1SjreaDk8DjTFCwWrvAEGtegYf94NUcd7w3HMPg+/65HcSQIJse2q4LeBFlU02KR/JBfkkKtKUtunkJKrNgFcrJMrh/AYYr75OcPf/8nAH9yUhrLfIZlpaYhVwb5FDu4w6O4ng77D1ErVevFOTB6EHX+Adj3rowf5RX0hkW/zVv8loycXtWMV3r+z6FdUypSS0sEI28DzuIkPL91eBEHcJyEEeWryXVNn4OS0T1TrsmIEqFuXoFqHR3fmolkk1/1zGN8INVO1GMHc73oF0rE/JWB2kB0cUjYgRO6TVS51I4mFoSZtzFnscLppAg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PD957D75Yrdf9lmPvUcVwG+E7bQaaZSSOdzVZXsr7Haeq3Xv/uApNjY8w/QcOXfg7xdhnWlRxR0437E+NA8SmHS4Xfju+bZ2A98X6/pL8EKdfv3VyGLGxcDmCi2C7yvBg5Q3s0l8uIGHOf5BAJ1ZfXOGf3J4UY9QBTWRKo8PEL8XlLiYwYmemfrAng3u1usZcSqdwqQQVwSnV3VcAek9kRJafIhw2uPg7a7+ipx/fbegXtF8mcCQHOZ9GrFW4oh7zH/kQEb3u/HmBVTWIctvtAPv52gnLTvE2AQJK+GOBdiUnClYmti5nw291IVjpvm0J+SasAkfg1llJ8f5J77q5w==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Tim Deegan <tim@xxxxxxx>, Henry Wang <Henry.Wang@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Fri, 14 Oct 2022 10:30:44 +0000
  • Ironport-data: A9a23:eXUAJqDY2l4bqhVW/xPiw5YqxClBgxIJ4kV8jS/XYbTApDN002BWy TAbUGmCOPeKYzTwc9h2ao+29RkG7MeEzoRiQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8mk/ngqoPUUIbsIjp2SRJvVBAvgBdin/9RqoNziJ2yDhjlV ena+qUzA3f4nW8pWo4ow/jb8kk25K2u4GlwUmEWPpingnePzxH5M7pHTU2BByOQapVZGOe8W 9HCwNmRlo8O105wYj8Nuu+TnnwiGtY+DyDX4pZlc/HKbix5jj4zys4G2M80Mi+7vdkrc+dZk 72hvbToIesg0zaldO41C3G0GAkmVUFKFSOuzdFSfqV/wmWfG0YAzcmCA2kfBIs9/Nt9UFhq8 NwGCwoMXz2v26GplefTpulE3qzPLeHNFaZG4jRK626cCvwrB5feX6/N+NlUmi8qgdxDFurfY MxfbidzaBPHYFtEPVJ/5JAWxb/0wCWgNWIA7gvN/MLb4ECKpOB1+KLqP9fPPMSDWO1en1qCp 3KA9GP8av0fHIzFkmTYrSj07gPJtTPjZY4/Eu2Vzcw0vlSC7WkJVyQ0UVTu9JFVjWb7AbqzM Xc8+CAjsKwz/0yDVcTmUluzp3vslg4RXZ9cHvM37CmJy7HI+ECJC24cVDlDZdc68sgsSlQCy Vuhj97vQzt1v9W9VXOY3qeZq3W1Iyd9BU8PYzUVCzQM5dbLqZs2yBnIS75e/LWdi9T0HXT6x W+MpS1n37EL15dTjuO84EzNhC+qqt7RVAkp6w7LX2WjqARkeIqiYI/u4l/ehRpdELukopC6l CBss6CjAComVvlhSATlrD0xIYyU
  • Ironport-hdrordr: A9a23:xqBmF6A6v+4iyRTlHeg+sceALOsnbusQ8zAXPh9KJCC9I/bzqy nxpp8mPH/P5wr5lktQ/OxoHJPwOU80kqQFmrX5XI3SJTUO3VHFEGgM1+vfKlHbak7DH6tmpN 1dmstFeaLN5DpB/KHHCWCDer5PoeVvsprY49s2p00dMT2CAJsQizuRZDzrcHGfE2J9dOcE/d enl4N6T33KQwVlUu2LQl0+G8TTrdzCk5zrJTYAGh4c8QGLyRel8qTzHRS01goXF2on+8ZozU H11yjCoomzufCyzRHRk0fV8pRtgdPkjv9OHtaFhMQ5IijlziyoeINicbufuy1dmpDm1H8a1P 335zswNcV67H3cOkmzvBvWwgHllA0j7nfzoGXo9UfLkIjcfnYXGsBBjYVWfl/y8Ew7puxx16 pNwiawq4dXJQmoplWz2/H4EzVR0makq3srluAey1ZFV5EFVbNXpYsDuGtIDZY7Gj7g4oxPKp gjMCjl3ocWTbqmVQGYgoE2q+bcHUjbXy32D3Tqg/blnQS/xxtCvgklLM92pAZ0yHtycegA2w 3+CNUZqFh/dL5pUUtDPpZxfSKWMB27ffueChPlHX3XUIc6Blnql7nbpJ0I2cDCQu178HJ1ou WKbG9l
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Oct 14, 2022 at 10:49:55AM +0200, Jan Beulich wrote:
> The addition of a call to shadow_blow_tables() from shadow_teardown()
> has resulted in the "no vcpus" related assertion becoming triggerable:
> If domain_create() fails with at least one page successfully allocated
> in the course of shadow_enable(), or if domain_create() succeeds and
> the domain is then killed without ever invoking XEN_DOMCTL_max_vcpus.
> 
> The assertion's comment was bogus anyway: Shadow mode has been getting
> enabled before allocation of vCPU-s for quite some time. Convert the
> assertion to a conditional: As long as there are no vCPU-s, there's
> nothing to blow away.
> 
> Fixes: e7aa55c0aab3 ("x86/p2m: free the paging memory pool preemptively")
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> A similar assertion/comment pair exists in _shadow_prealloc(); the
> comment is similarly bogus, and the assertion could in principle trigger
> e.g. when shadow_alloc_p2m_page() is called early enough. Replace those
> at the same time by a similar early return, here indicating failure to
> the caller (which will generally lead to the domain being crashed in
> shadow_prealloc()).

It's my understanding we do care about this because a control domain
could try to populate the p2m before calling XEN_DOMCTL_max_vcpus, and
hence could trigger the ASSERT, as otherwise asserting would be fine.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> While in shadow_blow_tables() the option exists to simply remove the
> assertion without adding a new conditional (the two loops simply will
> do nothing), the same isn't true for _shadow_prealloc(): There we
> would then trigger the ASSERT_UNREACHABLE() near the end of the
> function.

I think it's fine to exit early.

Thanks, Roger.



 


Rackspace

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