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

Re: [PATCH v2 3/7] x86/altcall: Optimise away endbr64 instruction where possible


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Mon, 14 Feb 2022 16:03:47 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=StROPKuxRDWHKFtz/wutII19P4YWxQwb/TJitVL2oMI=; b=NYNI+72uipR6a4GTXx9ab8wEZ2LDfHyBVKBSuM9+2Ii7yGJOYkKWkaeHVs7r5kSBW3sixJ/0WFC+VtGQBHxn0W8GvTtbbXVMMN6U9xh77qxKoqt44uWnTZFin4ezQp/wlCENsh1XC0bwN9fEsgmiq0pnsBXnVUaxKPSSKWrRiBAYiSXToYVbxgipHmOhxROitWZkPkivE0zlHyHXG0A2+stNxG1TWoEzS0oHfEuIyhXMJAh60WnqeUK4+Z/4lj5U/M/rieGbP4r9iEDCz/1lR9ItjJbssuhj6DE7Aw55I9gAmncX2gzRmJnLYKxN018BOpA/+0G+KcRBgnz1U0Dqdg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QIv5L6bdN1As3NPOqhgTXyY34aGrl5gh336Ba/XFBUtkF+THyiRcPoOCgkzxYoOPBRul8J+7uuidW+Coh70b1m7bnI55yj/Nz3uhALDXkCy2OosB8LuhWyLGzOk0Utb35g2y/uqMSbGT1apUmbEThGxs8wMLo6poYJz44+5ceSLekPc34IR5klclDowj5w6jMU+Xvh6SH442VImR/hJxuMCaEsIqRAtA0VG1+ItB/bSd1kOGLTUNuaxq05IioKsL6TxDVU8ipBGhlIghQ5Yt1faTO/L00DZ5bCQtsn6V2tjjHE6X2QB4UxLq3hhqZ3XVvSS2nCZDuELPyGuboXU4Yw==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 14 Feb 2022 16:03:57 +0000
  • Ironport-data: A9a23:MqqHt6NVg0E/+trvrR1xkMFynXyQoLVcMsEvi/4bfWQNrUoqhTFRy WJNXzrVaPjbNDT8fY1zOt7k/ENV7JWAztdrTgto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdpJYz/uUGuCJQUNUjMlkfZKhTr6UUsxNbVU8En1500s/w7RRbrNA2rBVPSvc4 bsenOWHULOV82Yc3rU8sv/rRLtH5ZweiRtA1rAMTakjUGz2zhH5OKk3N6CpR0YUd6EPdgKMq 0Qv+5nilo/R109F5tpICd8XeGVSKlLZFVDmZna7x8FOK/WNz8A/+v9TCRYSVatYo27UhIt8y etEj7itQ18qZInBuctAECANRkmSPYUekFPGCX22sMjVxEzaaXr8hf5pCSnaP6VBpLwxWzsXs 6VFdnZdNXhvhMrvqF6/YsBqit4uM4/AO4QHt2s75TrYEewnUdbIRKCiCdpwgmxu35gUQKa2i 8wxSyJhQTWQYx91Y3QTN50GgfjwhkDbWmgNwL6SjfVuuDWCpOBr65D9PdyQdtGUSMF9mkeDu nmA72n/GgsdNtGU1XyC6H3ErvDLtTP2XsQVDrLQ3vxgjUCXx2cTIAYLTlb9qv684nNSQPoGd RZSoHB36/Fvqgr7FbERQiFUvlbf4wVHVttuGtce9QvVyfDw7ie2Wys9G2sphMMdiOc6Qjkj1 1msltzvBCByvLD9dU9x5ot4vhvpZ3FLcDZqiTssCFJcvoK9+N1bYgfnE447eJNZmOEZDt0ZL 9qiiCElz4segscQv0lQ1QCW2mn8znQlo+Nc2+k2Yo5Hxl4jDGJGT9bxgbQ+0RqnBNzHJmRtR FBex6CjABkmVPlhbhClTuQXB62O7P2YKjDailMHN8B/q2jyqy7+INgJu2sWyKJV3iEsI2GBX aMukVkJuM870IWCMcebnL5d++x1lPO9RLwJp9jfb8ZUY4gZSeN01HoGWKJk5Ei0yBJEufhmY f+zKJ/wZV5HWfUP5GfnHI81jO50rh3SMEuOHPgXOTz8iuHADJNUIJ9YWGazghcRsv/V/lWNq 4wHXyZIoj0GONDDjuDs2dd7BXgBLGQhBICwrMpSd+WZJRFhFn1nAPjUqY7NsaQ890iMvuuXr Hy7RGFCz1/z2S/OJQmQMygxY7LzR5dv63k8OHV0b1qv3nEiZ6ep7bseKMRrLeV2qrQ7wK4mV eQBduWBHu9LFmbN9QMCYMSvt4dlbhmq216DZnL3fDglcpd8bAXV4du4LBD3/SwDA3Pv58szq rGtzC3BRp8HS1gwBcracqv3nViwoWIciKR5WE6Reotff0Dl8Y5LLS3tj6Bof5FQeEubnjbDj lSYGxYVo+XJsrQZytiRiPDWtZqtHst/AlFeQzvR44GpOHSI5WGk24JBDrqFJGiPSGPu9ay+T uxJ1PWgYuYflVNHvocgQbZmyaUyu4nmq7NAl1k2GXzKaxKgC696I2nA1s5K7/UfyrhcsAqwe 0SO5tgFZunZZJK7SAYcdFg/c+CO9fAIgT2Dv/06LXLz6DJz4LfaA15ZOAOBiXAFIbZ4WG//L TzNZCLCB9SDtycX
  • Ironport-hdrordr: A9a23:xbE/8KjeI9PU7xD8ZuGbuQVj6nBQX3513DAbv31ZSRFFG/FwyP rAoB1L73PJYWgqNU3IwerwRZVpQRvnhPtICRF4B8btYOCUghrVEGgE1/qi/9SAIVywygc578 ldmsdFeaTN5DRB/KXHCUyDYqwdKbq8geCVbIXlvg9QpGhRAskKhWYYNu/YKDwMeOAvP+tiKH P23Lsim9PUQwVwUi3NPAhjYwGsnayoqLvWJTo9QzI34giHij2lrJTgFQKD4xsYWzRThZ8/7G nsiWXCl+aemsD+7iWZ+37Y7pxQltek4MBEHtawhs8cLSipohq0Zb5mR6aJsFkO0aOSARcR4Z zxSiUbToNOAkDqDyeISNzWqlDdOQMVmjvfIJmj8CPeSILCNWkH4oF69P1km1PimjQdVZdHof 92Niuixupq5VmrplWN2/HYEx5tjUa6unwkjKoaiGFeS5IXbPtLoZUY5149KuZLIMvW0vFuLA BVNrCW2B+WSyLvU1nJ+m10hNC8VHU6GRmLBkAEp8yOyjBT2HR01VERysATlmoJsMtVcegJ28 3UdqBz0L1eRM4faqxwQO8HXMusE2TIBRbBKnibL1jrHLwOf3jNt5n06rMo4/zCQu1E8LIi3J DaFF9Iv287fEzjTcWIwZ1Q6xjIBH6wWDz8o/surqSReoeMMoYDHRfzOmzGovHQ1Mn3WPerKM pbEKgmdsPeEQ==
  • Ironport-sdr: oJ0/g/ThcQyWzIl9twQR8eq5Km5nVpSLzQ+q4Kl4BF0j/W6wjiG+RILhvx8fEb4LoB2Jcj23Ln RNyvWXFWQTRhdtavlToX8PycVOpRg/P2UhXZSAcP/xV6ufc2sKGL/CH02w4CWIfANTYTWLIonv oc4eCsZFMbyr+UGG4vw2foaGsNrg9PKZFFj0wKHL9i+QRwje0v8Nt6PWHGkK1FTwms5qzsqki0 TSX0/lJP0D13tFqEQkzKxVUJbgJqni2waSdaQk1WFRrSFkvh/jsmtUnYAV156bXUQrVMd6htOG 1mjnf9FOAlR1bQvHs+GEXfeV
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYIaJdlBm1laP+30eCBtzXO5xQTKyTA/+AgAAG84CAAAW9AIAAJPGA
  • Thread-topic: [PATCH v2 3/7] x86/altcall: Optimise away endbr64 instruction where possible

