[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/8] x86/shadow: properly handle get_page() failing
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Wed, 27 Jul 2022 19:06:31 +0000
- Accept-language: en-GB, en-US
- 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=w3X0sSNPYg3fWQo3nObExbbQryl3IJ699yK/+vB1REU=; b=f27/Dgkw2O/Y0T5jrtX6/sZsc5owWfxXc/X4FoVkaWESrJ1jCX1mzRUdp0etiGhziPxyTKzGxh6Yfg0d3bVlS4nQEuYtoFuHOKHgI6oBcufpcwJRRzdNDBxXcEMBia996TaAKZ6eOBEB43tGZ/iMeATe1GUV2s2u4+Crg+0jA76nLYPAR7iTsmZQK0w2ys4Zrd1rWge4zG8ctjOmzzxrnVT5n+pHNOKNfoPPBwUU3MNmui7VuG4Kh/1VYtf+BFkeeoieobUH1GkGTx2AMxP+VWZjYwYu2eD85nomps8Des+/MjcdASoxmmZBLQEpM1AWsWWd9QIk4LFq35HIac5iDA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EJQ11MICVY0qhdeIYDJtZVNjBUewDVcap5kTcVj8hUmx/CCXXdvmvt5vB58l8n6E5GuOF9VvAMn3IODWaeYh8jxyoVON9L20Omtw/fD7feohPT2H+ijOInqL6puYK+9g1vmOw1abw+2MTBoihZptBYwqrmmZwgmAZWM0r/eTSHgUxRmpBLz7xvA/Hqu3/BVQ4ilmPj2bOvjyPGxyx4a5gEV7htMzZU0aTdDitv1+FvjHOggRkVKjBkXN5YxwrBOUJnxZlXPIgvEoSL59fAfu8exSBE5YJwp+ESmIn87JU6BTN4t3je5JYq8FamjJuZJZoJYABVFNplrqDne2ZzuyFg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, "Tim (Xen.org)" <tim@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Wed, 27 Jul 2022 19:07:03 +0000
- Ironport-data: A9a23:ddYj16haUXtqFdIVUvWAUd0tX161TREKZh0ujC45NGQN5FlHY01je htvW2+FaKqIYGSmLYogbo62p0JU6JLRnIRrSVBtri9mH3sb9cadCdqndUqhZCn6wu8v7a5EA 2fyTvGacajYm1eF/k/F3oDJ9CU6j+fQLlbFILasEjhrQgN5QzsWhxtmmuoo6qZlmtH8CA6W0 T/Ii5S31GSNhnglaQr414rZ8Ek15Kuo52tC1rADTasjUGH2xiF94K03fcldH1OgKqFIE+izQ fr0zb3R1gs1KD90V7tJOp6iGqE7aua60Tqm0xK6aID76vR2nQQg075TCRYpQRw/ZwNlPTxG4 I4lWZSYEW/FN0BX8QgXe0Ew/ypWZcWq9FJbSJQWXAP6I0DuKhPRL/tS4E4eDdUdocBZPXl1q +URciwtKR+oit+S+efuIgVsrpxLwMjDGqo64ysl6A6DSPEsTNbEXrnA4sJe0HEonMdSEP3CZ s0fLz1ycBDHZB4JMVASYH48tL7w2j+jLHsG9hTJ/cLb4ECKpOB1+JHrPMDYZZqhQsJNk1zDj mnH4374ElcRM9n3JT+trSzx2L+RxnuTtIQ6P7S12OYzkRqpxUsvFzdHWWaV5vXghRvrMz5YA wlOksY0loAw/kG2Stj2XzWjvWWJ+BUbXrJ4DOkS+AyLjK3O7G6xGmkBZi5MbpohrsBebSwn0 BqFks3kARRrsaaJUjSN+7GMtzSwNCMJa2gYakc5oRAt5tDipMQ2kUjJR9M6Sqqt1IWpQXf33 iyAqzU4i/MLl8kX2q6n/FfBxTWxupzOSQ1z7QLSNo640j5EiEeeT9TAwTDmATxode51knHpU KA4pvWj
- Ironport-hdrordr: A9a23:9wxUwaylCzH91j+4Q8G/KrPxdegkLtp133Aq2lEZdPULSKGlfp GV9sjziyWetN9IYgBapTiBUJPwIk81bfZOkMUs1MSZLXPbUQyTXc5fBOrZsnDd8kjFmtK1up 0QFJSWZOeQMbE+t7eD3ODaKadv/DDkytHPuQ629R4EIm9XguNbnn5E422gYy9LrXx9dP4E/e 2nl696TlSbGUg/X4CePD0oTuLDr9rEmNbNehgdHSMq7wGIkHeB9KP6OwLw5GZfbxp/hZMZtU TVmQ3w4auu99uhzAXH6mPV55NK3PP819p4AtCWgMR9EESutu/oXvUiZ1SxhkFwnAid0idsrD AKmWZnAy1H0QKVQohym2q15+Cv6kd315ao8y7kvZKqm72EeNt9MbsBuWsRSGqm16Jr1usMr5 5jziaXsYFaAgjHmzm479/UVwtynk7xunY6l/UP5kYvGbf2x4Uh37D30XklZqvoJhiKobwPAa 1rFoXR9fxWeVSVYzTQuXRu2sWlWjA2Eg2dSkYPt8SJ23wO9UoJhXcw1YgahDMN5Zg9Q55L66 DNNblpjqhHSosTYbhmDOkMTMOrAijGQA7KMmiVPVP7fZt3cE7lutry+vE49euqcJsHwN87n4 nASkpRsSood0fnGaS1rep2G9D2MRGAtBjWu7FjDsJCy8zBrZLQQF6+YUFrlde8qPMCBcCeU+ qvOfttcoreEVc=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHYoQlhV37xX0Hh2kWLBKJVPAkGN62SK82AgAABxgCAAGhPAA==
- Thread-topic: [PATCH 2/8] x86/shadow: properly handle get_page() failing
On 27/07/2022 13:53, Jan Beulich wrote:
> On 27.07.2022 14:46, Andrew Cooper wrote:
>> On 26/07/2022 17:04, Jan Beulich wrote:
>>> --- a/xen/arch/x86/mm/shadow/multi.c
>>> +++ b/xen/arch/x86/mm/shadow/multi.c
>>> @@ -132,7 +132,8 @@ delete_fl1_shadow_status(struct domain *
>>> SHADOW_PRINTK("gfn=%"SH_PRI_gfn", type=%08x, smfn=%"PRI_mfn"\n",
>>> gfn_x(gfn), SH_type_fl1_shadow, mfn_x(smfn));
>>> ASSERT(mfn_to_page(smfn)->u.sh.head);
>>> - shadow_hash_delete(d, gfn_x(gfn), SH_type_fl1_shadow, smfn);
>>> + if ( !shadow_hash_delete(d, gfn_x(gfn), SH_type_fl1_shadow, smfn) )
>>> + ASSERT_UNREACHABLE();
>> I'd recommend crashing the domain here too. I know the we've already
>> got the return value we want, but this represents corruption in the
>> shadow pagetable metadata, and I highly doubt it is safe to let the
>> guest continue to execute in such circumstances.
> I'm happy to add or convert, but please clarify: DYM in addition to
> the assertion or in place of it?
Erm pass.
We still don't have a sensible construct for ASSERT_OR_DOMAIN_CRASH(),
or a well-thought-through piece of guidance.
Probably keep the ASSERT() ? Almost all state checking in the shadow
code is via asserts.
Mostly what I'm concerned by is it feeling weird to have one path which
only domain crashes, and one path which only asserts.
~Andrew
|
