The previous version of CET patches can be found in the following
link:
https://lkml.org/lkml/2018/8/30/582
Summary of changes from v3:
Move IBT legacy code bitmap allocation back to when the application
requests it. Most application do not need the bitmap. It is only
used when an application does dlopen() a legacy library.
In the previous version, we pre-allocated the bitmap for every IBT-
enabled application to avoid creating a hole in the linear address.
However, this created a problem when the system has limited memory.
H.J. Lu (1):
x86: Insert endbr32/endbr64 to vDSO
Yu-cheng Yu (8):
x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
x86/cet/ibt: User-mode indirect branch tracking support
x86/cet/ibt: Add IBT legacy code bitmap allocation function
mm/mmap: Add IBT bitmap size to address space limit check
x86/cet/ibt: ELF header parsing for IBT
x86/cet/ibt: Add arch_prctl functions for IBT
x86/cet/ibt: Add ENDBR to op-code-map
x86/cet: Add PTRACE interface for CET
arch/x86/Kconfig | 12 +++
arch/x86/Makefile | 7 ++
arch/x86/entry/vdso/.gitignore | 4 +
arch/x86/entry/vdso/Makefile | 12 ++-
arch/x86/entry/vdso/vdso-layout.lds.S | 1 +
arch/x86/include/asm/cet.h | 8 ++
arch/x86/include/asm/disabled-features.h | 8 +-
arch/x86/include/asm/fpu/regset.h | 7 +-
arch/x86/include/uapi/asm/elf_property.h | 1 +
arch/x86/include/uapi/asm/prctl.h | 1 +
arch/x86/include/uapi/asm/resource.h | 5 ++
arch/x86/kernel/cet.c | 76 +++++++++++++++++++
arch/x86/kernel/cet_prctl.c | 38 +++++++++-
arch/x86/kernel/cpu/common.c | 20 ++++-
arch/x86/kernel/elf.c | 8 +-
arch/x86/kernel/fpu/regset.c | 41 ++++++++++
arch/x86/kernel/process.c | 2 +
arch/x86/kernel/ptrace.c | 16 ++++
arch/x86/lib/x86-opcode-map.txt | 13 +++-
include/uapi/asm-generic/resource.h | 3 +
include/uapi/linux/elf.h | 1 +
mm/mmap.c | 12 ++-
tools/objtool/arch/x86/lib/x86-opcode-map.txt | 13 +++-
23 files changed, 296 insertions(+), 13 deletions(-)
--
2.17.1
From: "H.J. Lu" <redacted>
When Intel indirect branch tracking is enabled, functions in vDSO which
may be called indirectly must have endbr32 or endbr64 as the first
instruction. Compiler must support -fcf-protection=branch so that it
can be used to compile vDSO.
Signed-off-by: H.J. Lu <redacted>
---
arch/x86/entry/vdso/.gitignore | 4 ++++
arch/x86/entry/vdso/Makefile | 12 +++++++++++-
arch/x86/entry/vdso/vdso-layout.lds.S | 1 +
3 files changed, 16 insertions(+), 1 deletion(-)
Look in .note.gnu.property of an ELF file and check if Indirect
Branch Tracking needs to be enabled for the task.
Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
---
arch/x86/include/uapi/asm/elf_property.h | 1 +
arch/x86/kernel/elf.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)
Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.
Introduce:
arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
Enable the Indirect Branch Tracking legacy code bitmap.
The parameter 'addr' is a pointer to a user buffer.
On returning to the caller, the kernel fills the following:
*addr = IBT bitmap base address
*(addr + 1) = IBT bitmap size
Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
---
arch/x86/include/uapi/asm/prctl.h | 1 +
arch/x86/kernel/cet_prctl.c | 38 ++++++++++++++++++++++++++++++-
arch/x86/kernel/process.c | 1 +
3 files changed, 39 insertions(+), 1 deletion(-)
@@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)if(current->thread.cet.shstk_enabled)features|=GNU_PROPERTY_X86_FEATURE_1_SHSTK;+if(current->thread.cet.ibt_enabled)+features|=GNU_PROPERTY_X86_FEATURE_1_IBT;shstk_base=current->thread.cet.shstk_base;shstk_size=current->thread.cet.shstk_size;
@@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)return0;}+staticinthandle_bitmap(unsignedlongarg2)+{+unsignedlongaddr,size;++if(current->thread.cet.ibt_enabled){+interr;++err=cet_setup_ibt_bitmap();+if(err)+returnerr;++addr=current->thread.cet.ibt_bitmap_addr;+size=current->thread.cet.ibt_bitmap_size;+}else{+addr=0;+size=0;+}++if(put_user(addr,(unsignedlong__user*)arg2)||+put_user(size,(unsignedlong__user*)arg2+1))+return-EFAULT;++return0;+}+intprctl_cet(intoption,unsignedlongarg2){-if(!cpu_feature_enabled(X86_FEATURE_SHSTK))+if(!cpu_feature_enabled(X86_FEATURE_SHSTK)&&+!cpu_feature_enabled(X86_FEATURE_IBT))return-EINVAL;switch(option){
@@ -63,6 +91,8 @@ int prctl_cet(int option, unsigned long arg2)return-EPERM;if(arg2&GNU_PROPERTY_X86_FEATURE_1_SHSTK)cet_disable_free_shstk(current);+if(arg2&GNU_PROPERTY_X86_FEATURE_1_IBT)+cet_disable_ibt();return0;
@@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2)caseARCH_CET_ALLOC_SHSTK:returnhandle_alloc_shstk(arg2);+/*+*Allocatelegacybitmapandreturnaddress&sizetouser.+*/+caseARCH_CET_LEGACY_BITMAP:+returnhandle_bitmap(arg2);+default:return-EINVAL;}
@@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,caseARCH_CET_DISABLE:caseARCH_CET_LOCK:caseARCH_CET_ALLOC_SHSTK:+caseARCH_CET_LEGACY_BITMAP:returnprctl_cet(option,cpuid_enabled);}
@@ -283,3 +285,32 @@ int cet_setup_signal(bool ia32, unsigned long rstor_addr,set_shstk_ptr(ssp);return0;}++intcet_setup_ibt(void)+{+u64r;++if(!cpu_feature_enabled(X86_FEATURE_IBT))+return-EOPNOTSUPP;++rdmsrl(MSR_IA32_U_CET,r);+r|=(MSR_IA32_CET_ENDBR_EN|MSR_IA32_CET_NO_TRACK_EN);+wrmsrl(MSR_IA32_U_CET,r);++current->thread.cet.ibt_enabled=1;+return0;+}++voidcet_disable_ibt(void)+{+u64r;++if(!cpu_feature_enabled(X86_FEATURE_IBT))+return;++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;+}
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
The user-mode indirect branch tracking support is done mostly by
GCC to insert ENDBR64/ENDBR32 instructions at branch targets.
The kernel provides CPUID enumeration, feature MSR setup and
the allocation of legacy bitmap.
Signed-off-by: Yu-cheng Yu <redacted>
---
arch/x86/Kconfig | 12 ++++++++++++
arch/x86/Makefile | 7 +++++++
2 files changed, 19 insertions(+)
@@ -159,6 +159,13 @@ ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER endifendif+# Check compiler ibt support+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER+ ifeq ($(call cc-option-yn, -fcf-protection=branch), n)+$(errorCONFIG_X86_INTEL_BRANCH_TRACKING_USERnotsupportedbycompiler)+ endif+endif+## If the function graph tracer is used with mcount instead of fentry,# '-maccumulate-outgoing-args' is needed to prevent a GCC bug
@@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s) __setup("no_cet_shstk", setup_disable_shstk); #endif+#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER+static __init int setup_disable_ibt(char *s)+{+ /* require an exact match without trailing characters */+ if (strlen(s))+ return 0;
"s[0] (!= '\0')" will do the same check in constant time.
On Fri, Sep 21, 2018 at 08:05:47AM -0700, Yu-cheng Yu wrote:
quoted hunk
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
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.
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.
I wonder, how realistic a scenario where a userspace application enables IBT,
configures a huge prefetchable IO memory region (that just ignores bits
of offset beyond 16, for example), and start repeatedly loading a legacy
library there at different linear addresses.
return false;
if (is_data_mapping(flags) &&
--
2.17.1
On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.
Introduce:
arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
Enable the Indirect Branch Tracking legacy code bitmap.
The parameter 'addr' is a pointer to a user buffer.
On returning to the caller, the kernel fills the following:
*addr = IBT bitmap base address
*(addr + 1) = IBT bitmap size
Again, some structure with a size field would be better from
UAPI/extensibility standpoint.
One additional point: "size" in the structure from kernel should have
structure size expected by kernel, and at least providing there "0" from
user space shouldn't lead to failure (in fact, it is possible to provide
structure size back to userspace even if buffer is too small, along
with error).
@@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)if(current->thread.cet.shstk_enabled)features|=GNU_PROPERTY_X86_FEATURE_1_SHSTK;+if(current->thread.cet.ibt_enabled)+features|=GNU_PROPERTY_X86_FEATURE_1_IBT;shstk_base=current->thread.cet.shstk_base;shstk_size=current->thread.cet.shstk_size;
@@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)return0;}+staticinthandle_bitmap(unsignedlongarg2)+{+unsignedlongaddr,size;++if(current->thread.cet.ibt_enabled){+interr;++err=cet_setup_ibt_bitmap();+if(err)+returnerr;++addr=current->thread.cet.ibt_bitmap_addr;+size=current->thread.cet.ibt_bitmap_size;+}else{+addr=0;+size=0;+}++if(put_user(addr,(unsignedlong__user*)arg2)||+put_user(size,(unsignedlong__user*)arg2+1))+return-EFAULT;++return0;+}+intprctl_cet(intoption,unsignedlongarg2){-if(!cpu_feature_enabled(X86_FEATURE_SHSTK))+if(!cpu_feature_enabled(X86_FEATURE_SHSTK)&&+!cpu_feature_enabled(X86_FEATURE_IBT))
This check is repeated many times, it is probably worth defining
something like cpu_x86_cet_enabled() or something like that.
Besides, early introduction of the macro would allow avoiding all these
changes over the code in IBT patches, only macro definition has
to be changed that way.
quoted hunk
@@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2) case ARCH_CET_ALLOC_SHSTK: return handle_alloc_shstk(arg2);+ /*+ * Allocate legacy bitmap and return address & size to user.+ */+ case ARCH_CET_LEGACY_BITMAP:+ return handle_bitmap(arg2);+ default: return -EINVAL; }
@@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int option,caseARCH_CET_DISABLE:caseARCH_CET_LOCK:caseARCH_CET_ALLOC_SHSTK:+caseARCH_CET_LEGACY_BITMAP:returnprctl_cet(option,cpuid_enabled);}
I wonder, whether this duplication is really needed for CET-related
arch_prctl commands, why not just call them from do_arch_prctl_common?
On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
quoted
Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.
Introduce:
arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
Enable the Indirect Branch Tracking legacy code bitmap.
The parameter 'addr' is a pointer to a user buffer.
On returning to the caller, the kernel fills the following:
*addr = IBT bitmap base address
*(addr + 1) = IBT bitmap size
Again, some structure with a size field would be better from
UAPI/extensibility standpoint.
One additional point: "size" in the structure from kernel should have
structure size expected by kernel, and at least providing there "0" from
user space shouldn't lead to failure (in fact, it is possible to provide
structure size back to userspace even if buffer is too small, along
with error).
This has been in GLIBC v2.28. We cannot change it anymore.
@@ -20,6 +20,8 @@ static int handle_get_status(unsigned long arg2)if(current->thread.cet.shstk_enabled)features|=GNU_PROPERTY_X86_FEATURE_1_SHSTK;+if(current->thread.cet.ibt_enabled)+features|=GNU_PROPERTY_X86_FEATURE_1_IBT;shstk_base=current->thread.cet.shstk_base;shstk_size=current->thread.cet.shstk_size;
@@ -49,9 +51,35 @@ static int handle_alloc_shstk(unsigned long arg2)return0;}+staticinthandle_bitmap(unsignedlongarg2)+{+unsignedlongaddr,size;++if(current->thread.cet.ibt_enabled){+interr;++err=cet_setup_ibt_bitmap();+if(err)+returnerr;++addr=current->thread.cet.ibt_bitmap_addr;+size=current->thread.cet.ibt_bitmap_size;+}else{+addr=0;+size=0;+}++if(put_user(addr,(unsignedlong__user*)arg2)||+put_user(size,(unsignedlong__user*)arg2+1))+return-EFAULT;++return0;+}+intprctl_cet(intoption,unsignedlongarg2){-if(!cpu_feature_enabled(X86_FEATURE_SHSTK))+if(!cpu_feature_enabled(X86_FEATURE_SHSTK)&&+!cpu_feature_enabled(X86_FEATURE_IBT))
This check is repeated many times, it is probably worth defining
something like cpu_x86_cet_enabled() or something like that.
Besides, early introduction of the macro would allow avoiding all these
changes over the code in IBT patches, only macro definition has
to be changed that way.
Yes, that makes things easier.
quoted
@@ -73,6 +103,12 @@ int prctl_cet(int option, unsigned long arg2) case ARCH_CET_ALLOC_SHSTK: return handle_alloc_shstk(arg2);+ /*+ * Allocate legacy bitmap and return address & size to user.+ */+ case ARCH_CET_LEGACY_BITMAP:+ return handle_bitmap(arg2);+ default: return -EINVAL; }
@@ -797,6 +797,7 @@ long do_arch_prctl_common(struct task_struct *task, int
option,
case ARCH_CET_DISABLE:
case ARCH_CET_LOCK:
case ARCH_CET_ALLOC_SHSTK:
+ case ARCH_CET_LEGACY_BITMAP:
return prctl_cet(option, cpuid_enabled);
}
I wonder, whether this duplication is really needed for CET-related
arch_prctl commands, why not just call them from do_arch_prctl_common?
From: Andy Lutomirski <luto@kernel.org> Date: 2018-10-04 16:08:31
On Oct 4, 2018, at 8:37 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Thu, 2018-10-04 at 15:28 +0200, Eugene Syromiatnikov wrote:
quoted
On Fri, Sep 21, 2018 at 08:05:50AM -0700, Yu-cheng Yu wrote:
Update ARCH_CET_STATUS and ARCH_CET_DISABLE to include Indirect
Branch Tracking features.
Introduce:
arch_prctl(ARCH_CET_LEGACY_BITMAP, unsigned long *addr)
Enable the Indirect Branch Tracking legacy code bitmap.
The parameter 'addr' is a pointer to a user buffer.
On returning to the caller, the kernel fills the following:
*addr = IBT bitmap base address
*(addr + 1) = IBT bitmap size
Again, some structure with a size field would be better from
UAPI/extensibility standpoint.
One additional point: "size" in the structure from kernel should have
structure size expected by kernel, and at least providing there "0" from
user space shouldn't lead to failure (in fact, it is possible to provide
structure size back to userspace even if buffer is too small, along
with error).
This has been in GLIBC v2.28. We cannot change it anymore.
Sure you can. Just change ARCH_CET_LEGACY_BITMAP to a new number. You
might need to change all the constants. And if the ELF note by itself
causes a problem too, you may need to rename it. And maybe ask glibc
to kindly not enable code that depends on non-upstreamed kernel
features.
There is not, and has never been, any ABI compatibility requirement
that says that, if glibc 2.28 "enables" a feature, that the kernel
will ever enable it in a way that makes glibc 2.28 actually support
it. All the kernel needs to do is avoid making glibc 2.28 *crash*.
From: Andy Lutomirski <luto@kernel.org> Date: 2018-10-04 16:11:31
On Fri, Sep 21, 2018 at 8:10 AM Yu-cheng Yu [off-list ref] wrote:
quoted hunk
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
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:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
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
From: Andy Lutomirski <luto@amacapital.net> Date: 2018-10-05 16:28:11
On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
}
+
+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:
quoted
On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
}
+
+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
From: Andy Lutomirski <luto@amacapital.net> Date: 2018-10-05 17:08:03
On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu [off-list ref] wrote:
On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
quoted
quoted
On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
}
+
+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 [off-list ref] wrote:
quoted
On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
quoted
quoted
On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
}
+
+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.
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:
quoted
On Fri, Oct 5, 2018 at 10:03 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2018-10-05 at 09:28 -0700, Andy Lutomirski wrote:
quoted
quoted
On Oct 5, 2018, at 9:13 AM, Yu-cheng Yu [off-list ref] wrote:
quoted
On Wed, 2018-10-03 at 21:57 +0200, Eugene Syromiatnikov wrote:
quoted
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 <redacted>
---
arch/x86/kernel/cet.c | 45
+++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
}
+
+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.
quoted
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