From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71B4DC43A1D for ; Wed, 11 Jul 2018 23:05:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1DF7C20C10 for ; Wed, 11 Jul 2018 23:05:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1DF7C20C10 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389741AbeGKXK5 (ORCPT ); Wed, 11 Jul 2018 19:10:57 -0400 Received: from mga18.intel.com ([134.134.136.126]:3235 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388115AbeGKXK5 (ORCPT ); Wed, 11 Jul 2018 19:10:57 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2018 16:04:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,339,1526367600"; d="scan'208";a="72011249" Received: from 2b52.sc.intel.com ([143.183.136.52]) by orsmga001.jf.intel.com with ESMTP; 11 Jul 2018 16:04:06 -0700 Message-ID: <1531350028.15351.102.camel@intel.com> Subject: Re: [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support From: Yu-cheng Yu To: Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Cyrill Gorcunov , Florian Weimer , "H.J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , "Ravi V. Shankar" , Vedvyas Shanbhogue Date: Wed, 11 Jul 2018 16:00:28 -0700 In-Reply-To: References: <20180710222639.8241-1-yu-cheng.yu@intel.com> <20180710222639.8241-23-yu-cheng.yu@intel.com> <3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com> <1531347019.15351.89.camel@intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2018-07-11 at 15:40 -0700, Dave Hansen wrote: > On 07/11/2018 03:10 PM, Yu-cheng Yu wrote: > > > > On Tue, 2018-07-10 at 17:11 -0700, Dave Hansen wrote: > > > > > > Is this feature *integral* to shadow stacks?  Or, should it just > > > be > > > in a > > > different series? > > The whole CET series is mostly about SHSTK and only a minority for > > IBT. > > IBT changes cannot be applied by itself without first applying > > SHSTK > > changes.  Would the titles help, e.g. x86/cet/ibt, x86/cet/shstk, > > etc.? > That doesn't really answer what I asked, though. > > Do shadow stacks *require* IBT?  Or, should we concentrate on merging > shadow stacks themselves first and then do IBT at a later time, in a > different patch series? > > But, yes, better patch titles would help, although I'm not sure > that's > quite the format that Ingo and Thomas prefer. Shadow stack does not require IBT, but they complement each other.  If we can resolve the legacy bitmap, both features can be merged at the same time. > > > > > > > > > > > > > > +int cet_setup_ibt_bitmap(void) > > > > +{ > > > > + u64 r; > > > > + unsigned long bitmap; > > > > + unsigned long size; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return -EOPNOTSUPP; > > > > + > > > > + size = TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE; > > > Just a note: this table is going to be gigantic on 5-level paging > > > systems, and userspace won't, by default use any of that extra > > > address > > > space.  I think it ends up being a 512GB allocation in a 128TB > > > address > > > space. > > > > > > Is that a problem? > > > > > > On 5-level paging systems, maybe we should just stick it up in > > > the  > > > high part of the address space. > > We do not know in advance if dlopen() needs to create the bitmap. > >  Do > > we always reserve high address or force legacy libs to low address? > Does it matter?  Does code ever get pointers to this area?  Might > they > be depending on high address bits for the IBT being clear? GLIBC does the bitmap setup.  It sets bits in there. I thought you wanted a smaller bitmap?  One way is forcing legacy libs to low address, or not having the bitmap at all, i.e. turn IBT off. > > > > > > > > > > > > > > > + bitmap = ibt_mmap(0, size); > > > > + > > > > + if (bitmap >= TASK_SIZE_MAX) > > > > + return -ENOMEM; > > > > + > > > > + bitmap &= PAGE_MASK; > > > We're page-aligning the result of an mmap()?  Why? > > This may not be necessary.  The lower bits of MSR_IA32_U_CET are > > settings and not part of the bitmap address.  Is this is safer? > No.  If we have mmap() returning non-page-aligned addresses, we have > bigger problems.  Worst-case, do > > WARN_ON_ONCE(bitmap & ~PAGE_MASK); > Ok. > > > > > > > > > > > > > + current->thread.cet.ibt_bitmap_addr = bitmap; > > > > + current->thread.cet.ibt_bitmap_size = size; > > > > + return 0; > > > > +} > > > > + > > > > +void cet_disable_ibt(void) > > > > +{ > > > > + u64 r; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return; > > > Does this need a check for being already disabled? > > We need that.  We cannot write to those MSRs if the CPU does not > > support it. > No, I mean for code doing cet_disable_ibt() twice in a row. Got it. > > > > > > > > > > > > > > + rdmsrl(MSR_IA32_U_CET, r); > > > > + r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN > > > > | > > > > +        MSR_IA32_CET_NO_TRACK_EN); > > > > + wrmsrl(MSR_IA32_U_CET, r); > > > > + current->thread.cet.ibt_enabled = 0; > > > > +} > > > What's the locking for current->thread.cet? > > Now CET is not locked until the application calls ARCH_CET_LOCK. > No, I mean what is the in-kernel locking for the current->thread.cet > data structure?  Is there none because it's only every modified via > current->thread and it's entirely thread-local? Yes, that is the case. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-5.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=unavailable autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 816707D071 for ; Wed, 11 Jul 2018 23:04:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388120AbeGKXK5 (ORCPT ); Wed, 11 Jul 2018 19:10:57 -0400 Received: from mga18.intel.com ([134.134.136.126]:3235 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388115AbeGKXK5 (ORCPT ); Wed, 11 Jul 2018 19:10:57 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2018 16:04:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,339,1526367600"; d="scan'208";a="72011249" Received: from 2b52.sc.intel.com ([143.183.136.52]) by orsmga001.jf.intel.com with ESMTP; 11 Jul 2018 16:04:06 -0700 Message-ID: <1531350028.15351.102.camel@intel.com> Subject: Re: [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support From: Yu-cheng Yu To: Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Cyrill Gorcunov , Florian Weimer , "H.J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , "Ravi V. Shankar" , Vedvyas Shanbhogue Date: Wed, 11 Jul 2018 16:00:28 -0700 In-Reply-To: References: <20180710222639.8241-1-yu-cheng.yu@intel.com> <20180710222639.8241-23-yu-cheng.yu@intel.com> <3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com> <1531347019.15351.89.camel@intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Wed, 2018-07-11 at 15:40 -0700, Dave Hansen wrote: > On 07/11/2018 03:10 PM, Yu-cheng Yu wrote: > > > > On Tue, 2018-07-10 at 17:11 -0700, Dave Hansen wrote: > > > > > > Is this feature *integral* to shadow stacks?  Or, should it just > > > be > > > in a > > > different series? > > The whole CET series is mostly about SHSTK and only a minority for > > IBT. > > IBT changes cannot be applied by itself without first applying > > SHSTK > > changes.  Would the titles help, e.g. x86/cet/ibt, x86/cet/shstk, > > etc.? > That doesn't really answer what I asked, though. > > Do shadow stacks *require* IBT?  Or, should we concentrate on merging > shadow stacks themselves first and then do IBT at a later time, in a > different patch series? > > But, yes, better patch titles would help, although I'm not sure > that's > quite the format that Ingo and Thomas prefer. Shadow stack does not require IBT, but they complement each other.  If we can resolve the legacy bitmap, both features can be merged at the same time. > > > > > > > > > > > > > > +int cet_setup_ibt_bitmap(void) > > > > +{ > > > > + u64 r; > > > > + unsigned long bitmap; > > > > + unsigned long size; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return -EOPNOTSUPP; > > > > + > > > > + size = TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE; > > > Just a note: this table is going to be gigantic on 5-level paging > > > systems, and userspace won't, by default use any of that extra > > > address > > > space.  I think it ends up being a 512GB allocation in a 128TB > > > address > > > space. > > > > > > Is that a problem? > > > > > > On 5-level paging systems, maybe we should just stick it up in > > > the  > > > high part of the address space. > > We do not know in advance if dlopen() needs to create the bitmap. > >  Do > > we always reserve high address or force legacy libs to low address? > Does it matter?  Does code ever get pointers to this area?  Might > they > be depending on high address bits for the IBT being clear? GLIBC does the bitmap setup.  It sets bits in there. I thought you wanted a smaller bitmap?  One way is forcing legacy libs to low address, or not having the bitmap at all, i.e. turn IBT off. > > > > > > > > > > > > > > > + bitmap = ibt_mmap(0, size); > > > > + > > > > + if (bitmap >= TASK_SIZE_MAX) > > > > + return -ENOMEM; > > > > + > > > > + bitmap &= PAGE_MASK; > > > We're page-aligning the result of an mmap()?  Why? > > This may not be necessary.  The lower bits of MSR_IA32_U_CET are > > settings and not part of the bitmap address.  Is this is safer? > No.  If we have mmap() returning non-page-aligned addresses, we have > bigger problems.  Worst-case, do > > WARN_ON_ONCE(bitmap & ~PAGE_MASK); > Ok. > > > > > > > > > > > > > + current->thread.cet.ibt_bitmap_addr = bitmap; > > > > + current->thread.cet.ibt_bitmap_size = size; > > > > + return 0; > > > > +} > > > > + > > > > +void cet_disable_ibt(void) > > > > +{ > > > > + u64 r; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return; > > > Does this need a check for being already disabled? > > We need that.  We cannot write to those MSRs if the CPU does not > > support it. > No, I mean for code doing cet_disable_ibt() twice in a row. Got it. > > > > > > > > > > > > > > + rdmsrl(MSR_IA32_U_CET, r); > > > > + r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN > > > > | > > > > +        MSR_IA32_CET_NO_TRACK_EN); > > > > + wrmsrl(MSR_IA32_U_CET, r); > > > > + current->thread.cet.ibt_enabled = 0; > > > > +} > > > What's the locking for current->thread.cet? > > Now CET is not locked until the application calls ARCH_CET_LOCK. > No, I mean what is the in-kernel locking for the current->thread.cet > data structure?  Is there none because it's only every modified via > current->thread and it's entirely thread-local? Yes, that is the case. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yu-cheng Yu Subject: Re: [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support Date: Wed, 11 Jul 2018 16:00:28 -0700 Message-ID: <1531350028.15351.102.camel@intel.com> References: <20180710222639.8241-1-yu-cheng.yu@intel.com> <20180710222639.8241-23-yu-cheng.yu@intel.com> <3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com> <1531347019.15351.89.camel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Cyrill Gorcunov , Florian Weimer , "H.J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek Peter List-Id: linux-api@vger.kernel.org On Wed, 2018-07-11 at 15:40 -0700, Dave Hansen wrote: > On 07/11/2018 03:10 PM, Yu-cheng Yu wrote: > > > > On Tue, 2018-07-10 at 17:11 -0700, Dave Hansen wrote: > > > > > > Is this feature *integral* to shadow stacks?  Or, should it just > > > be > > > in a > > > different series? > > The whole CET series is mostly about SHSTK and only a minority for > > IBT. > > IBT changes cannot be applied by itself without first applying > > SHSTK > > changes.  Would the titles help, e.g. x86/cet/ibt, x86/cet/shstk, > > etc.? > That doesn't really answer what I asked, though. > > Do shadow stacks *require* IBT?  Or, should we concentrate on merging > shadow stacks themselves first and then do IBT at a later time, in a > different patch series? > > But, yes, better patch titles would help, although I'm not sure > that's > quite the format that Ingo and Thomas prefer. Shadow stack does not require IBT, but they complement each other.  If we can resolve the legacy bitmap, both features can be merged at the same time. > > > > > > > > > > > > > > +int cet_setup_ibt_bitmap(void) > > > > +{ > > > > + u64 r; > > > > + unsigned long bitmap; > > > > + unsigned long size; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return -EOPNOTSUPP; > > > > + > > > > + size = TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE; > > > Just a note: this table is going to be gigantic on 5-level paging > > > systems, and userspace won't, by default use any of that extra > > > address > > > space.  I think it ends up being a 512GB allocation in a 128TB > > > address > > > space. > > > > > > Is that a problem? > > > > > > On 5-level paging systems, maybe we should just stick it up in > > > the  > > > high part of the address space. > > We do not know in advance if dlopen() needs to create the bitmap. > >  Do > > we always reserve high address or force legacy libs to low address? > Does it matter?  Does code ever get pointers to this area?  Might > they > be depending on high address bits for the IBT being clear? GLIBC does the bitmap setup.  It sets bits in there. I thought you wanted a smaller bitmap?  One way is forcing legacy libs to low address, or not having the bitmap at all, i.e. turn IBT off. > > > > > > > > > > > > > > > + bitmap = ibt_mmap(0, size); > > > > + > > > > + if (bitmap >= TASK_SIZE_MAX) > > > > + return -ENOMEM; > > > > + > > > > + bitmap &= PAGE_MASK; > > > We're page-aligning the result of an mmap()?  Why? > > This may not be necessary.  The lower bits of MSR_IA32_U_CET are > > settings and not part of the bitmap address.  Is this is safer? > No.  If we have mmap() returning non-page-aligned addresses, we have > bigger problems.  Worst-case, do > > WARN_ON_ONCE(bitmap & ~PAGE_MASK); > Ok. > > > > > > > > > > > > > + current->thread.cet.ibt_bitmap_addr = bitmap; > > > > + current->thread.cet.ibt_bitmap_size = size; > > > > + return 0; > > > > +} > > > > + > > > > +void cet_disable_ibt(void) > > > > +{ > > > > + u64 r; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return; > > > Does this need a check for being already disabled? > > We need that.  We cannot write to those MSRs if the CPU does not > > support it. > No, I mean for code doing cet_disable_ibt() twice in a row. Got it. > > > > > > > > > > > > > > + rdmsrl(MSR_IA32_U_CET, r); > > > > + r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN > > > > | > > > > +        MSR_IA32_CET_NO_TRACK_EN); > > > > + wrmsrl(MSR_IA32_U_CET, r); > > > > + current->thread.cet.ibt_enabled = 0; > > > > +} > > > What's the locking for current->thread.cet? > > Now CET is not locked until the application calls ARCH_CET_LOCK. > No, I mean what is the in-kernel locking for the current->thread.cet > data structure?  Is there none because it's only every modified via > current->thread and it's entirely thread-local? Yes, that is the case. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f72.google.com (mail-pl0-f72.google.com [209.85.160.72]) by kanga.kvack.org (Postfix) with ESMTP id EAACF6B000A for ; Wed, 11 Jul 2018 19:04:23 -0400 (EDT) Received: by mail-pl0-f72.google.com with SMTP id m2-v6so15857724plt.14 for ; Wed, 11 Jul 2018 16:04:23 -0700 (PDT) Received: from mga07.intel.com (mga07.intel.com. [134.134.136.100]) by mx.google.com with ESMTPS id d1-v6si18729605pgo.337.2018.07.11.16.04.22 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Jul 2018 16:04:22 -0700 (PDT) Message-ID: <1531350028.15351.102.camel@intel.com> Subject: Re: [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support From: Yu-cheng Yu Date: Wed, 11 Jul 2018 16:00:28 -0700 In-Reply-To: References: <20180710222639.8241-1-yu-cheng.yu@intel.com> <20180710222639.8241-23-yu-cheng.yu@intel.com> <3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com> <1531347019.15351.89.camel@intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: owner-linux-mm@kvack.org List-ID: To: Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Cyrill Gorcunov , Florian Weimer , "H.J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , "Ravi V. Shankar" , Vedvyas Shanbhogue On Wed, 2018-07-11 at 15:40 -0700, Dave Hansen wrote: > On 07/11/2018 03:10 PM, Yu-cheng Yu wrote: > > > > On Tue, 2018-07-10 at 17:11 -0700, Dave Hansen wrote: > > > > > > Is this feature *integral* to shadow stacks?A A Or, should it just > > > be > > > in a > > > different series? > > The whole CET series is mostly about SHSTK and only a minority for > > IBT. > > IBT changes cannot be applied by itself without first applying > > SHSTK > > changes. A Would the titles help, e.g. x86/cet/ibt, x86/cet/shstk, > > etc.? > That doesn't really answer what I asked, though. > > Do shadow stacks *require* IBT?A A Or, should we concentrate on merging > shadow stacks themselves first and then do IBT at a later time, in a > different patch series? > > But, yes, better patch titles would help, although I'm not sure > that's > quite the format that Ingo and Thomas prefer. Shadow stack does not require IBT, but they complement each other. A If we can resolve the legacy bitmap, both features can be merged at the same time. > > > > > > > > > > > > > > +int cet_setup_ibt_bitmap(void) > > > > +{ > > > > + u64 r; > > > > + unsigned long bitmap; > > > > + unsigned long size; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return -EOPNOTSUPP; > > > > + > > > > + size = TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE; > > > Just a note: this table is going to be gigantic on 5-level paging > > > systems, and userspace won't, by default use any of that extra > > > address > > > space.A A I think it ends up being a 512GB allocation in a 128TB > > > address > > > space. > > > > > > Is that a problem? > > > > > > On 5-level paging systems, maybe we should just stick it up in > > > theA > > > high part of the address space. > > We do not know in advance if dlopen() needs to create the bitmap. > > A Do > > we always reserve high address or force legacy libs to low address? > Does it matter?A A Does code ever get pointers to this area?A A Might > they > be depending on high address bits for the IBT being clear? GLIBC does the bitmap setup. A It sets bits in there. I thought you wanted a smaller bitmap? A One way is forcing legacy libs to low address, or not having the bitmap at all, i.e. turn IBT off. > > > > > > > > > > > > > > > + bitmap = ibt_mmap(0, size); > > > > + > > > > + if (bitmap >= TASK_SIZE_MAX) > > > > + return -ENOMEM; > > > > + > > > > + bitmap &= PAGE_MASK; > > > We're page-aligning the result of an mmap()?A A Why? > > This may not be necessary. A The lower bits of MSR_IA32_U_CET are > > settings and not part of the bitmap address. A Is this is safer? > No.A A If we have mmap() returning non-page-aligned addresses, we have > bigger problems.A A Worst-case, do > > WARN_ON_ONCE(bitmap & ~PAGE_MASK); > Ok. > > > > > > > > > > > > > + current->thread.cet.ibt_bitmap_addr = bitmap; > > > > + current->thread.cet.ibt_bitmap_size = size; > > > > + return 0; > > > > +} > > > > + > > > > +void cet_disable_ibt(void) > > > > +{ > > > > + u64 r; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return; > > > Does this need a check for being already disabled? > > We need that. A We cannot write to those MSRs if the CPU does not > > support it. > No, I mean for code doing cet_disable_ibt() twice in a row. Got it. > > > > > > > > > > > > > > + rdmsrl(MSR_IA32_U_CET, r); > > > > + r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN > > > > | > > > > + A A A A A A A MSR_IA32_CET_NO_TRACK_EN); > > > > + wrmsrl(MSR_IA32_U_CET, r); > > > > + current->thread.cet.ibt_enabled = 0; > > > > +} > > > What's the locking for current->thread.cet? > > Now CET is not locked until the application callsA ARCH_CET_LOCK. > No, I mean what is the in-kernel locking for the current->thread.cet > data structure?A A Is there none because it's only every modified via > current->thread and it's entirely thread-local? Yes, that is the case.