Skip to contentSkip to conversation

kallsyms: Add gate page and all symbols support

PATCH Applied Keith Owens 3 participants 8 messages Nov 28, 2004 – Jan 18, 2005

Subsystems (2)
Memory management MM core
lore
Share & tools
lore

Keith Owens

author
#0 lore raw

cover kallsyms: Add gate page and all symbols support

Three patches with the overall aim of improving kernel backtraces using kallsyms.

1 Clean up the special casing of in_gate_area().

2 Add in_gate_area_no_task() for use from places where no task is valid.

3 Treat the gate page as part of the kernel. Honour CONFIG_KALLSYMS_ALL.

author
#1 lore raw

PATCH 1/3 kallsyms: Clean up x86-64 special casing of in_gate_area()

x86-64 has special case code for in_gate_area(), but it is not clean.

* Replace CONFIG_ARCH_GATE_AREA with __HAVE_ARCH_GATE_AREA. ARCH_GATE_AREA is not a config option.

* The definitions of get_gate_vma() and in_gate_area() are identical in include/asm-x86_64/page.h and include/linux/mm.h. Fold the duplicate definitions into include/linux/mm.h.

Does not affect kallsyms directly, this patch just creates a clean base for patch 2.

Signed-off-by Keith Owens <[email protected]>
include/asm-x86_64/page.h +1 -7
Index: 2.6.10-rc2-bk11/include/asm-x86_64/page.h
===================================================================
@@ -134,13 +134,7 @@ extern __inline__ int get_order(unsigned
 	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
 	 VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
 
-#define CONFIG_ARCH_GATE_AREA 1	
-
-#ifndef __ASSEMBLY__
-struct task_struct;
-struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
-int in_gate_area(struct task_struct *task, unsigned long addr);
-#endif
+#define __HAVE_ARCH_GATE_AREA 1	
 
 #endif /* __KERNEL__ */
include/linux/mm.h +0 -2
Index: 2.6.10-rc2-bk11/include/linux/mm.h
===================================================================
@@ -799,10 +799,8 @@ kernel_map_pages(struct page *page, int 
 }
 #endif
 
-#ifndef CONFIG_ARCH_GATE_AREA
 extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
 int in_gate_area(struct task_struct *task, unsigned long addr);
-#endif
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
mm/memory.c +2 -2
Index: 2.6.10-rc2-bk11/mm/memory.c
===================================================================
@@ -1811,7 +1811,7 @@ unsigned long vmalloc_to_pfn(void * vmal
 
 EXPORT_SYMBOL(vmalloc_to_pfn);
 
-#if !defined(CONFIG_ARCH_GATE_AREA)
+#if !defined(__HAVE_ARCH_GATE_AREA)
 
 #if defined(AT_SYSINFO_EHDR)
 struct vm_area_struct gate_vma;

@@ -1846,4 +1846,4 @@ int in_gate_area(struct task_struct *tas
 	return 0;
 }
 
-#endif
+#endif	/* __HAVE_ARCH_GATE_AREA */
author
#2 lore raw

PATCH 2/3 kallsyms: Add in_gate_area_no_task()

In reply to Keith Owens

Add in_gate_area_no_task() for use in places where no task is valid (e.g. kallsyms). If you have a valid task, use in_gate_area() as before.

Signed-off-by Keith Owens <[email protected]>
arch/x86_64/mm/init.c +10 -0
Index: 2.6.10-rc2-bk11/arch/x86_64/mm/init.c
===================================================================
@@ -623,3 +623,13 @@ int in_gate_area(struct task_struct *tas
 	struct vm_area_struct *vma = get_gate_vma(task);
 	return (addr >= vma->vm_start) && (addr < vma->vm_end);
 }
+
+/* Use this when you have no reliable task/vma, typically from interrupt
+ * context.  It is less reliable than using the task's vma and may give
+ * false positives.
+ */
+int in_gate_area_no_task(unsigned long addr)
+{
+	return ((addr >= VSYSCALL_START) && (addr < VSYSCALL_END) ||
+		(addr >= VSYSCALL32_BASE) && (addr < VSYSCALL32_END));
+}
include/linux/mm.h +6 -0
Index: 2.6.10-rc2-bk11/include/linux/mm.h
===================================================================
@@ -800,7 +800,13 @@ kernel_map_pages(struct page *page, int 
 #endif
 
 extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
+#ifdef	__HAVE_ARCH_GATE_AREA
+int in_gate_area_no_task(unsigned long addr);
 int in_gate_area(struct task_struct *task, unsigned long addr);
+#else
+int in_gate_area_no_task(unsigned long addr);
+#define in_gate_area(task, addr) ({(void)task; in_gate_area_no_task(addr);})
+#endif	/* __HAVE_ARCH_GATE_AREA */
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
mm/memory.c +1 -1
Index: 2.6.10-rc2-bk11/mm/memory.c
===================================================================
@@ -1837,7 +1837,7 @@ struct vm_area_struct *get_gate_vma(stru
 #endif
 }
 
-int in_gate_area(struct task_struct *task, unsigned long addr)
+int in_gate_area_no_task(unsigned long addr)
 {
 #ifdef AT_SYSINFO_EHDR
 	if ((addr >= FIXADDR_USER_START) && (addr < FIXADDR_USER_END))
author
#3 lore raw

PATCH 3/3 kallsyms: gate page is part of the kernel, honour CONFIG_KALLSYMS_ALL

In reply to Keith Owens

* Treat the gate page as part of the kernel, to improve kernel backtraces.

* Honour CONFIG_KALLSYMS_ALL, all symbols are valid, not just text.

Signed-off-by Keith Owens <[email protected]>
kernel/kallsyms.c +11 -4
Index: 2.6.10-rc2-bk11/kernel/kallsyms.c
===================================================================
@@ -18,6 +18,13 @@
 #include <linux/fs.h>
 #include <linux/err.h>
 #include <linux/proc_fs.h>
+#include <linux/mm.h>
+
+#ifdef CONFIG_KALLSYMS_ALL
+#define all_var 1
+#else
+#define all_var 0
+#endif
 
 /* These will be re-linked against their real values during the second link stage */
 extern unsigned long kallsyms_addresses[] __attribute__((weak));

@@ -30,7 +37,7 @@ extern u16 kallsyms_token_index[] __attr
 extern unsigned long kallsyms_markers[] __attribute__((weak));
 
 /* Defined by the linker script. */
-extern char _stext[], _etext[], _sinittext[], _einittext[];
+extern char _stext[], _etext[], _sinittext[], _einittext[], _end[];
 
 static inline int is_kernel_inittext(unsigned long addr)
 {

@@ -44,7 +51,7 @@ static inline int is_kernel_text(unsigne
 {
 	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
 		return 1;
-	return 0;
+	return in_gate_area_no_task(addr);
 }
 
 /* expand a compressed symbol data into the resulting uncompressed string,

@@ -147,7 +154,7 @@ const char *kallsyms_lookup(unsigned lon
 	namebuf[KSYM_NAME_LEN] = 0;
 	namebuf[0] = 0;
 
-	if (is_kernel_text(addr) || is_kernel_inittext(addr)) {
+	if (all_var || is_kernel_text(addr) || is_kernel_inittext(addr)) {
 		unsigned long symbol_end=0;
 
 		/* do a binary search on the sorted kallsyms_addresses array */

@@ -181,7 +188,7 @@ const char *kallsyms_lookup(unsigned lon
 			if (is_kernel_inittext(addr))
 				symbol_end = (unsigned long)_einittext;
 			else
-				symbol_end = (unsigned long)_etext;
+				symbol_end = all_var ? (unsigned long)_end : (unsigned long)_etext;
 		}
 
 		*symbolsize = symbol_end - kallsyms_addresses[low];
Applied
#4 lore raw
In reply to Keith Owens
Quoted 9 lines · Keith Owens

Three patches with the overall aim of improving kernel backtraces using kallsyms.

1 Clean up the special casing of in_gate_area().

2 Add in_gate_area_no_task() for use from places where no task is valid.

3 Treat the gate page as part of the kernel. Honour CONFIG_KALLSYMS_ALL.

Applied all three patches - thanks.

Sam

#5 lore raw
Quoted 1 line · Sam Ravnborg
Quoted 1 line

2 Add in_gate_area_no_task() for use from places where no task is valid.

Can you back that out ? Or at least explain why you need to add this "no_task" thing and not just use "current" where no task is available ? I think the above is bogus for 3 reasons:

- I tend to dislike adding functions
"foo_with_that_parameter_instead_of_that" ... We do it here or there, but the less we do it, the happier I am :)

- Since you unconditionally #define in_gate_area() to use
in_gate_area_no_task(), what is the point of having in_gate_area() at all ? Which rather means, what is the point of adding that "_no_task" version and not just change in_gate_area to not take a task ?

- I dislike the fact that you now define the prototype of the function
in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing so, since i want to inline it (to nothing in the ppc64 case since the vDSO I'm implementing doesn't need any special treatement of the gate area, it's a normal VMA added to the mm's at exec time).

Ben.

author
#6 lore raw
Quoted 6 lines · Benjamin Herrenschmidt
Quoted 1 line · Sam Ravnborg
Quoted 1 line

2 Add in_gate_area_no_task() for use from places where no task is valid.

Can you back that out ? Or at least explain why you need to add this "no_task" thing and not just use "current" where no task is available ?

kallsyms is used to look up a symbol for any task, e.g. to do a backtrace with symbol lookup of all running tasks, not just the current one. None of the kallsym interfaces allow you to specify which task you are making the query against, and the change to do so is too messy and intrusive for far too little return. The no_task variant asks "is this a possible gate address for any task?", at the small risk of getting false positives in kallsyms lookup.

Quoted 4 lines

- Since you unconditionally #define in_gate_area() to use in_gate_area_no_task(), what is the point of having in_gate_area() at all ? Which rather means, what is the point of adding that "_no_task" version and not just change in_gate_area to not take a task ?

x86-64 needs both variants. in_gate_area() is sometimes called in a context where you know the required task (mm/memory.c), sometimes when any task is implied (kernel/kallsyms.c). x86-64 makes it more complicated by using different gate pages depending on whether the specified task is in 32 bit emulation mode or not.

Quoted 3 lines

- I dislike the fact that you now define the prototype of the function in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing so, since i want to inline it

Maybe. I dislike copying definitions to multiple asm headers. If you think that the win of inlining the ppc64 version of these functions outweighs the header duplication then send a patch. Don't forget to duplicate the definition in include/asm-x86_64 as well.

Quoted 3 lines

(to nothing in the ppc64 case since the
vDSO I'm implementing doesn't need any special treatement of the gate area, it's a normal VMA added to the mm's at exec time).

Added to specific task's mm or to all tasks? If the gate VMA varies according to the task then you have to support the kallsyms "is this a possible gate address for any task?" question, like x86-64.

#7 lore raw
Quoted 10 lines
Quoted 5 lines

- I dislike the fact that you now define the prototype of the function in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing so, since i want to inline it

Maybe. I dislike copying definitions to multiple asm headers. If you think that the win of inlining the ppc64 version of these functions outweighs the header duplication then send a patch. Don't forget to duplicate the definition in include/asm-x86_64 as well.

Well, I just want to inline it as en empty function ...

Quoted 7 lines

(to nothing in the ppc64 case since the

Quoted 2 lines

vDSO I'm implementing doesn't need any special treatement of the gate area, it's a normal VMA added to the mm's at exec time).

Added to specific task's mm or to all tasks? If the gate VMA varies according to the task then you have to support the kallsyms "is this a possible gate address for any task?" question, like x86-64.

each task mm can have it differently. why would I need kallsym support ? the ppc64 vDSO support will not be part of the kernel address ranges, but will be somewhere in the normal userland areas.

Ben.