On 14/02/2022 13:51, Jan Beulich wrote:
> On 14.02.2022 14:31, Andrew Cooper wrote:
>> On 14/02/2022 13:06, Jan Beulich wrote:
>>> On 14.02.2022 13:56, Andrew Cooper wrote:
>>>> @@ -330,6 +333,41 @@ static void init_or_livepatch 
>>>> _apply_alternatives(struct alt_instr *start,
>>>>          add_nops(buf + a->repl_len, total_len - a->repl_len);
>>>>          text_poke(orig, buf, total_len);
>>>>      }
>>>> +
>>>> +    /*
>>>> +     * Clobber endbr64 instructions now that altcall has finished 
>>>> optimising
>>>> +     * all indirect branches to direct ones.
>>>> +     */
>>>> +    if ( force && cpu_has_xen_ibt )
>>>> +    {
>>>> +        void *const *val;
>>>> +        unsigned int clobbered = 0;
>>>> +
>>>> +        /*
>>>> +         * This is some minor structure (ab)use.  We walk the entire 
>>>> contents
>>>> +         * of .init.{ro,}data.cf_clobber as if it were an array of 
>>>> pointers.
>>>> +         *
>>>> +         * If the pointer points into .text, and at an endbr64 
>>>> instruction,
>>>> +         * nop out the endbr64.  This causes the pointer to no longer be a
>>>> +         * legal indirect branch target under CET-IBT.  This is a
>>>> +         * defence-in-depth measure, to reduce the options available to an
>>>> +         * adversary who has managed to hijack a function pointer.
>>>> +         */
>>>> +        for ( val = __initdata_cf_clobber_start;
>>>> +              val < __initdata_cf_clobber_end;
>>>> +              val++ )
>>>> +        {
>>>> +            void *ptr = *val;
>>>> +
>>>> +            if ( !is_kernel_text(ptr) || !is_endbr64(ptr) )
>>>> +                continue;
>>>> +
>>>> +            add_nops(ptr, 4);
>>> This literal 4 would be nice to have a #define next to where the ENDBR64
>>> encoding has its central place.
>> We don't have an encoding of ENDBR64 in a central place.
>>
>> The best you can probably have is
>>
>> #define ENDBR64_LEN 4
>>
>> in endbr.h ?
> Perhaps. That's not in this series nor in staging already, so it's a little
> hard to check. By "central place" I really meant is_enbr64() if that's the
> only place where the encoding actually appears.

endbr.h is the header which contains is_endbr64(), and deliberately does
not contain the raw encoding.

>
>>>> --- a/xen/arch/x86/xen.lds.S
>>>> +++ b/xen/arch/x86/xen.lds.S
>>>> @@ -221,6 +221,12 @@ SECTIONS
>>>>         *(.initcall1.init)
>>>>         __initcall_end = .;
>>>>  
>>>> +       . = ALIGN(POINTER_ALIGN);
>>>> +       __initdata_cf_clobber_start = .;
>>>> +       *(.init.data.cf_clobber)
>>>> +       *(.init.rodata.cf_clobber)
>>>> +       __initdata_cf_clobber_end = .;
>>>> +
>>>>         *(.init.data)
>>>>         *(.init.data.rel)
>>>>         *(.init.data.rel.*)
>>> With r/o data ahead and r/w data following, may I suggest to flip the
>>> order of the two section specifiers you add?
>> I don't follow.  This is all initdata which is merged together into a
>> single section.
>>
>> The only reason const data is split out in the first place is to appease
>> the toolchains, not because it makes a difference.
> It's marginal, I agree, but it would still seem more clean to me if all
> (pseudo) r/o init data lived side by side.

I still don't understand what you're asking.

There is no such thing as actually read-only init data.

Wherever the .init.rodata goes in here, it's bounded by .init.data.

~Andrew

 


Rackspace

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