[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/vPIC: check values loaded from state save record
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 25 Oct 2023 12:12:12 +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=TKpIPWAbwNNkHC6pCqCF07XWz1U58EjKJ5Jpd1HjDXE=; b=nZoUYqyJvgqtOzLIMcm9dNs7hWk6zyLlmdWsWRq1O4wMX4ubiaB0iVGberl0Z6gFs11vn+Vk6BEeIeO7kOhJ/IXZ1glTnkMd6eGDwi7b8z4NAP6VzB+LmpMmppLW/SYO2B41gDHH2JXmcHF5DE/9C6a4D97yDPo2CPuFoZ0sRiL2WmYL9f3yAa8Dp6JflapRvLhlu6w5DHPdGrbaZEarUl6TriJKJrQLpUHdDyapVlRFqATsf6vb5t/3xWYDDzMb9kk+dkbEINBzKFCRv/C1eFchH31JGNe2UIObkR7/aL0ce+yEEAU2KbQPQjKI4eHhBh5wjl1WYSP3o5WBjlh+Gw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hQXzDa+sAaWx+dikqhhrJcz0kPN08ysKBOp2ULP2ZXXtnJSMKlJO/nFFbEZX82x6JBvzyTxNuqK8VEK1Tn6nZycuf2+9V3Ea9fRwQSnF6jsnV1uEHoHfYdW6XYUiX9SkXqoS7FIyQTYEg+fvfbdNdfrAYhjr2QBciq8to+DVTQltfXiLAkuwtyxrMWuAsVq48XuFnl4tzEP9OVomeagty8GkerXQwFfrAOXJcF3CIxvrXMA3twzCx1hLENBKLKIw+CAwi2FxCSqeIRMKiskvjB8bNZPfW+S7pPyQc9lzA/aJPA7keSd6f5mdlt9lqz8T+0qZ+dR8FNCYaieNK+/0Iw==
- 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>
- Delivery-date: Wed, 25 Oct 2023 10:14:00 +0000
- Ironport-data: A9a23:iXTQzKOZveEKS0nvrR2DlsFynXyQoLVcMsEvi/4bfWQNrUp332BRn GRLUWmOb6yMYGOgf4h+O4+w9h5UuJfSn9VrSgto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CQ6jefQAOOkVIYoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGjxSs/nrRC9H5qyo42pA5AFmPJingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0uhPJl5Jz vhbES8ibg6anf6K/5u4Y9A506zPLOGzVG8ekldJ6GiASN0BGNXESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+PRxujaDpOBy+OGF3N79YNuFSN8Thk+Fj mnH4374ElcRM9n3JT+tqyjy2rKSx3mmMG4UPKSA0KFxh0CC/FYOAgMIdGTg+/Kf20HrDrqzL GRRoELCt5Ma9kamU938VB2Qu2Ofs1gXXN84O/037kSBx7TZ5y6dB3MYVXhRZdo+rsg0SDc2k FiTkLvBGjhHoLCTD3WH+d+pQSiaPCEUKSoJYnYCRA5cut37+tht3lTIU8ppF7OzgpvtAzbsz juWrS84wbIOkcoM0Kb99lfC696xmqX0oscOzl2/dgqYAslRPuZJu6TABYDn0Mt9
- Ironport-hdrordr: A9a23:gcBd5q7Hvd8L1GBceAPXwcGCI+orL9Y04lQ7vn2ZFiY5TiXIra qTdaogviMc7wxhLU3I+OrwSpVoJEm9yXcb2/hgAV7PZnibhILsFvAA0WKA+UySJ8SdzJ8h6U 4IScEXZLKQMbE5t7eZ3ODfKade/DDuytHduQ609QYdcegeUdAE0+4WMHfkLqQ6fngLObMJUL 6nouZXrTupfnoaKuy9G3k+RuDG4/nGjojvbxIqDwMurFDmt0Ln1JfKVzyjmjsOWTJGxrkvtU DDjgzC/62m99W20AXV2WP/54lf3PHh1txALsqRjdV9EESiti+YIKBaH5GStjE8p++irH4sjd n3uh8le/9+7nvAF1vF6icEj2PbvgoG2jvH8xu1kHHjqcv2SHYREMxan79UdRPf9g4JoMx8+L gj5RPci7NnSTf72Ajt7dnBUB9n0mCup2A5rOIVh3tDFaMDdb5qq5AF9k89KuZGIMvD0vFgLA BSNrCf2B8WSyLYU5nhhBgu/DV4ZAV3Iv+EKnJy8vB9nQImxUxR/g89/og+hXcbsLYwVpFo7/ nNW54Y7o2mdfVmJJ6VNN1xN/dfNVa9Ni4kEFjia2gPR5t3dk4l7aSHoYkd1aWUQqZN4IQ/n4 mEcFRZqG47ZkT0YPf+paFjw1Tzel/4cSvgzdwbx594p731WbzxdQWYQlAv+vHQ28k3M4nlYc z2FI1bBOKmEHDvF4hExGTFKuJvAEhbb/cu/volV1WS5v/aM4GCjJ29TN/jYIXWK3IfUGXnDh I4LVzODfQF0XqQHkXliwLaQHP1Z0z4+tZRPcHhjo4u9LQ=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Thu, May 11, 2023 at 01:50:33PM +0200, Jan Beulich wrote:
> Loading is_master from the state save record can lead to out-of-bounds
> accesses via at least the two container_of() uses by vpic_domain() and
> __vpic_lock(). Calculate the field from the supplied instance number
> instead. Adjust the public header comment accordingly.
>
> For ELCR follow what vpic_intercept_elcr_io()'s write path and
> vpic_reset() do.
>
> Convert ->int_output (which for whatever reason isn't a 1-bit bitfield)
> to boolean, also taking ->init_state into account.
>
> While there also correct vpic_domain() itself, to use its parameter in
> both places.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Of course an alternative would be to simply reject state save records
> with bogus values.
Likewise on the vPIC one, I feel it might be better to just reject
such bogus entries, instead of attempting to amend them.
This one however just unconditionally reset some values, but might be
simpler to just test if is_master == !inst and if it's master than bit
2 in s->elcr is set?
Also if we are serious about doing some sanity checks in the loaded
records, we could introduce a checker function for the load machinery.
Thanks, Roger.
|
