| Message ID | 20180921150553.21016-4-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control Flow Enforcement: Branch Tracking, PTRACE | expand |
On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > Indirect branch tracking provides an optional legacy code bitmap > that indicates locations of non-IBT compatible code. When set, > each bit in the bitmap represents a page in the linear address is > legacy code. > > We allocate the bitmap only when the application requests it. > Most applications do not need the bitmap. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > --- > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > index 6adfe795d692..a65d9745af08 100644 > --- a/arch/x86/kernel/cet.c > +++ b/arch/x86/kernel/cet.c > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > wrmsrl(MSR_IA32_U_CET, r); > current->thread.cet.ibt_enabled = 0; > } > + > +int cet_setup_ibt_bitmap(void) > +{ > + u64 r; > + unsigned long bitmap; > + unsigned long size; > + > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > + return -EOPNOTSUPP; > + > + if (!current->thread.cet.ibt_bitmap_addr) { > + /* > + * Calculate size and put in thread header. > + * may_expand_vm() needs this information. > + */ > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; TASK_SIZE_MAX is likely needed here, as an application can easily switch between long an 32-bit protected mode. And then the case of a CPU that doesn't support 5LPT. > + current->thread.cet.ibt_bitmap_size = size; > + bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, > + VM_DONTDUMP); > + > + if (bitmap >= TASK_SIZE) { Shouldn't bitmap be unmapped here? > + current->thread.cet.ibt_bitmap_size = 0; > + return -ENOMEM; > + } > + > + current->thread.cet.ibt_bitmap_addr = bitmap; > + } > + > + /* > + * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT > + * settings. Clear lower bits even bitmap is already > + * page-aligned. > + */ > + bitmap = current->thread.cet.ibt_bitmap_addr; > + bitmap &= PAGE_MASK; In a hypothetical situation of bitmap & PAGE_MASK < bitmap that would lead to bitmap pointing to unmapped memory. A check that bitmap is sane would probably be better. > + > + /* > + * Turn on IBT legacy bitmap. > + */ > + rdmsrl(MSR_IA32_U_CET, r); > + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap); > + wrmsrl(MSR_IA32_U_CET, r); > + return 0; > +} > -- > 2.17.1 >
On Fri, Sep 21, 2018 at 8:10 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > Indirect branch tracking provides an optional legacy code bitmap > that indicates locations of non-IBT compatible code. When set, > each bit in the bitmap represents a page in the linear address is > legacy code. > > We allocate the bitmap only when the application requests it. > Most applications do not need the bitmap. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > --- > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > index 6adfe795d692..a65d9745af08 100644 > --- a/arch/x86/kernel/cet.c > +++ b/arch/x86/kernel/cet.c > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > wrmsrl(MSR_IA32_U_CET, r); > current->thread.cet.ibt_enabled = 0; > } > + > +int cet_setup_ibt_bitmap(void) > +{ > + u64 r; > + unsigned long bitmap; > + unsigned long size; > + > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > + return -EOPNOTSUPP; > + > + if (!current->thread.cet.ibt_bitmap_addr) { > + /* > + * Calculate size and put in thread header. > + * may_expand_vm() needs this information. > + */ > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > + current->thread.cet.ibt_bitmap_size = size; > + bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, > + VM_DONTDUMP); > + > + if (bitmap >= TASK_SIZE) { > + current->thread.cet.ibt_bitmap_size = 0; > + return -ENOMEM; > + } > + > + current->thread.cet.ibt_bitmap_addr = bitmap; > + } > + > + /* > + * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT > + * settings. Clear lower bits even bitmap is already > + * page-aligned. > + */ > + bitmap = current->thread.cet.ibt_bitmap_addr; > + bitmap &= PAGE_MASK; > + > + /* > + * Turn on IBT legacy bitmap. > + */ > + rdmsrl(MSR_IA32_U_CET, r); > + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap); > + wrmsrl(MSR_IA32_U_CET, r); > + return 0; Why are you writing the MSRs in the case where the bitmap was already allocated?
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > > Indirect branch tracking provides an optional legacy code bitmap > > that indicates locations of non-IBT compatible code. When set, > > each bit in the bitmap represents a page in the linear address is > > legacy code. > > > > We allocate the bitmap only when the application requests it. > > Most applications do not need the bitmap. > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > --- > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 45 insertions(+) > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > > index 6adfe795d692..a65d9745af08 100644 > > --- a/arch/x86/kernel/cet.c > > +++ b/arch/x86/kernel/cet.c > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > > wrmsrl(MSR_IA32_U_CET, r); > > current->thread.cet.ibt_enabled = 0; > > } > > + > > +int cet_setup_ibt_bitmap(void) > > +{ > > + u64 r; > > + unsigned long bitmap; > > + unsigned long size; > > + > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > + return -EOPNOTSUPP; > > + > > + if (!current->thread.cet.ibt_bitmap_addr) { > > + /* > > + * Calculate size and put in thread header. > > + * may_expand_vm() needs this information. > > + */ > > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > > TASK_SIZE_MAX is likely needed here, as an application can easily switch > between long an 32-bit protected mode. And then the case of a CPU that > doesn't support 5LPT. If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have failed the allocation for bitmap size > TASK_SIZE. Please see values below, which is printed from the current code. Yu-cheng x64: TASK_SIZE_MAX = 0000 7fff ffff f000 TASK_SIZE = 0000 7fff ffff f000 bitmap size = 0000 0000 ffff ffff x32: TASK_SIZE_MAX = 0000 7fff ffff f000 TASK_SIZE = 0000 0000 ffff e000 bitmap size = 0000 0000 0001 ffff
> On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > >> On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: >>> On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: >>> Indirect branch tracking provides an optional legacy code bitmap >>> that indicates locations of non-IBT compatible code. When set, >>> each bit in the bitmap represents a page in the linear address is >>> legacy code. >>> >>> We allocate the bitmap only when the application requests it. >>> Most applications do not need the bitmap. >>> >>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> >>> --- >>> arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 45 insertions(+) >>> >>> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c >>> index 6adfe795d692..a65d9745af08 100644 >>> --- a/arch/x86/kernel/cet.c >>> +++ b/arch/x86/kernel/cet.c >>> @@ -314,3 +314,48 @@ void cet_disable_ibt(void) >>> wrmsrl(MSR_IA32_U_CET, r); >>> current->thread.cet.ibt_enabled = 0; >>> } >>> + >>> +int cet_setup_ibt_bitmap(void) >>> +{ >>> + u64 r; >>> + unsigned long bitmap; >>> + unsigned long size; >>> + >>> + if (!cpu_feature_enabled(X86_FEATURE_IBT)) >>> + return -EOPNOTSUPP; >>> + >>> + if (!current->thread.cet.ibt_bitmap_addr) { >>> + /* >>> + * Calculate size and put in thread header. >>> + * may_expand_vm() needs this information. >>> + */ >>> + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; >> >> TASK_SIZE_MAX is likely needed here, as an application can easily switch >> between long an 32-bit protected mode. And then the case of a CPU that >> doesn't support 5LPT. > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would have > failed the allocation for bitmap size > TASK_SIZE. Please see values below, > which is printed from the current code. > > Yu-cheng > > > x64: > TASK_SIZE_MAX = 0000 7fff ffff f000 > TASK_SIZE = 0000 7fff ffff f000 > bitmap size = 0000 0000 ffff ffff > > x32: > TASK_SIZE_MAX = 0000 7fff ffff f000 > TASK_SIZE = 0000 0000 ffff e000 > bitmap size = 0000 0000 0001 ffff > I haven’t followed all the details here, but I have a general policy of objecting to any new use of TASK_SIZE. If you really really need to depend on 32-bitness in new code, please figure out what exactly you mean by “32-bit” and use an explicit check. Some day I would love to delete TASK_SIZE.
On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote: > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > > > > Indirect branch tracking provides an optional legacy code bitmap > > > > that indicates locations of non-IBT compatible code. When set, > > > > each bit in the bitmap represents a page in the linear address is > > > > legacy code. > > > > > > > > We allocate the bitmap only when the application requests it. > > > > Most applications do not need the bitmap. > > > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > > > --- > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 45 insertions(+) > > > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > > > > index 6adfe795d692..a65d9745af08 100644 > > > > --- a/arch/x86/kernel/cet.c > > > > +++ b/arch/x86/kernel/cet.c > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > > > > wrmsrl(MSR_IA32_U_CET, r); > > > > current->thread.cet.ibt_enabled = 0; > > > > } > > > > + > > > > +int cet_setup_ibt_bitmap(void) > > > > +{ > > > > + u64 r; > > > > + unsigned long bitmap; > > > > + unsigned long size; > > > > + > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > + return -EOPNOTSUPP; > > > > + > > > > + if (!current->thread.cet.ibt_bitmap_addr) { > > > > + /* > > > > + * Calculate size and put in thread header. > > > > + * may_expand_vm() needs this information. > > > > + */ > > > > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch > > > between long an 32-bit protected mode. And then the case of a CPU that > > > doesn't support 5LPT. > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would > > have > > failed the allocation for bitmap size > TASK_SIZE. Please see values below, > > which is printed from the current code. > > > > Yu-cheng > > > > > > x64: > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > TASK_SIZE = 0000 7fff ffff f000 > > bitmap size = 0000 0000 ffff ffff > > > > x32: > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > TASK_SIZE = 0000 0000 ffff e000 > > bitmap size = 0000 0000 0001 ffff > > > > I haven’t followed all the details here, but I have a general policy of > objecting to any new use of TASK_SIZE. If you really really need to depend on > 32-bitness in new code, please figure out what exactly you mean by “32-bit” > and use an explicit check. The explicit check would be: test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX which is the same as TASK_SIZE. Or, do we want a new macro? #define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \ (IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \ (TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE)) Yu-cheng
On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote: > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > > > > > Indirect branch tracking provides an optional legacy code bitmap > > > > > that indicates locations of non-IBT compatible code. When set, > > > > > each bit in the bitmap represents a page in the linear address is > > > > > legacy code. > > > > > > > > > > We allocate the bitmap only when the application requests it. > > > > > Most applications do not need the bitmap. > > > > > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > > > > --- > > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 45 insertions(+) > > > > > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > > > > > index 6adfe795d692..a65d9745af08 100644 > > > > > --- a/arch/x86/kernel/cet.c > > > > > +++ b/arch/x86/kernel/cet.c > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > > > > > wrmsrl(MSR_IA32_U_CET, r); > > > > > current->thread.cet.ibt_enabled = 0; > > > > > } > > > > > + > > > > > +int cet_setup_ibt_bitmap(void) > > > > > +{ > > > > > + u64 r; > > > > > + unsigned long bitmap; > > > > > + unsigned long size; > > > > > + > > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > > + return -EOPNOTSUPP; > > > > > + > > > > > + if (!current->thread.cet.ibt_bitmap_addr) { > > > > > + /* > > > > > + * Calculate size and put in thread header. > > > > > + * may_expand_vm() needs this information. > > > > > + */ > > > > > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > > > > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch > > > > between long an 32-bit protected mode. And then the case of a CPU that > > > > doesn't support 5LPT. > > > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would > > > have > > > failed the allocation for bitmap size > TASK_SIZE. Please see values below, > > > which is printed from the current code. > > > > > > Yu-cheng > > > > > > > > > x64: > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > TASK_SIZE = 0000 7fff ffff f000 > > > bitmap size = 0000 0000 ffff ffff > > > > > > x32: > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > TASK_SIZE = 0000 0000 ffff e000 > > > bitmap size = 0000 0000 0001 ffff > > > > > > > I haven’t followed all the details here, but I have a general policy of > > objecting to any new use of TASK_SIZE. If you really really need to depend on > > 32-bitness in new code, please figure out what exactly you mean by “32-bit” > > and use an explicit check. > > The explicit check would be: > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX > > which is the same as TASK_SIZE. But this is only ever done in response to a syscall, right? So wouldn't in_compat_syscall() be the right check? Also, this whole thing makes me extremely nervous. The MSR only contains the start address, not the size, right? So what prevents some goof from causing the CPU to read way past the end of the bitmap if the bitmap is short because the kernel thought it was supposed to be 32-bit? I'm inclined to suggest something awful-ish: always allocate the bitmap as though it's for a 64-bit process, and just let it be at a high address. And add a syscall or arch_prctl() to manipulate it for the benefit of 32-bit programs that can't address it directly. > > Or, do we want a new macro? > > #define IBT_BITMAP_SIZE (test_thread_flag(TIF_ADDR32) ? \ > (IA32_PAGE_OFFSET / PAGE_SIZE / BITS_PER_BYTE) : \ > (TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE)) No. I don't like hiding magic like this in a macro that looks like a constant.
On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote: > On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote: > > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: > > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > > > > > > Indirect branch tracking provides an optional legacy code bitmap > > > > > > that indicates locations of non-IBT compatible code. When set, > > > > > > each bit in the bitmap represents a page in the linear address is > > > > > > legacy code. > > > > > > > > > > > > We allocate the bitmap only when the application requests it. > > > > > > Most applications do not need the bitmap. > > > > > > > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > > > > > --- > > > > > > arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ > > > > > > 1 file changed, 45 insertions(+) > > > > > > > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > > > > > > index 6adfe795d692..a65d9745af08 100644 > > > > > > --- a/arch/x86/kernel/cet.c > > > > > > +++ b/arch/x86/kernel/cet.c > > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > > > > > > wrmsrl(MSR_IA32_U_CET, r); > > > > > > current->thread.cet.ibt_enabled = 0; > > > > > > } > > > > > > + > > > > > > +int cet_setup_ibt_bitmap(void) > > > > > > +{ > > > > > > + u64 r; > > > > > > + unsigned long bitmap; > > > > > > + unsigned long size; > > > > > > + > > > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > > > + return -EOPNOTSUPP; > > > > > > + > > > > > > + if (!current->thread.cet.ibt_bitmap_addr) { > > > > > > + /* > > > > > > + * Calculate size and put in thread header. > > > > > > + * may_expand_vm() needs this information. > > > > > > + */ > > > > > > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > > > > > > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily switch > > > > > between long an 32-bit protected mode. And then the case of a CPU that > > > > > doesn't support 5LPT. > > > > > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps would > > > > have > > > > failed the allocation for bitmap size > TASK_SIZE. Please see values below, > > > > which is printed from the current code. > > > > > > > > Yu-cheng > > > > > > > > > > > > x64: > > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > > TASK_SIZE = 0000 7fff ffff f000 > > > > bitmap size = 0000 0000 ffff ffff > > > > > > > > x32: > > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > > TASK_SIZE = 0000 0000 ffff e000 > > > > bitmap size = 0000 0000 0001 ffff > > > > > > > > > > I haven’t followed all the details here, but I have a general policy of > > > objecting to any new use of TASK_SIZE. If you really really need to depend on > > > 32-bitness in new code, please figure out what exactly you mean by “32-bit” > > > and use an explicit check. > > > > The explicit check would be: > > > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX > > > > which is the same as TASK_SIZE. > > But this is only ever done in response to a syscall, right? So > wouldn't in_compat_syscall() be the right check? > > Also, this whole thing makes me extremely nervous. The MSR only > contains the start address, not the size, right? So what prevents > some goof from causing the CPU to read way past the end of the bitmap > if the bitmap is short because the kernel thought it was supposed to > be 32-bit? That's what I've mentioned initially: every syscall made with int 0x80 is interpreted as compat, even if it was made from long mode. > I'm inclined to suggest something awful-ish: always allocate the > bitmap as though it's for a 64-bit process, and just let it be at a > high address. And add a syscall or arch_prctl() to manipulate it for > the benefit of 32-bit programs that can't address it directly. That's likely the only way to go.
On Fri, 2018-10-05 at 10:26 -0700, Eugene Syromiatnikov wrote: > On Fri, Oct 05, 2018 at 10:07:46AM -0700, Andy Lutomirski wrote: > > On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > > > On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote: > > > > > On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > > > > > > > > > > On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote: > > > > > > > On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote: > > > > > > > Indirect branch tracking provides an optional legacy code bitmap > > > > > > > that indicates locations of non-IBT compatible code. When set, > > > > > > > each bit in the bitmap represents a page in the linear address is > > > > > > > legacy code. > > > > > > > > > > > > > > We allocate the bitmap only when the application requests it. > > > > > > > Most applications do not need the bitmap. > > > > > > > > > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > > > > > > --- > > > > > > > arch/x86/kernel/cet.c | 45 > > > > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > > > > 1 file changed, 45 insertions(+) > > > > > > > > > > > > > > diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c > > > > > > > index 6adfe795d692..a65d9745af08 100644 > > > > > > > --- a/arch/x86/kernel/cet.c > > > > > > > +++ b/arch/x86/kernel/cet.c > > > > > > > @@ -314,3 +314,48 @@ void cet_disable_ibt(void) > > > > > > > wrmsrl(MSR_IA32_U_CET, r); > > > > > > > current->thread.cet.ibt_enabled = 0; > > > > > > > } > > > > > > > + > > > > > > > +int cet_setup_ibt_bitmap(void) > > > > > > > +{ > > > > > > > + u64 r; > > > > > > > + unsigned long bitmap; > > > > > > > + unsigned long size; > > > > > > > + > > > > > > > + if (!cpu_feature_enabled(X86_FEATURE_IBT)) > > > > > > > + return -EOPNOTSUPP; > > > > > > > + > > > > > > > + if (!current->thread.cet.ibt_bitmap_addr) { > > > > > > > + /* > > > > > > > + * Calculate size and put in thread header. > > > > > > > + * may_expand_vm() needs this information. > > > > > > > + */ > > > > > > > + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; > > > > > > > > > > > > TASK_SIZE_MAX is likely needed here, as an application can easily > > > > > > switch > > > > > > between long an 32-bit protected mode. And then the case of a CPU > > > > > > that > > > > > > doesn't support 5LPT. > > > > > > > > > > If we had calculated bitmap size from TASK_SIZE_MAX, all 32-bit apps > > > > > would > > > > > have > > > > > failed the allocation for bitmap size > TASK_SIZE. Please see values > > > > > below, > > > > > which is printed from the current code. > > > > > > > > > > Yu-cheng > > > > > > > > > > > > > > > x64: > > > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > > > TASK_SIZE = 0000 7fff ffff f000 > > > > > bitmap size = 0000 0000 ffff ffff > > > > > > > > > > x32: > > > > > TASK_SIZE_MAX = 0000 7fff ffff f000 > > > > > TASK_SIZE = 0000 0000 ffff e000 > > > > > bitmap size = 0000 0000 0001 ffff > > > > > > > > > > > > > I haven’t followed all the details here, but I have a general policy of > > > > objecting to any new use of TASK_SIZE. If you really really need to > > > > depend on > > > > 32-bitness in new code, please figure out what exactly you mean by “32- > > > > bit” > > > > and use an explicit check. > > > > > > The explicit check would be: > > > > > > test_thread_flag(TIF_ADDR32) ? IA32_PAGE_OFFSET : TASK_SIZE_MAX > > > > > > which is the same as TASK_SIZE. > > > > But this is only ever done in response to a syscall, right? So > > wouldn't in_compat_syscall() be the right check? > > > > Also, this whole thing makes me extremely nervous. The MSR only > > contains the start address, not the size, right? So what prevents > > some goof from causing the CPU to read way past the end of the bitmap > > if the bitmap is short because the kernel thought it was supposed to > > be 32-bit? > > That's what I've mentioned initially: every syscall made with int 0x80 > is interpreted as compat, even if it was made from long mode. > > > I'm inclined to suggest something awful-ish: always allocate the > > bitmap as though it's for a 64-bit process, and just let it be at a > > high address. And add a syscall or arch_prctl() to manipulate it for > > the benefit of 32-bit programs that can't address it directly. > > That's likely the only way to go. This bitmap is needed only when the app does dlopen() a non-IBT .so file. Most applications do not need it. Can't we let dlopen mmap() the bitmap when needed and pass it to the kernel? Yu-cheng
diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c index 6adfe795d692..a65d9745af08 100644 --- a/arch/x86/kernel/cet.c +++ b/arch/x86/kernel/cet.c @@ -314,3 +314,48 @@ void cet_disable_ibt(void) wrmsrl(MSR_IA32_U_CET, r); current->thread.cet.ibt_enabled = 0; } + +int cet_setup_ibt_bitmap(void) +{ + u64 r; + unsigned long bitmap; + unsigned long size; + + if (!cpu_feature_enabled(X86_FEATURE_IBT)) + return -EOPNOTSUPP; + + if (!current->thread.cet.ibt_bitmap_addr) { + /* + * Calculate size and put in thread header. + * may_expand_vm() needs this information. + */ + size = TASK_SIZE / PAGE_SIZE / BITS_PER_BYTE; + current->thread.cet.ibt_bitmap_size = size; + bitmap = do_mmap_locked(0, size, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, + VM_DONTDUMP); + + if (bitmap >= TASK_SIZE) { + current->thread.cet.ibt_bitmap_size = 0; + return -ENOMEM; + } + + current->thread.cet.ibt_bitmap_addr = bitmap; + } + + /* + * Lower bits of MSR_IA32_CET_LEG_IW_EN are for IBT + * settings. Clear lower bits even bitmap is already + * page-aligned. + */ + bitmap = current->thread.cet.ibt_bitmap_addr; + bitmap &= PAGE_MASK; + + /* + * Turn on IBT legacy bitmap. + */ + rdmsrl(MSR_IA32_U_CET, r); + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap); + wrmsrl(MSR_IA32_U_CET, r); + return 0; +}
Indirect branch tracking provides an optional legacy code bitmap that indicates locations of non-IBT compatible code. When set, each bit in the bitmap represents a page in the linear address is legacy code. We allocate the bitmap only when the application requests it. Most applications do not need the bitmap. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)