[PATCH v2 0/3] x86: PER_CPU segment improvements

[PATCH v2 0/3] x86: PER_CPU segment improvements

[Andy Lutomirski]

x86 sets up a per-cpu GDT entry so that vgetcpu can use LSL on it
to determine the CPU number and node.

This series, in little baby steps, cleans up that code and sets
the accessed and 32-bit flags on the segment.

The accessed bit prevents user code from setting the accessed bit
on its own, and making the segment 32-bit prevents concerns about
shenanigans involving CPU oddities with 16-bit data segments.

The latter isn't a real problem -- if it were a 16-bit read/write
segment, it could be used to bypass espfix64, but fortunately
RO segments can't be loaded into SS.

Changes from v1:
 - Fix patch 2's changelog.
 - Learn to spell "Not a system segment".

Andy Lutomirski (3):
  x86,vdso: Change the PER_CPU segment to use struct desc_struct
  x86,vdso: Make the PER_CPU segment start out accessed
  x86,vdso: Make the PER_CPU segment 32 bits

 arch/x86/kernel/vsyscall_64.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

-- 
1.9.3

[PATCH v2 1/3] x86,vdso: Change the PER_CPU segment to use struct desc_struct

[Andy Lutomirski]

This makes it easier to see what's going on.  It produces exactly
the same segment descriptor as the old code.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/vsyscall_64.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
index e1e1e80..28d0ad6 100644
--- a/arch/x86/kernel/vsyscall_64.c
+++ b/arch/x86/kernel/vsyscall_64.c
@@ -289,7 +289,7 @@ sigsegv:
  */
 static void vsyscall_set_cpu(int cpu)
 {
-	unsigned long d;
+	struct desc_struct d;
 	unsigned long node = 0;
 #ifdef CONFIG_NUMA
 	node = cpu_to_node(cpu);
@@ -298,13 +298,17 @@ static void vsyscall_set_cpu(int cpu)
 		write_rdtscp_aux((node << 12) | cpu);
 
 	/*
-	 * Store cpu number in limit so that it can be loaded quickly
-	 * in user space in vgetcpu. (12 bits for the CPU and 8 bits for the node)
+	 * Store cpu number in limit so that it can be loaded
+	 * quickly in user space in vgetcpu. (12 bits for the CPU
+	 * and 8 bits for the node)
 	 */
-	d = 0x0f40000000000ULL;
-	d |= cpu;
-	d |= (node & 0xf) << 12;
-	d |= (node >> 4) << 48;
+	memset(&d, 0, sizeof(d));
+	d.limit0 = cpu | ((node & 0xf) << 12);
+	d.limit = node >> 4;
+	d.type = 4;		/* RO data, expand down */
+	d.dpl = 3;		/* Visible to user code */
+	d.s = 1;		/* Not a system segment */
+	d.p = 1;		/* Present */
 
 	write_gdt_entry(get_cpu_gdt_table(cpu), GDT_ENTRY_PER_CPU, &d, DESCTYPE_S);
 }
-- 
1.9.3

[PATCH v2 2/3] x86,vdso: Make the PER_CPU segment start out accessed

[Andy Lutomirski]

The first userspace attempt to read or write the PER_CPU segment
will write the accessed bit to the GDT.  This is visible to
userspace using the LAR instruction.

Set the segment's accessed bit at boot to prevent userspace
access to segments from having side effects.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/vsyscall_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
index 28d0ad6..ae80ef6 100644
--- a/arch/x86/kernel/vsyscall_64.c
+++ b/arch/x86/kernel/vsyscall_64.c
@@ -305,7 +305,7 @@ static void vsyscall_set_cpu(int cpu)
 	memset(&d, 0, sizeof(d));
 	d.limit0 = cpu | ((node & 0xf) << 12);
 	d.limit = node >> 4;
-	d.type = 4;		/* RO data, expand down */
+	d.type = 5;		/* RO data, expand down, accessed */
 	d.dpl = 3;		/* Visible to user code */
 	d.s = 1;		/* Not a system segment */
 	d.p = 1;		/* Present */
-- 
1.9.3

[PATCH v2 3/3] x86,vdso: Make the PER_CPU segment 32 bits

[Andy Lutomirski]

IMO users ought not to be able to use 16-bit segments without using
modify_ldt.  Fortunately, it's impossible to break espfix64 by
loading the PER_CPU segment into SS because it's a read-only
segment, but marking it 32-bit seems less fragile.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/vsyscall_64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
index ae80ef6..0d71181 100644
--- a/arch/x86/kernel/vsyscall_64.c
+++ b/arch/x86/kernel/vsyscall_64.c
@@ -309,6 +309,7 @@ static void vsyscall_set_cpu(int cpu)
 	d.dpl = 3;		/* Visible to user code */
 	d.s = 1;		/* Not a system segment */
 	d.p = 1;		/* Present */
+	d.d = 1;		/* 32-bit */
 
 	write_gdt_entry(get_cpu_gdt_table(cpu), GDT_ENTRY_PER_CPU, &d, DESCTYPE_S);
 }
-- 
1.9.3

Re: [PATCH v2 0/3] x86: PER_CPU segment improvements

[Andy Lutomirski]

On Wed, Jul 30, 2014 at 3:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> x86 sets up a per-cpu GDT entry so that vgetcpu can use LSL on it
> to determine the CPU number and node.

hpa, if/when you apply this, can you put it onto a fresh branch based
on something at least as new as 3.17-rc1?  It should apply cleanly
regardless, but the nine followup patches that I want to send also
depend on cleanup that isn't in tip/x86/vdso.

Thanks,
Andy

>
> This series, in little baby steps, cleans up that code and sets
> the accessed and 32-bit flags on the segment.
>
> The accessed bit prevents user code from setting the accessed bit
> on its own, and making the segment 32-bit prevents concerns about
> shenanigans involving CPU oddities with 16-bit data segments.
>
> The latter isn't a real problem -- if it were a 16-bit read/write
> segment, it could be used to bypass espfix64, but fortunately
> RO segments can't be loaded into SS.
>
> Changes from v1:
>  - Fix patch 2's changelog.
>  - Learn to spell "Not a system segment".
>
> Andy Lutomirski (3):
>   x86,vdso: Change the PER_CPU segment to use struct desc_struct
>   x86,vdso: Make the PER_CPU segment start out accessed
>   x86,vdso: Make the PER_CPU segment 32 bits
>
>  arch/x86/kernel/vsyscall_64.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> --
> 1.9.3
>



-- 
Andy Lutomirski
AMA Capital Management, LLC