[PATCH] Make LIST_POISON less deadly (v3)

[PATCH] Make LIST_POISON less deadly (v3)

[Avi Kivity]

The list macros use LIST_POISON1 and LIST_POISON2 as undereferencable
pointers in order to trap erronous use of freed list_heads.  Unfortunately
userspace can arrange for those pointers to actually be dereferencable,
potentially turning an oops to an expolit.

To avoid this allow architectures (currently x86_64 only) to override
the default values for these pointers with truly-undereferncable values.
This is easy on x86_64 as the virtual address space is large and contains
unmapped ranges.

Signed-off-by: Avi Kivity <avi@qumranet.com>
---

Changes since v1:
- add CONFIG_ILLEGAL_POINTER_VALUE instead of new header <asm/poison.h>

Changes since v2:
- switched the range from an uncanonical address to a canonical unmapped
  address, since uncannonical addresses generate #GP instead of #PF, losing
  the faulting address.

 arch/x86/Kconfig       |    5 +++++
 include/linux/poison.h |   10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fe361ae..54d4e36 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1021,6 +1021,11 @@ config ARCH_MEMORY_PROBE
 	def_bool X86_64
 	depends on MEMORY_HOTPLUG
 
+config ILLEGAL_POINTER_VALUE
+       hex
+       default 0 if X86_32
+       default 0xffffc10000000000 if X86_64
+
 source "mm/Kconfig"
 
 config HIGHPTE
diff --git a/include/linux/poison.h b/include/linux/poison.h
index 9f31683..0d105a5 100644
--- a/include/linux/poison.h
+++ b/include/linux/poison.h
@@ -1,14 +1,20 @@
 #ifndef _LINUX_POISON_H
 #define _LINUX_POISON_H
 
+#ifdef CONFIG_ILLEGAL_POINTER_VALUE
+#define POISON_POINTER_DELTA CONFIG_ILLEGAL_POINTER_VALUE
+#else
+#define POISON_POINTER_DELTA 0L
+#endif
+
 /********** include/linux/list.h **********/
 /*
  * These are non-NULL pointers that will result in page faults
  * under normal circumstances, used to verify that nobody uses
  * non-initialized list entries.
  */
-#define LIST_POISON1  ((void *) 0x00100100)
-#define LIST_POISON2  ((void *) 0x00200200)
+#define LIST_POISON1  ((void *) 0x00100100 + POISON_POINTER_DELTA)
+#define LIST_POISON2  ((void *) 0x00200200 + POISON_POINTER_DELTA)
 
 /********** include/linux/timer.h **********/
 /*
-- 
1.5.5.1

Re: [PATCH] Make LIST_POISON less deadly (v3)

[Ingo Molnar]

* Avi Kivity <avi@qumranet.com> wrote:

> The list macros use LIST_POISON1 and LIST_POISON2 as undereferencable 
> pointers in order to trap erronous use of freed list_heads.  
> Unfortunately userspace can arrange for those pointers to actually be 
> dereferencable, potentially turning an oops to an expolit.
> 
> To avoid this allow architectures (currently x86_64 only) to override 
> the default values for these pointers with truly-undereferncable 
> values. This is easy on x86_64 as the virtual address space is large 
> and contains unmapped ranges.

looks good - thanks Avi - i've added your patch to the -tip tree. I have 
opened a separate topic for it: tip/safe-poison-pointers. (because it 
affects other architectures as well)

	Ingo

Re: [PATCH] Make LIST_POISON less deadly (v3)

[Andi Kleen]

Avi Kivity <avi@qumranet.com> writes:
>  
> +config ILLEGAL_POINTER_VALUE
> +       hex
> +       default 0 if X86_32
> +       default 0xffffc10000000000 if X86_64

Don't make it exactly 0xffffc10000000000 but 0xffffc10000000000 + 0x1000000
or so, otherwise list_entry() users which subtract offsets would still
end up in user space and there might be actually something in there
by default.

-Andi

Re: [PATCH] Make LIST_POISON less deadly (v3)

[Avi Kivity]

Andi Kleen wrote:
> Avi Kivity <avi@qumranet.com> writes:
>   
>>  
>> +config ILLEGAL_POINTER_VALUE
>> +       hex
>> +       default 0 if X86_32
>> +       default 0xffffc10000000000 if X86_64
>>     
>
> Don't make it exactly 0xffffc10000000000 but 0xffffc10000000000 + 0x1000000
> or so, otherwise list_entry() users which subtract offsets would still
> end up in user space and there might be actually something in there
> by default.
>   

The poison code adds a large offset (>1MB) so we're safe here.

-- 
error compiling committee.c: too many arguments to function