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

Re: [PATCH] x86/boot: Restrict directmap permissions for .text/.rodata


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 24 Mar 2023 14:40:04 +0000
  • 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=gT0bYj5k2GcZSQ76d4PpQEnfQLUlmVolK1Tz/ZW7cWs=; b=P+cJ/H1T4F4dKkg/Qmfd5OaqQ9wHhnILr9CZmBeQADinQYfR6Rr5Ezg2Dh5BHKn8eiFghZwuecTwYzLU9hXH/IJVt5SmOTrUSqgR9pSfYFehEG22LFsRAnvFwUkwMtflBHrN0fC9k26r+NoS/IqBG7jjmqfCs67tXMi4eJ5CTb6pQjFMxiNI1ydI8PzpD0Yj51wx1u70jxpb1TSnOXDSD7ZEdULTeZBd51XU3iw9AwQsbChMikHvDPrPSLgR/sf7g0ue/rx+FIfTSqsFKeoUDtX5YjJ/XSHK+CUoHOqBKrkeB+1gyHsoaL3Zhc1SPgltKFHXLt3/KW+1XWkZphOEuA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LHveL29O0MSuPDwQTdWCrlqcFJi8jkrELHB88dCcOqII+zrasU6ee9XB8NUiYFkjIaO8RNK7ggo1OuC7bIrqVfrkHSuQITGBN6e5lYe6xQ2zVkJQvGvLSd7m981AXVlXqFGJWkBvFkcYkRR/wY9RCB6JVA2Ltn8hCUkkKjbWsUgN2pA5HDmqdGg+5LSVnPyQuIX1sA0QhioGuxn9ybwm7m/TQsfq4xbUaVnOqpu3dtmSj4tHATZ4Vvz3xyU3WS31zuaW0wfVsWxB4s2UrSOjrm2IMaPyS/fieWnbhaCypyY0/bKoJRDX9D2r5g1UYGWNOTRGn5gdf1QiazIFlUxRyA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 24 Mar 2023 14:40:30 +0000
  • Ironport-data: A9a23:IktT+6y07ZEjo3zhEAl6t+cVxyrEfRIJ4+MujC+fZmUNrF6WrkUAz mQWXWHSPa6CYjDweNF/PNm19E8B6MCBz99iTgdv/yAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTrafYEidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw//F+UMHUMja4mtC5QRlP6wT5jcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KVwX1 tMoJ20PUiugnceT2763ZrYyhP12eaEHPKtH0p1h5RfwKK58BLrlGuDN79Ie2yosjMdTG/qYf 9AedTdkcBXHZVtIJ0sTD5U92uyvgxETcRUB8A7T+fVxvjGVkFMZPLvFabI5fvSjQ8lPk1nej WXB52njWTkRNcCFyCrD+XWp7gPKtXqjBNxLROzir5aGhnWh92Y1KhhIXmegqPuYoH+RVdADF HUbr39GQa8asRbDosPGdx+yrWOAvxUcc8FNCOB84waIooLL5y6JC25CSSROAPQ2uclzSTE02 1uhm9LyGScpoLCTUWia9LqfsXW1Iyd9EIMZTSoNTA9A6d+6pog21kjLVow7TPHzicDpEzbtx TzMtDI5m7gYkc8M0eO84EzDhDWv4JPOS2bZ+znqY45s1SshDKbNWmBiwQKzASpoRGpBcmS8g Q==
  • Ironport-hdrordr: A9a23:SLNKRq1k2GsgHuo7LBg+vwqjBHUkLtp133Aq2lEZdPRUGvbo8f xG/c566faQsl0ssR4b+OxoVJPwJE80lqQFmLX5X43SJDUO0VHARO4N0WKL+UyaJ8SUzJ846U 4PSdkYNPTASXVBoILdxiLQKbodKd+8mpyAtKPl400oZydMRIFP0zxQNya8NQlNaDQuP+tbKL OsosVGoja7eWcadK2Aa0UtVfTYutvOmInHTHc9dnwa1DU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 06/12/2021 3:21 pm, Jan Beulich wrote:
> On 06.12.2021 16:11, Andrew Cooper wrote:
>> On 06/12/2021 13:58, Jan Beulich wrote:
>>> On 06.12.2021 14:08, Andrew Cooper wrote:
>>>> While we've been diligent to ensure that the main text/data/rodata
>>>> mappings
>>>> have suitable restrictions, their aliases via the directmap were
>>>> left fully
>>>> RW.  Worse, we even had pieces of code making use of this as a
>>>> feature.
>>>>
>>>> Restrict the permissions, as we have no legitimate need for
>>>> writeability of
>>>> these areas via the directmap alias.
>>> Where do we end up reading .text and/or .rodata through the
>>> directmap? Can't
>>> we zap the mappings altogether?
>>
>> I felt it was safer to keep readability via the directmap.
>>
>> I'm not aware of any logic we have which reads the directmap in order,
>> but it ought to be possible.
>
> Could you add a sentence to this effect to this description, please?

Ok.  The commit message a rewrite anyway, given changes to the boot cpu
stack.

>
>>> As to superpage shattering - I understand this is not deemed to be
>>> an issue
>>> in the common case since, with Xen moved as high up below 4G as
>>> possible,
>>> it wouldn't normally live inside a 1G mapping anyway? This may want
>>> calling
>>> out here. Plus, in non-EFI, non-XEN_ALIGN_2M builds isn't this going to
>>> shatter a 2M page at the tail of .rodata?
>>
>> cpu0_stack has already shattered down to 4k, which is likely in the same
>> superpage as rodata in a non-2M build.
>>
>> But at the end of the day, it is a security/performance tradeoff.
>>
>> memcpy(__va(__pa(divide_error)), "\x0f\x0b", 2);
>> asm ("div %ecx" :: "c" (0));
>>
>> is an especially low barrier for an attacker who has a partial write
>> gadget.
>>
>> The security benefits are substantial, and the perf downsides are a
>> handful of extra pagetables, and a handful of pagewalks taking extra
>> steps, in non-fast paths (i.e. distinctly marginal).
>
> How do you easily know what paths there are accessing data on the same
> (potential) superpage? However, thinking about it, with the directmap
> mapping presumably not getting used at all, how the mapping is arranged
> doesn't really matter (except for the extra memory needed, but as you
> say that's probably marginal).

Indeed.  Any path which requires Xen to reach into guest state via the
directmap isn't a fastpath in the first place.

~Andrew



 


Rackspace

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