history of extratext sections?

history of extratext sections?

[Robin Getz]

Paul:

I noticed that when passing a zero address to kallsyms_lookup(), the kernel 
thought it was a valid kernel address, even if it was not for the specific 
architecture I was running things on.

This was because is_kernel_extratext() was checking against labels that don't 
exist on many archs. Since PPC is the only kernel which defines _extra_text, 
(which doesn't seem to be used anymore?) there are three options:
 - make the check dependant on PPC
 - make the check dependant on extratext being populated
 - remove _extra_text support from:
    linux-2.6.x/arch/ppc/kernel/vmlinux.lds.S
    linux-2.6.x/include/asm-generic/sections.h
    linux-2.6.x/kernel/kallsyms.c
    linux-2.6.x/scripts/kallsyms.c

Since I don't know the history on that label I thought I would ask (since you 
seem to be the only arch using it) before I sent a patch.

-Robin

Because #1 & #2 are trivial, here is what I was thinking:

 - make the check dependant on PPC
===================================================================
--- linux-2.6.x/kernel/kallsyms.c       (revision 3760)
+++ linux-2.6.x/kernel/kallsyms.c       (working copy)
@@ -51,7 +51,8 @@
 static inline int is_kernel_extratext(unsigned long addr)
 {
+ #ifdef CONFIG_PPC
        if (addr >= (unsigned long)_sextratext
            && addr <= (unsigned long)_eextratext)
                return 1;
+ #endif
        return 0;
 }

OR 

 - make the check dependant on extratext being populated
===================================================================
--- linux-2.6.x/kernel/kallsyms.c       (revision 3760)
+++ linux-2.6.x/kernel/kallsyms.c       (working copy)
@@ -51,7 +51,8 @@
 static inline int is_kernel_extratext(unsigned long addr)
 {
        if (addr >= (unsigned long)_sextratext
-           && addr <= (unsigned long)_eextratext)
+           && addr <= (unsigned long)_eextratext
+           && _sextratext && _eextratext)
                return 1;
        return 0;
 }

Re: history of extratext sections?

[Mike Frysinger]

On 10/24/07, Robin Getz <rgetz@blackfin.uclinux.org> wrote:
>  - make the check dependant on extratext being populated
> ===================================================================
> --- linux-2.6.x/kernel/kallsyms.c       (revision 3760)
> +++ linux-2.6.x/kernel/kallsyms.c       (working copy)
> @@ -51,7 +51,8 @@
>  static inline int is_kernel_extratext(unsigned long addr)
>  {
>         if (addr >= (unsigned long)_sextratext
> -           && addr <= (unsigned long)_eextratext)
> +           && addr <= (unsigned long)_eextratext
> +           && _sextratext && _eextratext)
>                 return 1;
>         return 0;
>  }

