[PATCH 0/4] x86: fix free_thread_info() with uninitalized thread_info

[PATCH 0/4] x86: fix free_thread_info() with uninitalized thread_info

[Akinobu Mita]

This patch series fixes a problem described below. The actual fix is only
made by the patch 3/4. The rest of patches help it make simple and there
is no actual behavioral change.

x86 arch specific free_thread_info() accesses thread_info->task to call
free_thread_xstate(). But the thread_info may not be initialized yet.
So invalid pointer derefence may happen in free_thread_xstate().

It happens in the following scenario in dup_task_struct()

1. call alloc_task_struct() to allocate empty task_struct
2. call alloc_thread_info() to allocate empty thread_info
3. call arch_dup_task_struct()

x86 arch specific arch_dup_task_struct() copies task_struct from source
task_struct. it also allocates empty xstate and copy from source if
source task_struct has ->thread.xstate.

If the xstate allocation failed, arch_dup_task_struct() returns error.

4. call free_thread_info() to deallocate thread_info

x86 arch specific free_thread_info() calls free_thread_xstate() with
thread_info->task. But the thread_info is not initialized yet.

[PATCH 1/4] convert task_struct allocator macros to inline functions

[Akinobu Mita]

Usually inline function is better than function-like macro.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 kernel/fork.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Index: 2.6/kernel/fork.c
===================================================================
--- 2.6.orig/kernel/fork.c
+++ 2.6/kernel/fork.c
@@ -92,9 +92,19 @@ int nr_processes(void)
 }
 
 #ifndef __HAVE_ARCH_TASK_STRUCT_ALLOCATOR
-# define alloc_task_struct()	kmem_cache_alloc(task_struct_cachep, GFP_KERNEL)
-# define free_task_struct(tsk)	kmem_cache_free(task_struct_cachep, (tsk))
+
 static struct kmem_cache *task_struct_cachep;
+
+static inline struct task_struct *alloc_task_struct(void)
+{
+	return kmem_cache_alloc(task_struct_cachep, GFP_KERNEL);
+}
+
+static inline void free_task_struct(struct task_struct *tsk)
+{
+	kmem_cache_free(task_struct_cachep, tsk);
+}
+
 #endif
 
 #ifndef __HAVE_ARCH_THREAD_INFO_ALLOCATOR

[PATCH 2/4] x86: arch specific task_struct allocator

[Akinobu Mita]

This patch defines x86 specific task_struct allocator.
It is just duplication of generic task-struct allocator for now.
Forthcoming patch will make actual change to it.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 arch/x86/include/asm/thread_info.h |    6 ++++++
 arch/x86/kernel/process.c          |   16 ++++++++++++++++
 2 files changed, 22 insertions(+)

Index: 2.6/arch/x86/kernel/process.c
===================================================================
--- 2.6.orig/arch/x86/kernel/process.c
+++ 2.6/arch/x86/kernel/process.c
@@ -44,8 +44,24 @@ void free_thread_info(struct thread_info
 	free_pages((unsigned long)ti, get_order(THREAD_SIZE));
 }
 
