diff mbox

UBUNTU: SAUCE: disable_nx should not be in __cpuinitdata section for X86_32

Message ID 1333027656-8665-1-git-send-email-tim.gardner@canonical.com
State New
Headers show

Commit Message

Tim Gardner March 29, 2012, 1:27 p.m. UTC 
BugLink: http://bugs.launchpad.net/bugs/968233

I noticed a section mismatch warning while building 3.2.0-20.33 for X86_32.

 AR      arch/x86/lib/lib.a
  LD      vmlinux.o
  MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference from the function load_elf_binary() to the variable .cpuinit.data:disable_nx
The function load_elf_binary() references
the variable __cpuinitdata disable_nx.
This is often because load_elf_binary lacks a __cpuinitdata
annotation or the annotation of disable_nx is wrong.

load_elf_binary() is definitely called after initialization.

This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX emulation', so
this is not an upstream problem.

Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 arch/x86/mm/setup_nx.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Tim Gardner March 29, 2012, 1:31 p.m. UTC | #1 
This is for Oneiric
Seth Forshee March 29, 2012, 1:37 p.m. UTC | #2 
On Thu, Mar 29, 2012 at 07:27:36AM -0600, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/968233
> 
> I noticed a section mismatch warning while building 3.2.0-20.33 for X86_32.
> 
>  AR      arch/x86/lib/lib.a
>   LD      vmlinux.o
>   MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference from the function load_elf_binary() to the variable .cpuinit.data:disable_nx
> The function load_elf_binary() references
> the variable __cpuinitdata disable_nx.
> This is often because load_elf_binary lacks a __cpuinitdata
> annotation or the annotation of disable_nx is wrong.
> 
> load_elf_binary() is definitely called after initialization.
> 
> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX emulation', so
> this is not an upstream problem.
> 
> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  arch/x86/mm/setup_nx.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
> index 90c9eff3..89fd946 100644
> --- a/arch/x86/mm/setup_nx.c
> +++ b/arch/x86/mm/setup_nx.c
> @@ -6,7 +6,11 @@
>  #include <asm/pgtable.h>
>  #include <asm/proto.h>
>  
> +#ifdef CONFIG_X86_32
> +int disable_nx; /* referenced by load_elf_binary() */
> +#else
>  int disable_nx __cpuinitdata;
> +#endif
>  
>  /*
>   * noexec = on|off
> -- 
> 1.7.9.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Stefan Bader March 29, 2012, 2:50 p.m. UTC | #3 
On 29.03.2012 15:27, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/968233
> 
> I noticed a section mismatch warning while building 3.2.0-20.33 for X86_32.
> 
>  AR      arch/x86/lib/lib.a
>   LD      vmlinux.o
>   MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference from the function load_elf_binary() to the variable .cpuinit.data:disable_nx
> The function load_elf_binary() references
> the variable __cpuinitdata disable_nx.
> This is often because load_elf_binary lacks a __cpuinitdata
> annotation or the annotation of disable_nx is wrong.
> 
> load_elf_binary() is definitely called after initialization.
> 
> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX emulation', so
> this is not an upstream problem.
> 
> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  arch/x86/mm/setup_nx.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
> index 90c9eff3..89fd946 100644
> --- a/arch/x86/mm/setup_nx.c
> +++ b/arch/x86/mm/setup_nx.c
> @@ -6,7 +6,11 @@
>  #include <asm/pgtable.h>
>  #include <asm/proto.h>
>  
> +#ifdef CONFIG_X86_32
> +int disable_nx; /* referenced by load_elf_binary() */
> +#else
>  int disable_nx __cpuinitdata;
> +#endif
>  
>  /*
>   * noexec = on|off

Hm, maybe I just understand the annotation incorrectly. But I thought it marks
functions and variables as only used during init. Which is wrong on 32bit, but
why is it then still considered needed on 64bit? Probably not even needed if
this is solely used for nx emulation...
Tim Gardner March 29, 2012, 3:09 p.m. UTC | #4 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 03/29/2012 08:50 AM, Stefan Bader wrote:
> On 29.03.2012 15:27, Tim Gardner wrote:
>> BugLink: http://bugs.launchpad.net/bugs/968233
>> 
>> I noticed a section mismatch warning while building 3.2.0-20.33
>> for X86_32.
>> 
>> AR      arch/x86/lib/lib.a LD      vmlinux.o MODPOST vmlinux.o 
>> WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference
>> from the function load_elf_binary() to the variable
>> .cpuinit.data:disable_nx The function load_elf_binary()
>> references the variable __cpuinitdata disable_nx. This is often
>> because load_elf_binary lacks a __cpuinitdata annotation or the
>> annotation of disable_nx is wrong.
>> 
>> load_elf_binary() is definitely called after initialization.
>> 
>> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX
>> emulation', so this is not an upstream problem.
>> 
>> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp> 
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> --- 
>> arch/x86/mm/setup_nx.c |    4 ++++ 1 files changed, 4
>> insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c 
>> index 90c9eff3..89fd946 100644 --- a/arch/x86/mm/setup_nx.c +++
>> b/arch/x86/mm/setup_nx.c @@ -6,7 +6,11 @@ #include
>> <asm/pgtable.h> #include <asm/proto.h>
>> 
>> +#ifdef CONFIG_X86_32 +int disable_nx; /* referenced by
>> load_elf_binary() */ +#else int disable_nx __cpuinitdata; 
>> +#endif
>> 
>> /* * noexec = on|off
> 
> Hm, maybe I just understand the annotation incorrectly. But I
> thought it marks functions and variables as only used during init.
> Which is wrong on 32bit, but why is it then still considered needed
> on 64bit? Probably not even needed if this is solely used for nx
> emulation...
> 

The code in load_elf_binary() that references disable_nx is "#ifdef
CONFIG_X86_32", so its unused in 64 bit.

rtg
- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJPdHsdAAoJED12yEX6FEfKubsP/RGhL3SSO8B+nSB2w9R7nh7h
sWf/c+Hw/+D7UtGxDTRs8OMEzQYkA6zKkBKNSn6U8GHTAaOF1do6mtpZmIWAunLd
7BniyuReiG7oon3MxXx97nsdovxX/LGUE3mBs9S+4mEIfkYttS+gunKc2vveH4ft
ax0m4Xqu30QK9AdaUDuhCxmLrSyWci+l3MSrnmS2HBF6O/+p1MiMzuYL146XFFv6
cK18oqFPCnFREgXvFHxN9afJsjGQtPVRvhCu5TsiYy0o9dYNeDgSZnVzCr+0moiO
L25KrAuDjxsBdkTp4jMpF3kuu1FP9g2JO5snaMOkGRMc6uBDXBF9Ii9wJkBGKd8b
TqRv9bRF2at7eV44zyapk2OXGWIhoWkAfc5pm7UZ30hq8n0SRBjauxKParJLPr0I
2xMsIiq/iAypFjAbFY6YWGzSYua+iO5TIYx3xoOyNr0U7NgovQEQzYMaYdJ1PD/f
3373Gt4J2GDd71sR77LlmEi7m9slD6xlQ0ihLdmIfpchek4paqJ82iIQT+wZVbtS
zslJXpvA3GNfTif8nV2aC3eJWfyb/L9acvyWRevY3aOKFpRMx69/uMqSbtX/vztm
i3Js7MS5d/Eq44Qt1koJ9e4EMx/kAWZ69N403rDFMKGeOkLMyyIyrxUu66YU1Lsi
/8/ejwSvZT0icvv896tv
=arLI
-----END PGP SIGNATURE-----
Stefan Bader March 29, 2012, 3:21 p.m. UTC | #5 
On 29.03.2012 17:09, Tim Gardner wrote:
> On 03/29/2012 08:50 AM, Stefan Bader wrote:
>> On 29.03.2012 15:27, Tim Gardner wrote:
>>> BugLink: http://bugs.launchpad.net/bugs/968233
>>>
>>> I noticed a section mismatch warning while building 3.2.0-20.33
>>> for X86_32.
>>>
>>> AR      arch/x86/lib/lib.a LD      vmlinux.o MODPOST vmlinux.o 
>>> WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference
>>> from the function load_elf_binary() to the variable
>>> .cpuinit.data:disable_nx The function load_elf_binary()
>>> references the variable __cpuinitdata disable_nx. This is often
>>> because load_elf_binary lacks a __cpuinitdata annotation or the
>>> annotation of disable_nx is wrong.
>>>
>>> load_elf_binary() is definitely called after initialization.
>>>
>>> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX
>>> emulation', so this is not an upstream problem.
>>>
>>> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp> 
>>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> --- 
>>> arch/x86/mm/setup_nx.c |    4 ++++ 1 files changed, 4
>>> insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c 
>>> index 90c9eff3..89fd946 100644 --- a/arch/x86/mm/setup_nx.c +++
>>> b/arch/x86/mm/setup_nx.c @@ -6,7 +6,11 @@ #include
>>> <asm/pgtable.h> #include <asm/proto.h>
>>>
>>> +#ifdef CONFIG_X86_32 +int disable_nx; /* referenced by
>>> load_elf_binary() */ +#else int disable_nx __cpuinitdata; 
>>> +#endif
>>>
>>> /* * noexec = on|off
> 
>> Hm, maybe I just understand the annotation incorrectly. But I
>> thought it marks functions and variables as only used during init.
>> Which is wrong on 32bit, but why is it then still considered needed
>> on 64bit? Probably not even needed if this is solely used for nx
>> emulation...
> 
> 
> The code in load_elf_binary() that references disable_nx is "#ifdef
> CONFIG_X86_32", so its unused in 64 bit.
> 
> rtg

My question probably should have been: is an #else required? Which I can answer
myself: yes. So the remaining question just is: would it hurt to just make that
a normal (not __cpuinitdata) variable in all cases? Beside of "wasting" one int.

-Stefan
Stefan Bader March 29, 2012, 3:42 p.m. UTC | #6 
On 29.03.2012 15:27, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/968233
> 
> I noticed a section mismatch warning while building 3.2.0-20.33 for X86_32.
> 
>  AR      arch/x86/lib/lib.a
>   LD      vmlinux.o
>   MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x187833): Section mismatch in reference from the function load_elf_binary() to the variable .cpuinit.data:disable_nx
> The function load_elf_binary() references
> the variable __cpuinitdata disable_nx.
> This is often because load_elf_binary lacks a __cpuinitdata
> annotation or the annotation of disable_nx is wrong.
> 
> load_elf_binary() is definitely called after initialization.
> 
> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX emulation', so
> this is not an upstream problem.
> 
> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  arch/x86/mm/setup_nx.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
> index 90c9eff3..89fd946 100644
> --- a/arch/x86/mm/setup_nx.c
> +++ b/arch/x86/mm/setup_nx.c
> @@ -6,7 +6,11 @@
>  #include <asm/pgtable.h>
>  #include <asm/proto.h>
>  
> +#ifdef CONFIG_X86_32
> +int disable_nx; /* referenced by load_elf_binary() */
> +#else
>  int disable_nx __cpuinitdata;
> +#endif
>  
>  /*
>   * noexec = on|off

Meh, I guess it is optimization of size.
Tim Gardner March 29, 2012, 3:42 p.m. UTC | #7 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 03/29/2012 09:21 AM, Stefan Bader wrote:
> On 29.03.2012 17:09, Tim Gardner wrote:
>> On 03/29/2012 08:50 AM, Stefan Bader wrote:
>>> On 29.03.2012 15:27, Tim Gardner wrote:
>>>> BugLink: http://bugs.launchpad.net/bugs/968233
>>>> 
>>>> I noticed a section mismatch warning while building
>>>> 3.2.0-20.33 for X86_32.
>>>> 
>>>> AR      arch/x86/lib/lib.a LD      vmlinux.o MODPOST
>>>> vmlinux.o WARNING: vmlinux.o(.text+0x187833): Section
>>>> mismatch in reference from the function load_elf_binary() to
>>>> the variable .cpuinit.data:disable_nx The function
>>>> load_elf_binary() references the variable __cpuinitdata
>>>> disable_nx. This is often because load_elf_binary lacks a
>>>> __cpuinitdata annotation or the annotation of disable_nx is
>>>> wrong.
>>>> 
>>>> load_elf_binary() is definitely called after initialization.
>>>> 
>>>> This code was added by 'UBUNTU: ubuntu: nx-emu - i386: NX 
>>>> emulation', so this is not an upstream problem.
>>>> 
>>>> Reported-by: Tetsuo Handa <from-ubuntu@I-love.SAKURA.ne.jp> 
>>>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> --- 
>>>> arch/x86/mm/setup_nx.c |    4 ++++ 1 files changed, 4 
>>>> insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
>>>>  index 90c9eff3..89fd946 100644 --- a/arch/x86/mm/setup_nx.c
>>>> +++ b/arch/x86/mm/setup_nx.c @@ -6,7 +6,11 @@ #include 
>>>> <asm/pgtable.h> #include <asm/proto.h>
>>>> 
>>>> +#ifdef CONFIG_X86_32 +int disable_nx; /* referenced by 
>>>> load_elf_binary() */ +#else int disable_nx __cpuinitdata; 
>>>> +#endif
>>>> 
>>>> /* * noexec = on|off
>> 
>>> Hm, maybe I just understand the annotation incorrectly. But I 
>>> thought it marks functions and variables as only used during
>>> init. Which is wrong on 32bit, but why is it then still
>>> considered needed on 64bit? Probably not even needed if this is
>>> solely used for nx emulation...
>> 
>> 
>> The code in load_elf_binary() that references disable_nx is
>> "#ifdef CONFIG_X86_32", so its unused in 64 bit.
>> 
>> rtg
> 
> My question probably should have been: is an #else required? Which
> I can answer myself: yes. So the remaining question just is: would
> it hurt to just make that a normal (not __cpuinitdata) variable in
> all cases? Beside of "wasting" one int.
> 
> -Stefan
> 

It probably wouldn't make any difference. I just chose not to alter
the amd64 behavior.

rtg
- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJPdILyAAoJED12yEX6FEfKybEP+weTmqOuhb8SAE5c8sK9otYh
iYy/vzCI/UkbHT4+QmlPiiD6K5aoyLcuNWGjp6xzDHmvrBdVKY87SzqoHozqLXol
cJv8svb6e3cCJ+thzvt1F1ZBMj4c6fTJ7U098M+65zDwpcoau1SSXzhcimnR4o1/
BAi9KqsJzWkF6dQokv6Llw7JIs6Yg4X8jUkmCilzugWe4hdIJdte711qLvyHd7J9
VTgxuG9LQ+BFygkIosIhK8y0QLihyuAVkWq0d+0tNh2k4bEpAF3i2G8pGNAS11zx
/6AoZhjnLiLmZMqL3Ug6/lTfmUfcU5B4yKJij98tQIwLMCZ9lcrZwKDz+PNPn1kK
ReIENg0IqJI0eMBryyjAQYHuoVkcpwuPWQmp7d3dTojjkDL85tfAzxUsPjKsTlQG
ji+T/fOLAWAyYy0f5C2zK5zdStc6tqNjUI87Lqk1y+yzn06N/bkI9eIsEWKOJus2
rUHfzHAGZ1QFCgFtbyy3VcthVc7sfEWrIGPVaCbAWzTqcTQlzJOGydLYdkBGDtVq
Z+esDXuESCRnQk0MxY74PIOvIhv5/maC7AsSP+z6D1BhaGzgEaKL+yHEHLhqfmbZ
DEQI18HFstQtmfk0NI4EkiA9Gds6sU128gRMZGKgQ8uNlUQMuuyIfiEeg09QaINs
SVNFTXYfmcFc5YJUVkM8
=qZor
-----END PGP SIGNATURE-----
Tim Gardner March 29, 2012, 3:48 p.m. UTC | #8
diff mbox

Patch

diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
index 90c9eff3..89fd946 100644
--- a/arch/x86/mm/setup_nx.c
+++ b/arch/x86/mm/setup_nx.c
@@ -6,7 +6,11 @@ 
 #include <asm/pgtable.h>
 #include <asm/proto.h>
 
+#ifdef CONFIG_X86_32
+int disable_nx; /* referenced by load_elf_binary() */
+#else
 int disable_nx __cpuinitdata;
+#endif
 
 /*
  * noexec = on|off