[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 9/9] x86/shadow: harden shadow_size()
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Wed, 11 Jan 2023 23:15:54 +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=slJOBE0mbK8K/SCPeC6NuKHHLlciEoPKDGwPLBhaokI=; b=ChrJ9VNzWyny/uenxMrvdJwkgbXsHOUb2ago5wakIF+kbQW4Qz0pjGh8lEgaRAAupxwyjKd7jP25oYs/g+xJBD4Oq8sHiq9FagexIZiV6oWB1bf/Cga6g3pw/PpeVXTwtPZgKiScq1RiLUtrSr0IVrAVLQYbaDJrxOY82I+xRVagV0XkeyUqZH9jOfA+nIlTdxetj4rgoQ0RUILv1RdGrUapcx77EnrYj6Tio5DkrBcqYZngJtt2ZsENr1+xdy1LzYzT28Kr64a0f4eFFNb6MajoYaQju+uJ9Z9tZKdyo+Os1gprlvvfesy2/9VxCm/ypBUrjpUG4XrHk7s9FDBwLA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Nds3jGcURGqMSuukeSyz5UDTRKTp0KFhT8LggkajqVuGM0VfHlqF6NQnOzxWAHjYV3yZecoOWWkS04+DI74M5RcAg8MQ8VuUxelyfQHSC5Og4uRDu59W4y22OjiZg4T57R/RUJIvtQS1+ZkgNsCUbuNDcZddglslXpY1/TSQiMo8poySYazlEO1xQDz1fBBU6I+iSLAS1+vqJF7YXnD8yyfcgoRvnkM1YVDgAJJStsaacQNihLaf7hgz8hJPdV4paqR5WfHmF71miA0ILePb+MaxXnFL2ay+tW/H1IvYTCjBuT9Lzmi7pidDcpvBvKQ2wwI82ir3Shf8fBjuY+eZLQ==
- 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>, "Tim (Xen.org)" <tim@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>
- Delivery-date: Wed, 11 Jan 2023 23:16:27 +0000
- Ironport-data: A9a23:Qmwatazk4O2zKbZ+teR6t+cGxyrEfRIJ4+MujC+fZmUNrF6WrkVRz mQWXzzSM/yNZmbwc4x2PISypE8Pv5KGyYRrGwZq+SAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTbaeYUidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw//F+U0HUMja4mtC5QRnPKkT5zcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KTl8+ /MYCCoXVRasjb2V+JeRTvNO1v12eaEHPKtH0p1h5RfwKK9/BLvkGuDN79Ie2yosjMdTG/qYf 9AedTdkcBXHZVtIJ0sTD5U92uyvgxETcRUB8A7T+fVxvjaVlVMouFTuGIO9ltiibMNZhEuH4 EnB+Hz0GEoyP92D0zuVtHmrg4cjmAuqBNxNS+XmrJaGhnWL31cCNjRKbmKy4tO7ulC6XYMEI BALr39GQa8asRbDosPGdx+yrWOAvxUcc8FNCOB84waIooLE7gDcCmUaQzppbN09qNRwVTEsz kWOnd7iGXpoqrL9YW2Z3qeZq3W1Iyd9BXMDYAcUQA1D5MPsyLzflTrKR9dnVaWy19v8HGipx yjQ9XdnwbIOkcQMyqO3u0jdhC6hrYTISQhz4RjLWmWi7UVyY4vNi5GU1GU3JM1odO6xJmRtd lBd8yRCxIji1a2wqRE=
- Ironport-hdrordr: A9a23:OEBT06664SBrvsnFyAPXwOPXdLJyesId70hD6qkRc20xTiX8ra rCoB1173PJYVoqN03I4OrwQZVoIkmsl6Kdg7NwAV7KZmCPhILPFu9fBODZsl7d8kPFl9K14p 0QF5SWWOeaMbGjt7eA3OBjKadH/DBbytHOuQ4D9QYUcei1UdAb0ztE
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHZJcS6xQlTtpJxX0O79yAhgOZJ+66Z2cOA
- Thread-topic: [PATCH v2 9/9] x86/shadow: harden shadow_size()
On 11/01/2023 1:57 pm, Jan Beulich wrote:
> Make HVM=y release build behavior prone against array overrun, by
> (ab)using array_access_nospec(). This is in particular to guard against
> e.g. SH_type_unused making it here unintentionally.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: New.
>
> --- a/xen/arch/x86/mm/shadow/private.h
> +++ b/xen/arch/x86/mm/shadow/private.h
> @@ -27,6 +27,7 @@
> // been included...
> #include <asm/page.h>
> #include <xen/domain_page.h>
> +#include <xen/nospec.h>
> #include <asm/x86_emulate.h>
> #include <asm/hvm/support.h>
> #include <asm/atomic.h>
> @@ -368,7 +369,7 @@ shadow_size(unsigned int shadow_type)
> {
> #ifdef CONFIG_HVM
> ASSERT(shadow_type < ARRAY_SIZE(sh_type_to_size));
> - return sh_type_to_size[shadow_type];
> + return array_access_nospec(sh_type_to_size, shadow_type);
I don't think this is warranted.
First, if the commit message were accurate, then it's a problem for all
arrays of size SH_type_unused, yet you've only adjusted a single instance.
Secondly, if it were reliably 16 then we could do the basically-free
"type &= 15;" modification. (It appears my change to do this
automatically hasn't been taken yet.), but we'll end up with lfence
variation here.
But the value isn't attacker controlled. shadow_type always comes from
Xen's metadata about the guest, not the guest itself. So I don't see
how this can conceivably be a speculative issue even in principle.
~Andrew
|