i think you'd want:
if (_sextratext && _eextratext &&
  ... everything else ...
-mike

Re: history of extratext sections?

[Robin Getz]

On Wed 24 Oct 2007 08:36, Robin Getz pondered:
> Paul:
> 
> I noticed that when passing a zero address to kallsyms_lookup(), the
> kernel thought it was a valid kernel address, even if it was not for the
> specific architecture it was running on.
> 
> This was because is_kernel_extratext() was checking against labels that
> don't exist on many archs. Since PPC is the only kernel which defines
> _extra_text, (which doesn't seem to be used anymore?) there are three options:
>  - make the check dependant on PPC
>  - make the check dependant on extratext being populated
>  - remove _extra_text support from:
>     linux-2.6.x/arch/ppc/kernel/vmlinux.lds.S
>     linux-2.6.x/include/asm-generic/sections.h
>     linux-2.6.x/kernel/kallsyms.c
>     linux-2.6.x/scripts/kallsyms.c
> 
> Since I don't know the history on that label I thought I would ask
> (since PPC seem to be the only arch using it) before I sent a patch.

OK, it looks like:

David Woodhouse added this functionality 5 May 2005:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=075d6eb16d273dab7b7b4b83fcee8bce4ee387ed

and then Jon Loeliger removed everything in this section 17 Sep 2005
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=6c45ab992e4299c869fb26427944a8f8ea177024

David/Jon:

Is this section still used on PPC, or can the entire support for extratext be removed?

Thanks

Re: history of extratext sections?

[David Woodhouse]

On Fri, 2007-10-26 at 05:57 -0400, Robin Getz wrote:
> Is this section still used on PPC, or can the entire support for
> extratext be removed?

I think it can die.

-- 
dwmw2

Re: history of extratext sections?

[Jon Loeliger]

David Woodhouse wrote:
> On Fri, 2007-10-26 at 05:57 -0400, Robin Getz wrote:
>> Is this section still used on PPC, or can the entire support for
>> extratext be removed?
> 
> I think it can die.
> 

Agreed.   For history, see this:

http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019734.html

and

http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019736.html

where Arnd says:

> The users of the ppc64 function in_kernel_text() can probably be converted
> to the generic is_kernel_text() function.
> 
> 	Arnd <><

and

http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019751.html

Where Paul said:

>> And I assume that the obvious mappings can take place (ie, that
>> "pmac.text" can just be placed in regular .text, etc), right?
> 
> Yes.
> 
> Paul.

HTH,
jdl

[patch] remove support for un-needed _extratext section

[Robin Getz]

From: Robin Getz <rgetz@blackfin.uclinux.org>

when passing a zero address to kallsyms_lookup(), the kernel thought it was a
valid kernel address, even if it is not. This is because is_ksym_addr() called 
is_kernel_extratext() and checked against labels that don't exist on many 
archs (which default as zero). Since PPC was the only kernel which defines 
_extra_text, (in 2005), and no longer needs it, this patch removes _extra_text
support.

 arch/ppc/kernel/vmlinux.lds.S  |    5 -----
 include/asm-generic/sections.h |    2 --
 kernel/kallsyms.c              |   11 +----------
 scripts/kallsyms.c             |   12 +++---------
 4 files changed, 4 insertions(+), 26 deletions(-)

Signed-off-by: Robin Getz <rgetz@blackfin.uclinux.org>
cc: David Woodhouse <dwmw2@infradead.org>
cc: Jon Loeliger <jdl@freescale.com>
cc: Paul Mackerras <paulus@samba.org>

----

For some history (provided by Jon):
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019734.html
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019736.html
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019751.html

Index: kernel/kallsyms.c
===================================================================
--- kernel/kallsyms.c	(revision 3780)
+++ kernel/kallsyms.c	(working copy)
@@ -48,14 +48,6 @@
 	return 0;
 }
 
-static inline int is_kernel_extratext(unsigned long addr)
-{
-	if (addr >= (unsigned long)_sextratext
-	    && addr <= (unsigned long)_eextratext)
-		return 1;
-	return 0;
-}
-
 static inline int is_kernel_text(unsigned long addr)
 {
 	if ((addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
@@ -79,8 +71,7 @@
 	if (all_var)
 		return is_kernel(addr);
 
-	return is_kernel_text(addr) || is_kernel_inittext(addr) ||
-		is_kernel_extratext(addr);
+	return is_kernel_text(addr) || is_kernel_inittext(addr);
 }
 
 /* expand a compressed symbol data into the resulting uncompressed string,
Index: include/asm-generic/sections.h
===================================================================
--- include/asm-generic/sections.h	(revision 3780)
+++ include/asm-generic/sections.h	(working copy)
@@ -8,8 +8,6 @@
 extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
-extern char _sextratext[] __attribute__((weak));
-extern char _eextratext[] __attribute__((weak));
 extern char _end[];
 extern char __per_cpu_start[], __per_cpu_end[];
 extern char __kprobes_text_start[], __kprobes_text_end[];
Index: scripts/kallsyms.c
===================================================================
--- scripts/kallsyms.c	(revision 3780)
+++ scripts/kallsyms.c	(working copy)
@@ -45,7 +45,7 @@
 
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
-static unsigned long long _text, _stext, _etext, _sinittext, _einittext, _sextratext, _eextratext;
+static unsigned long long _text, _stext, _etext, _sinittext, _einittext;
 static unsigned long long _stext_l1, _etext_l1;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
@@ -104,10 +104,6 @@
 		_sinittext = s->addr;
 	else if (strcmp(sym, "_einittext") == 0)
 		_einittext = s->addr;
-	else if (strcmp(sym, "_sextratext") == 0)
-		_sextratext = s->addr;
-	else if (strcmp(sym, "_eextratext") == 0)
-		_eextratext = s->addr;
 	else if (strcmp(sym, "_stext_l1" ) == 0)
 		_stext_l1 = s->addr;
 	else if (strcmp(sym, "_etext_l1" ) == 0)
@@ -175,18 +171,16 @@
 	if (!all_symbols) {
 		if ((s->addr < _stext || s->addr > _etext)
 		    && (s->addr < _sinittext || s->addr > _einittext)
-		    && (s->addr < _sextratext || s->addr > _eextratext)
 		    && (s->addr < _stext_l1 || s->addr > _etext_l1))
 			return 0;
 		/* Corner case.  Discard any symbols with the same value as
-		 * _etext _einittext or _eextratext; they can move between pass
+		 * _etext; they can move between pass
 		 * 1 and 2 when the kallsyms data are added.  If these symbols
 		 * move then they may get dropped in pass 2, which breaks the
 		 * kallsyms rules.
 		 */
 		if ((s->addr == _etext && strcmp((char*)s->sym + offset, "_etext")) ||
-		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
-		    (s->addr == _eextratext && strcmp((char*)s->sym + offset, "_eextratext")))
+		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")))
 			return 0;
 	}
 
Index: arch/ppc/kernel/vmlinux.lds.S
===================================================================
--- arch/ppc/kernel/vmlinux.lds.S	(revision 3780)
+++ arch/ppc/kernel/vmlinux.lds.S	(working copy)
@@ -144,11 +144,6 @@
 
   . = ALIGN(4096);
   __init_end = .;
-
-  . = ALIGN(4096);
-  _sextratext = .;
-  _eextratext = .;
-
   __bss_start = .;
   .bss       :
   {

Re: [patch] remove support for un-needed _extratext section

[Andrew Morton]

On Fri, 26 Oct 2007 15:43:56 -0400
Robin Getz <rgetz@blackfin.uclinux.org> wrote:

> From: Robin Getz <rgetz@blackfin.uclinux.org>
> 
> when passing a zero address to kallsyms_lookup(), the kernel thought it was a
> valid kernel address, even if it is not. This is because is_ksym_addr() called 
> is_kernel_extratext() and checked against labels that don't exist on many 
> archs (which default as zero). Since PPC was the only kernel which defines 
> _extra_text, (in 2005), and no longer needs it, this patch removes _extra_text
> support.

I hit numerous rejects here.  I am not sure which kernel you patched but I
suspect it was not an up-to-date one.

> --- kernel/kallsyms.c	(revision 3780)
> +++ kernel/kallsyms.c	(working copy)

Please prepare patches in `patch -p1' form.  This should have been

--- foo/kernel/kallsyms.c	(revision 3780)
+++ bar/kernel/kallsyms.c	(working copy)

>  		 */
>  		if ((s->addr == _etext && strcmp((char*)s->sym + offset, "_etext")) ||
> -		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
> -		    (s->addr == _eextratext && strcmp((char*)s->sym + offset, "_eextratext")))
> +		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")))
>  			return 0;
>  	}

I don't understand why this hunk is changing _einittext handling, and I
suspect this was a mistake, so I left that bit out.


I ended up with this:


From: Robin Getz <rgetz@blackfin.uclinux.org>

When passing a zero address to kallsyms_lookup(), the kernel thought it was
a valid kernel address, even if it is not.  This is because is_ksym_addr()
called is_kernel_extratext() and checked against labels that don't exist on
many archs (which default as zero).  Since PPC was the only kernel which
defines _extra_text, (in 2005), and no longer needs it, this patch removes
_extra_text support.

For some history (provided by Jon):
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019734.html
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019736.html
 http://ozlabs.org/pipermail/linuxppc-dev/2005-September/019751.html

Signed-off-by: Robin Getz <rgetz@blackfin.uclinux.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jon Loeliger <jdl@freescale.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/ppc/kernel/vmlinux.lds.S  |    5 -----
 include/asm-generic/sections.h |    2 --
 kernel/kallsyms.c              |   11 +----------
 scripts/kallsyms.c             |   24 ++++++++++--------------
 4 files changed, 11 insertions(+), 31 deletions(-)

diff -puN arch/ppc/kernel/vmlinux.lds.S~remove-support-for-un-needed-_extratext-section arch/ppc/kernel/vmlinux.lds.S
--- a/arch/ppc/kernel/vmlinux.lds.S~remove-support-for-un-needed-_extratext-section
+++ a/arch/ppc/kernel/vmlinux.lds.S
@@ -143,11 +143,6 @@ SECTIONS
 
   . = ALIGN(4096);
   __init_end = .;
-
-  . = ALIGN(4096);
-  _sextratext = .;
-  _eextratext = .;
-
   __bss_start = .;
   .bss       :
   {
diff -puN include/asm-generic/sections.h~remove-support-for-un-needed-_extratext-section include/asm-generic/sections.h
--- a/include/asm-generic/sections.h~remove-support-for-un-needed-_extratext-section
+++ a/include/asm-generic/sections.h
@@ -8,8 +8,6 @@ extern char _data[], _sdata[], _edata[];
 extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
-extern char _sextratext[] __attribute__((weak));
-extern char _eextratext[] __attribute__((weak));
 extern char _end[];
 extern char __per_cpu_start[], __per_cpu_end[];
 extern char __kprobes_text_start[], __kprobes_text_end[];
diff -puN kernel/kallsyms.c~remove-support-for-un-needed-_extratext-section kernel/kallsyms.c
--- a/kernel/kallsyms.c~remove-support-for-un-needed-_extratext-section
+++ a/kernel/kallsyms.c
@@ -48,14 +48,6 @@ static inline int is_kernel_inittext(uns
 	return 0;
 }
 
-static inline int is_kernel_extratext(unsigned long addr)
-{
-	if (addr >= (unsigned long)_sextratext
-	    && addr <= (unsigned long)_eextratext)
-		return 1;
-	return 0;
-}
-
 static inline int is_kernel_text(unsigned long addr)
 {
 	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
@@ -75,8 +67,7 @@ static int is_ksym_addr(unsigned long ad
 	if (all_var)
 		return is_kernel(addr);
 
-	return is_kernel_text(addr) || is_kernel_inittext(addr) ||
-		is_kernel_extratext(addr);
+	return is_kernel_text(addr) || is_kernel_inittext(addr);
 }
 
 /* expand a compressed symbol data into the resulting uncompressed string,
diff -puN scripts/kallsyms.c~remove-support-for-un-needed-_extratext-section scripts/kallsyms.c
--- a/scripts/kallsyms.c~remove-support-for-un-needed-_extratext-section
+++ a/scripts/kallsyms.c
@@ -41,7 +41,7 @@ struct sym_entry {
 
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
-static unsigned long long _text, _stext, _etext, _sinittext, _einittext, _sextratext, _eextratext;
+static unsigned long long _text, _stext, _etext, _sinittext, _einittext;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
 
@@ -99,10 +99,6 @@ static int read_symbol(FILE *in, struct 
 		_sinittext = s->addr;
 	else if (strcmp(sym, "_einittext") == 0)
 		_einittext = s->addr;
-	else if (strcmp(sym, "_sextratext") == 0)
-		_sextratext = s->addr;
-	else if (strcmp(sym, "_eextratext") == 0)
-		_eextratext = s->addr;
 	else if (toupper(stype) == 'A')
 	{
 		/* Keep these useful absolute symbols */
@@ -165,18 +161,18 @@ static int symbol_valid(struct sym_entry
 	 * and inittext sections are discarded */
 	if (!all_symbols) {
 		if ((s->addr < _stext || s->addr > _etext)
-		    && (s->addr < _sinittext || s->addr > _einittext)
-		    && (s->addr < _sextratext || s->addr > _eextratext))
+		    && (s->addr < _sinittext || s->addr > _einittext))
 			return 0;
 		/* Corner case.  Discard any symbols with the same value as
-		 * _etext _einittext or _eextratext; they can move between pass
-		 * 1 and 2 when the kallsyms data are added.  If these symbols
-		 * move then they may get dropped in pass 2, which breaks the
-		 * kallsyms rules.
+		 * _etext _einittext; they can move between pass 1 and 2 when
+		 * the kallsyms data are added.  If these symbols move then
+		 * they may get dropped in pass 2, which breaks the kallsyms
+		 * rules.
 		 */
-		if ((s->addr == _etext && strcmp((char*)s->sym + offset, "_etext")) ||
-		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
-		    (s->addr == _eextratext && strcmp((char*)s->sym + offset, "_eextratext")))
+		if ((s->addr == _etext &&
+				strcmp((char*)s->sym + offset, "_etext")) ||
+		    (s->addr == _einittext &&
+				strcmp((char*)s->sym + offset, "_einittext")))
 			return 0;
 	}
 
_

Re: [patch] remove support for un-needed _extratext section

[Robin Getz]

On Mon 29 Oct 2007 18:22, Andrew Morton pondered:

> I hit numerous rejects here.  I am not sure which kernel you patched but
> I suspect it was not an up-to-date one.

Sorry about that - I will do so in the future. Thanks for reviewing and fixing up.

> > --- kernel/kallsyms.c	(revision 3780)
> > +++ kernel/kallsyms.c	(working copy)
> 
> Please prepare patches in `patch -p1' form.  This should have been
> 
> --- foo/kernel/kallsyms.c	(revision 3780)
> +++ bar/kernel/kallsyms.c	(working copy)

Sorry twice.

> >  		 */
> >  		if ((s->addr == _etext && strcmp((char*)s->sym + offset, "_etext")) ||
> > -		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
> > -		    (s->addr == _eextratext && strcmp((char*)s->sym + offset, "_eextratext")))
> > +		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")))
> >  			return 0;
> >  	}
> 
> I don't understand why this hunk is changing _einittext handling, and I
> suspect this was a mistake, so I left that bit out.

I didn't think I did change the _einittext handling - just removed
_eextratext, and removed the trailing ||, and closed the parentheses for 
the if statement.

-  (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
+  (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")))

I think this is the same as what you have here - isn't it?

> -		if ((s->addr == _etext && strcmp((char*)s->sym + offset, "_etext")) ||
> -		    (s->addr == _einittext && strcmp((char*)s->sym + offset, "_einittext")) ||
> -		    (s->addr == _eextratext && strcmp((char*)s->sym + offset, "_eextratext")))
> +		if ((s->addr == _etext &&
> +				strcmp((char*)s->sym + offset, "_etext")) ||
> +		    (s->addr == _einittext &&
> +				strcmp((char*)s->sym + offset, "_einittext")))
>  			return 0;

?