+static struct kmem_cache *task_struct_cachep;
+
+struct task_struct *alloc_task_struct(void)
+{
+	return kmem_cache_alloc(task_struct_cachep, GFP_KERNEL);
+}
+
+void free_task_struct(struct task_struct *tsk)
+{
+	kmem_cache_free(task_struct_cachep, tsk);
+}
+
 void arch_task_cache_init(void)
 {
+	task_struct_cachep = kmem_cache_create("task_struct",
+				sizeof(struct task_struct), ARCH_MIN_TASKALIGN,
+				SLAB_PANIC, NULL);
+
         task_xstate_cachep =
         	kmem_cache_create("task_xstate", xstate_size,
 				  __alignof__(union thread_xstate),
Index: 2.6/arch/x86/include/asm/thread_info.h
===================================================================
--- 2.6.orig/arch/x86/include/asm/thread_info.h
+++ 2.6/arch/x86/include/asm/thread_info.h
@@ -260,5 +260,11 @@ extern void arch_task_cache_init(void);
 extern void free_thread_info(struct thread_info *ti);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #define arch_task_cache_init arch_task_cache_init
+
+#define __HAVE_ARCH_TASK_STRUCT_ALLOCATOR
+
+extern struct task_struct *alloc_task_struct(void);
+extern void free_task_struct(struct task_struct *tsk);
+
 #endif
 #endif /* _ASM_X86_THREAD_INFO_H */

[PATCH 3/4] x86: call free_thread_xstate() in free_task_struct()

[Akinobu Mita]

x86 arch specific free_thread_info() accesses thread_info->task to call
free_thread_xstate(). But the thread_info may not be initialized yet.
So invalid pointer derefence may happen in free_thread_xstate().

It happens in the following scenario in dup_task_struct()

1. call alloc_task_struct() to allocate empty task_struct
2. call alloc_thread_info() to allocate empty thread_info
3. call arch_dup_task_struct()

x86 arch specific arch_dup_task_struct() copies task_struct from source
task_struct. it also allocates empty xstate and copy from source if
source task_struct has ->thread.xstate.

If the xstate allocation failed, arch_dup_task_struct() returns error.

4. call free_thread_info() to deallocate thread_info

x86 arch specific free_thread_info() calls free_thread_xstate() with
thread_info->task. But the thread_info is not initialized yet.

This patch resolves the issue by moving the free_thread_xstate() call
into free_task_struct().

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 arch/x86/kernel/process.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: 2.6/arch/x86/kernel/process.c
===================================================================
--- 2.6.orig/arch/x86/kernel/process.c
+++ 2.6/arch/x86/kernel/process.c
@@ -40,7 +40,6 @@ void free_thread_xstate(struct task_stru
 
 void free_thread_info(struct thread_info *ti)
 {
-	free_thread_xstate(ti->task);
 	free_pages((unsigned long)ti, get_order(THREAD_SIZE));
 }
 
@@ -53,6 +52,7 @@ struct task_struct *alloc_task_struct(vo
 
 void free_task_struct(struct task_struct *tsk)
 {
+	free_thread_xstate(tsk);
 	kmem_cache_free(task_struct_cachep, tsk);
 }
 

[PATCH 4/4] x86: use generic thread_info allocator

[Akinobu Mita]

Use generic thread_info allocator for x86.
Because thread_info allocator for x86 and generic one are same now.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 arch/x86/include/asm/page_32.h     |    6 +++---
 arch/x86/include/asm/page_64.h     |    4 ++--
 arch/x86/include/asm/thread_info.h |   13 -------------
 arch/x86/kernel/process.c          |    5 -----
 4 files changed, 5 insertions(+), 23 deletions(-)

Index: 2.6/arch/x86/include/asm/thread_info.h
===================================================================
--- 2.6.orig/arch/x86/include/asm/thread_info.h
+++ 2.6/arch/x86/include/asm/thread_info.h
@@ -147,18 +147,6 @@ struct thread_info {
 
 #define PREEMPT_ACTIVE		0x10000000
 
-/* thread information allocation */
-#ifdef CONFIG_DEBUG_STACK_USAGE
-#define THREAD_FLAGS (GFP_KERNEL | __GFP_ZERO)
-#else
-#define THREAD_FLAGS GFP_KERNEL
-#endif
-
-#define __HAVE_ARCH_THREAD_INFO_ALLOCATOR
-
-#define alloc_thread_info(tsk)						\
-	((struct thread_info *)__get_free_pages(THREAD_FLAGS, THREAD_ORDER))
-
 #ifdef CONFIG_X86_32
 
 #define STACK_WARN	(THREAD_SIZE/8)
@@ -257,7 +245,6 @@ static inline void set_restore_sigmask(v
 
 #ifndef __ASSEMBLY__
 extern void arch_task_cache_init(void);
-extern void free_thread_info(struct thread_info *ti);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #define arch_task_cache_init arch_task_cache_init
 
Index: 2.6/arch/x86/kernel/process.c
===================================================================
--- 2.6.orig/arch/x86/kernel/process.c
+++ 2.6/arch/x86/kernel/process.c
@@ -38,11 +38,6 @@ void free_thread_xstate(struct task_stru
 	}
 }
 
-void free_thread_info(struct thread_info *ti)
-{
-	free_pages((unsigned long)ti, get_order(THREAD_SIZE));
-}
-
 static struct kmem_cache *task_struct_cachep;
 
 struct task_struct *alloc_task_struct(void)
Index: 2.6/arch/x86/include/asm/page_32.h
===================================================================
--- 2.6.orig/arch/x86/include/asm/page_32.h
+++ 2.6/arch/x86/include/asm/page_32.h
@@ -14,11 +14,11 @@
 #define __PAGE_OFFSET		_AC(CONFIG_PAGE_OFFSET, UL)
 
 #ifdef CONFIG_4KSTACKS
-#define THREAD_ORDER	0
+#define THREAD_SIZE_ORDER	0
 #else
-#define THREAD_ORDER	1
+#define THREAD_SIZE_ORDER	1
 #endif
-#define THREAD_SIZE 	(PAGE_SIZE << THREAD_ORDER)
+#define THREAD_SIZE 	(PAGE_SIZE << THREAD_SIZE_ORDER)
 
 #define STACKFAULT_STACK 0
 #define DOUBLEFAULT_STACK 1
Index: 2.6/arch/x86/include/asm/page_64.h
===================================================================
--- 2.6.orig/arch/x86/include/asm/page_64.h
+++ 2.6/arch/x86/include/asm/page_64.h
@@ -3,8 +3,8 @@
 
 #define PAGETABLE_LEVELS	4
 
-#define THREAD_ORDER	1
-#define THREAD_SIZE  (PAGE_SIZE << THREAD_ORDER)
+#define THREAD_SIZE_ORDER	1
+#define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
 #define CURRENT_MASK (~(THREAD_SIZE - 1))
 
 #define EXCEPTION_STACK_ORDER 0