2.6.19-git13: uts banner changes break SLES9 (at least)

2.6.19-git13: uts banner changes break SLES9 (at least)

[Andy Whitcroft]

test.kernel.org testing seems to have shaken out a problem with the 
kernel banner changing, introduced by this commit:

	[PATCH] Fix linux banner utsname information
	commit a2ee8649ba6d71416712e798276bf7c40b64e6e5

We first noticed it with 2.6.19-git13 as we use this version string as 
part of our boot validation process, which started tripping for every 
job.  Although we have been able to modify our validation, I am 
concerned that this is a widespread mechanism for finding the version of 
the kernel from non-running kernels.  It appears that SLES9 and possibly 
SLES10 is going to be affected too.

On a SLES9 box here, making an initrd for this kernel fails as below:

	Module list:	sym53c8xx reiserfs
	Kernel version:	%s (powerpc)
	Kernel image:	/boot/vmlinuz-autobench
	Initrd image:	/boot/initrd-autobench.img.new
	No modules found for kernel %s

If you follow the initrd build process it appears that they look at the 
compressed kernel and extract the internal version number from it, in 
order to find the modules.  For this they use the get_kernel_version, 
which starts returning %s with this change:

	# get_kernel_version /boot/vmlinuz-autobench
	%s

Obviously this method is dubious at best for finding the kernel version 
here.  I do wonder if there should be some approved interface for 
getting this information out of the kernel.  Perhaps something similar 
to the IKCFG_ST<config>IKCFG_ED bracketing the uname structure or something.

Andi, just a heads up.

-apw

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Andy Whitcroft wrote:

> 	# get_kernel_version /boot/vmlinuz-autobench
> 	%s

It expects the content from `cat /proc/version`:

...
      for (i = 0; i < in; i++)
        if (buffer[i] == 'L' && buffer[i+1] == 'i' &&
            buffer[i+2] == 'n' && buffer[i+3] == 'u' &&
            buffer[i+4] == 'x' && buffer[i+5] == ' ' &&
            buffer[i+6] == 'v' && buffer[i+7] == 'e' &&
            buffer[i+8] == 'r' && buffer[i+9] == 's' &&
            buffer[i+10] == 'i' && buffer[i+11] == 'o' &&
            buffer[i+12] == 'n' && buffer[i+13] == ' ')
          { 
            found = 1;
            break;
          } 
...

The change breaks the assumption:
 const char linux_banner[] =
-       "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
-       LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
+       "Linux version %s (" LINUX_COMPILE_BY "@"
+       LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") %s\n";

Please revert this change.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
> 
> Please revert this change.

Well, that "get_kernel_version" is definitely buggered, and should be 
fixed. And we do want the new behaviour for /proc/version.

So I don't think we should revert it, but we should:

 - use separate strings for /proc/version and the static string. Because 
   there just isn't any point to sharing it that much, and the static 
   string might as well be made into __initdata, so you don't even lose 
   the 20-odd bytes of memory at run-time ;)

 - strongly encourage "get_kernel_version" users to just stop using that 
   crap. Ask the build system for the version instead or something, don't 
   expect to dig it out of the binary (if you create an RPM for any other 
   package, you sure as _hell_ don't start doing strings on the binary and 
   try to figure out what the kernel is - you do it as part of the build)

What crud. I'm even slightly inclined to just let SLES9 be broken, just to 
let people know how unacceptable it is to look for strings in kernel 
binaries. But sadly, I don't think the poor users should be penalized for 
some idiotic SLES developers bad taste.

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Linus Torvalds wrote:
> 
> What crud. I'm even slightly inclined to just let SLES9 be broken, just to 
> let people know how unacceptable it is to look for strings in kernel 
> binaries. But sadly, I don't think the poor users should be penalized for 
> some idiotic SLES developers bad taste.

Does this fix the problem?

			Linus

----
diff --git a/fs/proc/proc_misc.c b/fs/proc/proc_misc.c
index dc3e580..6a56c4f 100644
--- a/fs/proc/proc_misc.c
+++ b/fs/proc/proc_misc.c
@@ -47,6 +47,7 @@
 #include <linux/vmalloc.h>
 #include <linux/crash_dump.h>
 #include <linux/pid_namespace.h>
+#include <linux/compile.h>
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
 #include <asm/io.h>
@@ -253,7 +254,12 @@ static int version_read_proc(char *page, char **start, off_t off,
 {
 	int len;
 
-	len = sprintf(page, linux_banner,
+	/* FIXED STRING! Don't touch! */
+	len = snprintf(page, PAGE_SIZE,
+		"Linux version %s"
+		" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
+		" (" LINUX_COMPILER ")"
+		" %s\n",
 		utsname()->release, utsname()->version);
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e8bfac3..b0c4a05 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -17,8 +17,6 @@
 #include <asm/byteorder.h>
 #include <asm/bug.h>
 
-extern const char linux_banner[];
-
 #define INT_MAX		((int)(~0U>>1))
 #define INT_MIN		(-INT_MAX - 1)
 #define UINT_MAX	(~0U)
diff --git a/init/main.c b/init/main.c
index 036f97c..c783695 100644
--- a/init/main.c
+++ b/init/main.c
@@ -483,6 +483,12 @@ void __init __attribute__((weak)) smp_setup_processor_id(void)
 {
 }
 
+static char __initdata linux_banner[] =
+	"Linux version " UTS_RELEASE
+	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
+	" (" LINUX_COMPILER ")"
+	" " UTS_VERSION "\n";
+
 asmlinkage void __init start_kernel(void)
 {
 	char * command_line;
@@ -509,7 +515,7 @@ asmlinkage void __init start_kernel(void)
 	boot_cpu_init();
 	page_address_init();
 	printk(KERN_NOTICE);
-	printk(linux_banner, UTS_RELEASE, UTS_VERSION);
+	printk(linux_banner);
 	setup_arch(&command_line);
 	unwind_setup();
 	setup_per_cpu_areas();
diff --git a/init/version.c b/init/version.c
index 2a5dfcd..9d96d36 100644
--- a/init/version.c
+++ b/init/version.c
@@ -33,8 +33,3 @@ struct uts_namespace init_uts_ns = {
 	},
 };
 EXPORT_SYMBOL_GPL(init_uts_ns);
-
-const char linux_banner[] =
-	"Linux version %s (" LINUX_COMPILE_BY "@"
-	LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") %s\n";
-

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Linus Torvalds wrote:

> +static char __initdata linux_banner[] =
> +	"Linux version " UTS_RELEASE
> +	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
> +	" (" LINUX_COMPILER ")"
> +	" " UTS_VERSION "\n";

main.o gets linked after misc.o, so this will not work. Having both as
globals in the same c file in the right order, that will probably fix
it. Let my try.

strings ../O-maple_defconfig/vmlinux | grep -w Linux
Starting Linux PPC64 %s
Linux
Linux version %s (olaf@g5) (gcc version 4.1.0 (SUSE Linux)) %s
<6>%s: device enabled (Linux)
Linux version 2.6.19-g9202f325-dirty (olaf@g5) (gcc version 4.1.0 (SUSE Linux)) #3 SMP Mon Dec 11 18:59:15 CET 2006
Linux
Linux
Linux

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Olaf Hering wrote:

> On Mon, Dec 11, Linus Torvalds wrote:
> 
> > +static char __initdata linux_banner[] =
> > +	"Linux version " UTS_RELEASE
> > +	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
> > +	" (" LINUX_COMPILER ")"
> > +	" " UTS_VERSION "\n";
> 
> main.o gets linked after misc.o, so this will not work. Having both as
> globals in the same c file in the right order, that will probably fix
> it. Let my try.

Hmm, even moving this to linux_banner doesnt work, just because
__initdata is in a different section.
Only 'const char static_linux_banner' works.
Maybe the guy who did it back in Summer 2000 should have asked for a standard?!


+++ linux-2.6/init/version.c
@@ -34,6 +34,13 @@ struct uts_namespace init_uts_ns = {
 };
 EXPORT_SYMBOL_GPL(init_uts_ns);
 
+/* keep this static string before linux_banner */
+char __initdata static_linux_banner[] =
+       "Linux version " UTS_RELEASE
+       " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
+       " (" LINUX_COMPILER ")"
+       " " UTS_VERSION "\n";
+
 const char linux_banner[] =
        "Linux version %s (" LINUX_COMPILE_BY "@"
        LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") %s\n";

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
> 
> Hmm, even moving this to linux_banner doesnt work, just because
> __initdata is in a different section.

Heh. Let's just change the "version_read_proc" string to not trigger.

Something like this instead (which replaces the "Linux" with "%s" in 
/proc, and just takes it from "utsname()->sysname" instead. So now you can 
lie and call yourself "OS X" in your virtual partition if you want to ;)

		Linus

diff --git a/fs/proc/proc_misc.c b/fs/proc/proc_misc.c
index dc3e580..92ea774 100644
--- a/fs/proc/proc_misc.c
+++ b/fs/proc/proc_misc.c
@@ -47,6 +47,7 @@
 #include <linux/vmalloc.h>
 #include <linux/crash_dump.h>
 #include <linux/pid_namespace.h>
+#include <linux/compile.h>
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
 #include <asm/io.h>
@@ -253,8 +254,15 @@ static int version_read_proc(char *page, char **start, off_t off,
 {
 	int len;
 
-	len = sprintf(page, linux_banner,
-		utsname()->release, utsname()->version);
+	/* FIXED STRING! Don't touch! */
+	len = snprintf(page, PAGE_SIZE,
+		"%s version %s"
+		" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
+		" (" LINUX_COMPILER ")"
+		" %s\n",
+		utsname()->sysname,
+		utsname()->release,
+		utsname()->version);
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
 
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e8bfac3..b0c4a05 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -17,8 +17,6 @@
 #include <asm/byteorder.h>
 #include <asm/bug.h>
 
-extern const char linux_banner[];
-
 #define INT_MAX		((int)(~0U>>1))
 #define INT_MIN		(-INT_MAX - 1)
 #define UINT_MAX	(~0U)
diff --git a/init/main.c b/init/main.c
index 036f97c..c783695 100644
--- a/init/main.c
+++ b/init/main.c
@@ -483,6 +483,12 @@ void __init __attribute__((weak)) smp_setup_processor_id(void)
 {
 }
 
+static char __initdata linux_banner[] =
+	"Linux version " UTS_RELEASE
+	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
+	" (" LINUX_COMPILER ")"
+	" " UTS_VERSION "\n";
+
 asmlinkage void __init start_kernel(void)
 {
 	char * command_line;
@@ -509,7 +515,7 @@ asmlinkage void __init start_kernel(void)
 	boot_cpu_init();
 	page_address_init();
 	printk(KERN_NOTICE);
-	printk(linux_banner, UTS_RELEASE, UTS_VERSION);
+	printk(linux_banner);
 	setup_arch(&command_line);
 	unwind_setup();
 	setup_per_cpu_areas();
diff --git a/init/version.c b/init/version.c
index 2a5dfcd..9d96d36 100644
--- a/init/version.c
+++ b/init/version.c
@@ -33,8 +33,3 @@ struct uts_namespace init_uts_ns = {
 	},
 };
 EXPORT_SYMBOL_GPL(init_uts_ns);
-
-const char linux_banner[] =
-	"Linux version %s (" LINUX_COMPILE_BY "@"
-	LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") %s\n";
-

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Herbert Poetzl]

On Mon, Dec 11, 2006 at 10:26:13AM -0800, Linus Torvalds wrote:
> 
> 
> On Mon, 11 Dec 2006, Olaf Hering wrote:
> > 
> > Hmm, even moving this to linux_banner doesnt work, just because
> > __initdata is in a different section.
> 
> Heh. Let's just change the "version_read_proc" string to not trigger.
> 
> Something like this instead (which replaces the "Linux" with "%s" in 
> /proc, and just takes it from "utsname()->sysname" instead. So now you can 
> lie and call yourself "OS X" in your virtual partition if you want to ;)

cool!

should definitely work for all 'known' cases

thanks,
Herbert

> 		Linus
> 
> diff --git a/fs/proc/proc_misc.c b/fs/proc/proc_misc.c
> index dc3e580..92ea774 100644
> --- a/fs/proc/proc_misc.c
> +++ b/fs/proc/proc_misc.c
> @@ -47,6 +47,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/crash_dump.h>
>  #include <linux/pid_namespace.h>
> +#include <linux/compile.h>
>  #include <asm/uaccess.h>
>  #include <asm/pgtable.h>
>  #include <asm/io.h>
> @@ -253,8 +254,15 @@ static int version_read_proc(char *page, char **start, off_t off,
>  {
>  	int len;
>  
> -	len = sprintf(page, linux_banner,
> -		utsname()->release, utsname()->version);
> +	/* FIXED STRING! Don't touch! */
> +	len = snprintf(page, PAGE_SIZE,
> +		"%s version %s"
> +		" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
> +		" (" LINUX_COMPILER ")"
> +		" %s\n",
> +		utsname()->sysname,
> +		utsname()->release,
> +		utsname()->version);
>  	return proc_calc_metrics(page, start, off, count, eof, len);
>  }
>  
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index e8bfac3..b0c4a05 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -17,8 +17,6 @@
>  #include <asm/byteorder.h>
>  #include <asm/bug.h>
>  
> -extern const char linux_banner[];
> -
>  #define INT_MAX		((int)(~0U>>1))
>  #define INT_MIN		(-INT_MAX - 1)
>  #define UINT_MAX	(~0U)
> diff --git a/init/main.c b/init/main.c
> index 036f97c..c783695 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -483,6 +483,12 @@ void __init __attribute__((weak)) smp_setup_processor_id(void)
>  {
>  }
>  
> +static char __initdata linux_banner[] =
> +	"Linux version " UTS_RELEASE
> +	" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
> +	" (" LINUX_COMPILER ")"
> +	" " UTS_VERSION "\n";
> +
>  asmlinkage void __init start_kernel(void)
>  {
>  	char * command_line;
> @@ -509,7 +515,7 @@ asmlinkage void __init start_kernel(void)
>  	boot_cpu_init();
>  	page_address_init();
>  	printk(KERN_NOTICE);
> -	printk(linux_banner, UTS_RELEASE, UTS_VERSION);
> +	printk(linux_banner);
>  	setup_arch(&command_line);
>  	unwind_setup();
>  	setup_per_cpu_areas();
> diff --git a/init/version.c b/init/version.c
> index 2a5dfcd..9d96d36 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -33,8 +33,3 @@ struct uts_namespace init_uts_ns = {
>  	},
>  };
>  EXPORT_SYMBOL_GPL(init_uts_ns);
> -
> -const char linux_banner[] =
> -	"Linux version %s (" LINUX_COMPILE_BY "@"
> -	LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") %s\n";
> -

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Herbert Poetzl wrote:
> 
> cool!
> 
> should definitely work for all 'known' cases

No it doesn't.

Do a

	git grep '".*Linux version .*"'

on the kernel, and see just how CRAP that "get_kernel_version" test is, 
and has always been.

But let's hope that CIFS is never compiled into a SLES kernel. Because 
this isn't worth fixing at that point, and the SLES people should just fix 
their piece of crap initrd script.

And next time somebody says "random vmlinux binary" to me, I'll blacklist 
their email address. You shouldn't do initrd for "random binaries". Just 
pass the release name somewhere (maybe in the name of the binary, for 
example, and if the name doesn't have a version in it, tough titties).

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Linus Torvalds wrote:

> Do a
> 
> 	git grep '".*Linux version .*"'
> 
> on the kernel, and see just how CRAP that "get_kernel_version" test is, 
> and has always been.
> 
> But let's hope that CIFS is never compiled into a SLES kernel. Because 
> this isn't worth fixing at that point, and the SLES people should just fix 
> their piece of crap initrd script.

arch/powerpc/boot/wrapper:156:    version=`${CROSS}strings "$kernel" | grep '^Linux version [-0-9.]' | \

make $uboot appears to broken as well. I dont have a mkimage here,
so I cant say what it expects. Maybe the wrapper script can use 
'make kernelrelease'

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
> 
> arch/powerpc/boot/wrapper:156:    version=`${CROSS}strings "$kernel" | grep '^Linux version [-0-9.]' | \

This is also obviously broken (and really sad), but actually ends up being 
better than what get_kernel_version apparently does, by at least adding 
the requirement that the string "Linux version" be slightly more correct.

However, it's also TOTALLY BROKEN. 

For example, if the Linux version string happens to be preceded by a byte 
that is a valid ASCII character, the grep will fail miserably. So that PoS 
is actually going to fail for various random kernels too, and depends 
intimately on just what variable _happened_ to be linked just before the 
version string.

The fact is, doing strings on the kernel is just not a viable alternative 
to real versioning.

			Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Paul Mackerras]

Linus Torvalds writes:

> On Mon, 11 Dec 2006, Olaf Hering wrote:
> > 
> > arch/powerpc/boot/wrapper:156:    version=`${CROSS}strings "$kernel" | grep '^Linux version [-0-9.]' | \
> 
> This is also obviously broken (and really sad), but actually ends up being 
> better than what get_kernel_version apparently does, by at least adding 
> the requirement that the string "Linux version" be slightly more correct.
> 
> However, it's also TOTALLY BROKEN. 

It's the minimum effort for the barely acceptable outcome. :)

The wrapper script, although it currently lives in arch/powerpc/boot,
is designed and intended to be standalone, so that people can use it
outside the kernel tree, and possibly even without having the kernel
source easily to hand.  Therefore I didn't want to use any kernel
header files.

Apparently the only reason "mkimage" wants to know the kernel version
is to put it as a comment in the image, which can be displayed to the
user when booting with u-boot (the bootloader used on some embedded
platforms).  So it's not critical if the grep fails, and it's slightly
more useful to do the grep than it would be to not even try to provide
any version string to mkimage.

If there is a reliable way to get the version string, great, I'll use
that.

Paul.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[David Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Tue, 12 Dec 2006 09:04:41 +1100

> If there is a reliable way to get the version string, great, I'll use
> that.

FWIW, on sparc and sparc64 we have this information block for
the boot loader.

The first two instructions at the entry point simply branch
over the boot loader information block header.

The information block starts with a known magic string "HdrS" which
does not match any valid Sparc instruction.  Any tool can search for
it starting at the symbol "_start" in the kernel image.

Inside this information block we stick a 32-bit word which contains
LINUX_VERSION_CODE.

That only gives you the version, not the whole version string, but you
could put the whole string in such a location when adding such a
facility to powerpc if you wanted to.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Gerd Hoffmann]

  Hi,

> That only gives you the version, not the whole version string, but you
> could put the whole string in such a location when adding such a
> facility to powerpc if you wanted to.

Hmm, as we have those fancy ELFNOTE macros now, can't we just the
version string into one?

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Andy Whitcroft]

Linus Torvalds wrote:
> 
> On Mon, 11 Dec 2006, Herbert Poetzl wrote:
>> cool!
>>
>> should definitely work for all 'known' cases
> 
> No it doesn't.
> 
> Do a
> 
> 	git grep '".*Linux version .*"'
> 
> on the kernel, and see just how CRAP that "get_kernel_version" test is, 
> and has always been.
> 
> But let's hope that CIFS is never compiled into a SLES kernel. Because 
> this isn't worth fixing at that point, and the SLES people should just fix 
> their piece of crap initrd script.
> 
> And next time somebody says "random vmlinux binary" to me, I'll blacklist 
> their email address. You shouldn't do initrd for "random binaries". Just 
> pass the release name somewhere (maybe in the name of the binary, for 
> example, and if the name doesn't have a version in it, tough titties).
> 
> 		Linus
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

I am afraid to report that this second version also fails for me, as you 
point out CIFS can break us if defined.  In fact we used to get away 
with this on my test system due to ordering magic luck, I presume the 
move to __initdata has triggered this.  Much as I agree that this is 
wrong we are still going to break people with this.

Before:

Module list:	sym53c8xx reiserfs
Kernel version:	2.6.19-git12-autokern1 (powerpc)
Kernel image:	/boot/vmlinuz-autobench
Initrd image:	/boot/initrd-autobench.img.new
Shared libs:	lib/ld-2.3.3.so lib/libc.so.6 lib/libselinux.so.1
Cannot determine dependencies of module sym53c8xx. Is modules.dep up to 
date?
Modules:	
none
5735 blocks

After:

Module list:	sym53c8xx reiserfs
Kernel version:	 (powerpc)
Kernel image:	/boot/vmlinuz-autobench
Initrd image:	/boot/initrd-autobench.img.new
No modules found for kernel

-apw

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Andy Whitcroft wrote:
> 
> I am afraid to report that this second version also fails for me, as you point
> out CIFS can break us if defined.

Olaf, will you admit that the SLES9 code is crap now?

Andy, does just replacing the "__initdata" with "const" fix it for you? 
That should hopefully mean that IN PRACTICE the Linux version string will 
be the first one to be triggered, if only because init/main.c is linked 
reasonably early, and all the other "Linux version" strings will hopefully 
be in the same rodata section.

Sad, sad. We shouldn't need to work around tools that are so _obviously_ 
broken like this.

			Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Andy Whitcroft]

Linus Torvalds wrote:
> 
> On Mon, 11 Dec 2006, Andy Whitcroft wrote:
>> I am afraid to report that this second version also fails for me, as you point
>> out CIFS can break us if defined.
> 
> Olaf, will you admit that the SLES9 code is crap now?
> 
> Andy, does just replacing the "__initdata" with "const" fix it for you? 
> That should hopefully mean that IN PRACTICE the Linux version string will 
> be the first one to be triggered, if only because init/main.c is linked 
> reasonably early, and all the other "Linux version" strings will hopefully 
> be in the same rodata section.

Yes that does make things 'work' again.  This all seems pretty fragile :(.

> 
> Sad, sad. We shouldn't need to work around tools that are so _obviously_ 
> broken like this.

-apw

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Herbert Poetzl]

On Mon, Dec 11, 2006 at 07:20:57PM +0000, Andy Whitcroft wrote:
> Linus Torvalds wrote:
> >
> >On Mon, 11 Dec 2006, Herbert Poetzl wrote:
> >>cool!
> >>
> >>should definitely work for all 'known' cases
> >
> >No it doesn't.

well, the 'method' not the actual patch, i.e.
you should be as lucky as before, if the banner
string is not touched at all, and the version
entry does duplicate the parts ...

> >Do a
> >
> >	git grep '".*Linux version .*"'
> >
> >on the kernel, and see just how CRAP that "get_kernel_version" test is, 
> >and has always been.
> >
> >But let's hope that CIFS is never compiled into a SLES kernel. Because 
> >this isn't worth fixing at that point, and the SLES people should just fix 
> >their piece of crap initrd script.
> >
> >And next time somebody says "random vmlinux binary" to me, I'll blacklist 
> >their email address. You shouldn't do initrd for "random binaries". Just 
> >pass the release name somewhere (maybe in the name of the binary, for 
> >example, and if the name doesn't have a version in it, tough titties).
> >
> >		Linus
> >-
> >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >Please read the FAQ at  http://www.tux.org/lkml/
> 
> I am afraid to report that this second version also fails for me, as you 
> point out CIFS can break us if defined.  In fact we used to get away 
> with this on my test system due to ordering magic luck, I presume the 
> move to __initdata has triggered this.  Much as I agree that this is 
> wrong we are still going to break people with this.

maybe it would make sense (in SLES) to have that
a special elf section, which is carefully added
to the beginning of the compiled kernel, right
after what is left of the initialization/boot code?

not sure that is mainline stuff though ...

best,
Herbert

> Before:
> 
> Module list:	sym53c8xx reiserfs
> Kernel version:	2.6.19-git12-autokern1 (powerpc)
> Kernel image:	/boot/vmlinuz-autobench
> Initrd image:	/boot/initrd-autobench.img.new
> Shared libs:	lib/ld-2.3.3.so lib/libc.so.6 lib/libselinux.so.1
> Cannot determine dependencies of module sym53c8xx. Is modules.dep up to 
> date?
> Modules:	
> none
> 5735 blocks
> 
> After:
> 
> Module list:	sym53c8xx reiserfs
> Kernel version:	 (powerpc)
> Kernel image:	/boot/vmlinuz-autobench
> Initrd image:	/boot/initrd-autobench.img.new
> No modules found for kernel
> 
> -apw

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Andy Whitcroft wrote:

> I am afraid to report that this second version also fails for me, as you 
> point out CIFS can break us if defined.  In fact we used to get away 
> with this on my test system due to ordering magic luck, I presume the 
> move to __initdata has triggered this.  Much as I agree that this is 
> wrong we are still going to break people with this.

I'm looking at cifs_strtoUCS and wonder if its safe to check 'len &&
*from'. IF it really is, the functions could snprintf to the stack and
pass this to cifs_strtoUCS.

Quick, compile tested, patch below.


Index: linux-2.6/fs/cifs/connect.c
===================================================================
--- linux-2.6.orig/fs/cifs/connect.c
+++ linux-2.6/fs/cifs/connect.c
@@ -2070,6 +2070,7 @@ CIFSSessSetup(unsigned int xid, struct c
 	      char session_key[CIFS_SESS_KEY_SIZE],
 	      const struct nls_table *nls_codepage)
 {
+	char banner[2*32+1];
 	struct smb_hdr *smb_buffer;
 	struct smb_hdr *smb_buffer_response;
 	SESSION_SETUP_ANDX *pSMB;
@@ -2135,6 +2136,8 @@ CIFSSessSetup(unsigned int xid, struct c
 	memcpy(bcc_ptr, (char *) session_key, CIFS_SESS_KEY_SIZE);
 	bcc_ptr += CIFS_SESS_KEY_SIZE;
 
+	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
+		utsname()->release);
 	if (ses->capabilities & CAP_UNICODE) {
 		if ((long) bcc_ptr % 2) { /* must be word aligned for Unicode */
 			*bcc_ptr = 0;
@@ -2160,12 +2163,8 @@ CIFSSessSetup(unsigned int xid, struct c
 		bcc_ptr += 2 * bytes_returned;
 		bcc_ptr += 2;
 		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, "Linux version ",
-				  32, nls_codepage);
-		bcc_ptr += 2 * bytes_returned;
-		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, utsname()->release,
-				  32, nls_codepage);
+		    cifs_strtoUCS((__le16 *) bcc_ptr, banner,
+				  64, nls_codepage);
 		bcc_ptr += 2 * bytes_returned;
 		bcc_ptr += 2;
 		bytes_returned =
@@ -2189,10 +2188,8 @@ CIFSSessSetup(unsigned int xid, struct c
 			*bcc_ptr = 0;
 			bcc_ptr++;
 		}
-		strcpy(bcc_ptr, "Linux version ");
-		bcc_ptr += strlen("Linux version ");
-		strcpy(bcc_ptr, utsname()->release);
-		bcc_ptr += strlen(utsname()->release) + 1;
+		strcpy(bcc_ptr, banner);
+		bcc_ptr += strlen(banner) + 1;
 		strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
 		bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;
 	}
@@ -2360,6 +2357,7 @@ CIFSNTLMSSPNegotiateSessSetup(unsigned i
 			      struct cifsSesInfo *ses, int * pNTLMv2_flag,
 			      const struct nls_table *nls_codepage)
 {
+	char banner[2*32+1];
 	struct smb_hdr *smb_buffer;
 	struct smb_hdr *smb_buffer_response;
 	SESSION_SETUP_ANDX *pSMB;
@@ -2445,6 +2443,8 @@ CIFSNTLMSSPNegotiateSessSetup(unsigned i
 	SecurityBlob->DomainName.Buffer = 0;
 	SecurityBlob->DomainName.Length = 0;
 	SecurityBlob->DomainName.MaximumLength = 0;
+	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
+		utsname()->release);
 	if (ses->capabilities & CAP_UNICODE) {
 		if ((long) bcc_ptr % 2) {
 			*bcc_ptr = 0;
@@ -2452,11 +2452,7 @@ CIFSNTLMSSPNegotiateSessSetup(unsigned i
 		}
 
 		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, "Linux version ",
-				  32, nls_codepage);
-		bcc_ptr += 2 * bytes_returned;
-		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, utsname()->release, 32,
+		    cifs_strtoUCS((__le16 *) bcc_ptr, banner, 64,
 				  nls_codepage);
 		bcc_ptr += 2 * bytes_returned;
 		bcc_ptr += 2;	/* null terminate Linux version */
@@ -2471,10 +2467,8 @@ CIFSNTLMSSPNegotiateSessSetup(unsigned i
 		*(bcc_ptr + 2) = 0;
 		bcc_ptr += 2;	/* null domain */
 	} else {		/* ASCII */
-		strcpy(bcc_ptr, "Linux version ");
-		bcc_ptr += strlen("Linux version ");
-		strcpy(bcc_ptr, utsname()->release);
-		bcc_ptr += strlen(utsname()->release) + 1;
+		strcpy(bcc_ptr, banner);
+		bcc_ptr += strlen(banner) + 1;
 		strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
 		bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;
 		bcc_ptr++;	/* empty domain field */
@@ -2694,6 +2688,7 @@ CIFSNTLMSSPAuthSessSetup(unsigned int xi
 		char *ntlm_session_key, int ntlmv2_flag,
 		const struct nls_table *nls_codepage)
 {
+	char banner[2*32+1];
 	struct smb_hdr *smb_buffer;
 	struct smb_hdr *smb_buffer_response;
 	SESSION_SETUP_ANDX *pSMB;
@@ -2792,6 +2787,8 @@ CIFSNTLMSSPAuthSessSetup(unsigned int xi
 	SecurityBlobLength += CIFS_SESS_KEY_SIZE;
 	bcc_ptr += CIFS_SESS_KEY_SIZE;
 
+	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
+		utsname()->release);
 	if (ses->capabilities & CAP_UNICODE) {
 		if (domain == NULL) {
 			SecurityBlob->DomainName.Buffer = 0;
@@ -2843,11 +2840,7 @@ CIFSNTLMSSPAuthSessSetup(unsigned int xi
 			bcc_ptr++;
 		}
 		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, "Linux version ",
-				  32, nls_codepage);
-		bcc_ptr += 2 * bytes_returned;
-		bytes_returned =
-		    cifs_strtoUCS((__le16 *) bcc_ptr, utsname()->release, 32,
+		    cifs_strtoUCS((__le16 *) bcc_ptr, banner, 64,
 				  nls_codepage);
 		bcc_ptr += 2 * bytes_returned;
 		bcc_ptr += 2;	/* null term version string */
@@ -2897,10 +2890,8 @@ CIFSNTLMSSPAuthSessSetup(unsigned int xi
 		}
 		/* BB fill in our workstation name if known BB */
 
-		strcpy(bcc_ptr, "Linux version ");
-		bcc_ptr += strlen("Linux version ");
-		strcpy(bcc_ptr, utsname()->release);
-		bcc_ptr += strlen(utsname()->release) + 1;
+		strcpy(bcc_ptr, banner);
+		bcc_ptr += strlen(banner) + 1;
 		strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
 		bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;
 		bcc_ptr++;	/* null domain */
Index: linux-2.6/fs/cifs/sess.c
===================================================================
--- linux-2.6.orig/fs/cifs/sess.c
+++ linux-2.6/fs/cifs/sess.c
@@ -77,6 +77,7 @@ static __u32 cifs_ssetup_hdr(struct cifs
 static void unicode_ssetup_strings(char ** pbcc_area, struct cifsSesInfo *ses,
 			    const struct nls_table * nls_cp)
 {
+	char banner[2*32+1];
 	char * bcc_ptr = *pbcc_area;
 	int bytes_ret = 0;
 
@@ -113,12 +114,11 @@ static void unicode_ssetup_strings(char 
 	bcc_ptr += 2;  /* account for null terminator */
 
 	/* Copy OS version */
-	bytes_ret = cifs_strtoUCS((__le16 *)bcc_ptr, "Linux version ", 32,
+	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
+		init_utsname()->release);
+	bytes_ret = cifs_strtoUCS((__le16 *)bcc_ptr, banner, 32,
 				  nls_cp);
 	bcc_ptr += 2 * bytes_ret;
-	bytes_ret = cifs_strtoUCS((__le16 *) bcc_ptr, init_utsname()->release,
-				  32, nls_cp);
-	bcc_ptr += 2 * bytes_ret;
 	bcc_ptr += 2; /* trailing null */
 
 	bytes_ret = cifs_strtoUCS((__le16 *) bcc_ptr, CIFS_NETWORK_OPSYS,
@@ -132,6 +132,7 @@ static void unicode_ssetup_strings(char 
 static void ascii_ssetup_strings(char ** pbcc_area, struct cifsSesInfo *ses,
 			  const struct nls_table * nls_cp)
 {
+	char banner[2*32+1];
 	char * bcc_ptr = *pbcc_area;
 
 	/* copy user */
@@ -159,10 +160,10 @@ static void ascii_ssetup_strings(char **
 
 	/* BB check for overflow here */
 
-	strcpy(bcc_ptr, "Linux version ");
-	bcc_ptr += strlen("Linux version ");
-	strcpy(bcc_ptr, init_utsname()->release);
-	bcc_ptr += strlen(init_utsname()->release) + 1;
+	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
+		init_utsname()->release);
+	strcpy(bcc_ptr, banner);
+	bcc_ptr += strlen(banner) + 1;
 
 	strcpy(bcc_ptr, CIFS_NETWORK_OPSYS);
 	bcc_ptr += strlen(CIFS_NETWORK_OPSYS) + 1;

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
> 
> Quick, compile tested, patch below.

No. We don't do this. We don't add TOTAL CRAP to the kernel just because 
somebody is being an idiot in user space.

This is definitely a user space bug. It was a serious bug before, it just 
wasn't obvious.

The thing is, if anybody runs SLES, they expect support. That's what the 
"E" in "Enterprise" means.

So I would suggest SLES now show that support by fixing THEIR BUG.

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Linus Torvalds wrote:
> 
> So I would suggest SLES now show that support by fixing THEIR BUG.

Btw, if you still want to use "get_kernel_version" or whatever the broken 
program was, I'd suggest:

 - extend the check to verify that the version number that follows looks 
   valid. It had better be something like a number with dots and perhaps a 
   "v" in front of it or something.

 - extend the check to check that it has parenthesis and a date there 
   somewhere too.

In other words, make the string grep really REALLY anal. Rather than grep 
for something totally trivial like "Linux version " that is so common that 
I could easily see it finding that particular string sequence in any 
random binary, not just the Linux kernel (eg some internal thing that says 
"tested with Linux version 2.6")

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Greg KH]

On Mon, Dec 11, 2006 at 12:05:36PM -0800, Linus Torvalds wrote:
> 
> 
> On Mon, 11 Dec 2006, Olaf Hering wrote:
> > 
> > Quick, compile tested, patch below.
> 
> No. We don't do this. We don't add TOTAL CRAP to the kernel just because 
> somebody is being an idiot in user space.
> 
> This is definitely a user space bug. It was a serious bug before, it just 
> wasn't obvious.
> 
> The thing is, if anybody runs SLES, they expect support. That's what the 
> "E" in "Enterprise" means.
> 
> So I would suggest SLES now show that support by fixing THEIR BUG.

I agree, it's SuSE's bug, not the kernel's issue.

I'll take this off-list with the SuSE developers to get this fixed.

thanks,

greg k-h

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Olaf Hering wrote:

> On Mon, Dec 11, Andy Whitcroft wrote:
> 
> > I am afraid to report that this second version also fails for me, as you 
> > point out CIFS can break us if defined.  In fact we used to get away 
> > with this on my test system due to ordering magic luck, I presume the 
> > move to __initdata has triggered this.  Much as I agree that this is 
> > wrong we are still going to break people with this.
> 
> I'm looking at cifs_strtoUCS and wonder if its safe to check 'len &&
> *from'. IF it really is, the functions could snprintf to the stack and
> pass this to cifs_strtoUCS.
> 
> Quick, compile tested, patch below.
> 
> 
> Index: linux-2.6/fs/cifs/connect.c
> ===================================================================
> --- linux-2.6.orig/fs/cifs/connect.c
> +++ linux-2.6/fs/cifs/connect.c
> @@ -2070,6 +2070,7 @@ CIFSSessSetup(unsigned int xid, struct c
>  	      char session_key[CIFS_SESS_KEY_SIZE],
>  	      const struct nls_table *nls_codepage)
>  {
> +	char banner[2*32+1];
>  	struct smb_hdr *smb_buffer;
>  	struct smb_hdr *smb_buffer_response;
>  	SESSION_SETUP_ANDX *pSMB;
> @@ -2135,6 +2136,8 @@ CIFSSessSetup(unsigned int xid, struct c
>  	memcpy(bcc_ptr, (char *) session_key, CIFS_SESS_KEY_SIZE);
>  	bcc_ptr += CIFS_SESS_KEY_SIZE;
>  
> +	snprintf(banner, sizeof(banner), "%s version %s", utsname()->sysname,
> +		utsname()->release);
>  	if (ses->capabilities & CAP_UNICODE) {
>  		if ((long) bcc_ptr % 2) { /* must be word aligned for Unicode */
>  			*bcc_ptr = 0;
> @@ -2160,12 +2163,8 @@ CIFSSessSetup(unsigned int xid, struct c
>  		bcc_ptr += 2 * bytes_returned;
>  		bcc_ptr += 2;
>  		bytes_returned =
> -		    cifs_strtoUCS((__le16 *) bcc_ptr, "Linux version ",
> -				  32, nls_codepage);
> -		bcc_ptr += 2 * bytes_returned;
> -		bytes_returned =
> -		    cifs_strtoUCS((__le16 *) bcc_ptr, utsname()->release,
> -				  32, nls_codepage);
> +		    cifs_strtoUCS((__le16 *) bcc_ptr, banner,
> +				  64, nls_codepage);
>  		bcc_ptr += 2 * bytes_returned;
>  		bcc_ptr += 2;
>  		bytes_returned =

new_utsname->release is 65 bytes, so with a very long uname -r, the
current code already truncates release.

Steve, is 32 a hard limit in the protocol?

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Theodore Tso]

On Mon, Dec 11, 2006 at 07:20:57PM +0000, Andy Whitcroft wrote:
> I am afraid to report that this second version also fails for me, as you 
> point out CIFS can break us if defined.  In fact we used to get away 
> with this on my test system due to ordering magic luck, I presume the 
> move to __initdata has triggered this.  Much as I agree that this is 
> wrong we are still going to break people with this.

But does your problem go away if you compile CIFS as a module?  If so,
then we're no worse off than before.  Still, whoever wrote the SLES
initrd needs to receive 100 lashes with a wet noodle for not proposing
a more robust solution.  

As far as whether or not it should be _mandatory_, to be able to pull
out the version information from an arbitrary bzImage file, can folks
agree that it would at least be a nice-to-have feature?  Sometimes
when you're out in the field you don't know what you're faced with,
especially if you're dealing with a customer who likes to build their
own kernels, and who might not have, ah, a very well defined release
process.  Sure, you can _call_ them incompetent, and it might even be
true, but wouldn't be nice if there was an easy way to look at a
bzImage file and be able to tell what kernel version it was built
from?

Clearly, if the goal is to make it easy to pull out, it will be
architecture specific, since it depends on the layout of the kernel
image file.  At least for x86 and x86_64, though, there's an obvious
place for it --- in the first 512 bytes of the image, in what was
previously the floppy bootstrap code.  Plenty of space there for a
100-150 bytes worth of version information.

						- Ted

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Arjan van de Ven]

> As far as whether or not it should be _mandatory_, to be able to pull
> out the version information from an arbitrary bzImage file, can folks
> agree that it would at least be a nice-to-have feature?  

I would really like for modinfo to work. it may not work on bzImage as
is, but it should work after gunzipping it... and it's the standard way
of conveying all this kind of info anyway

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[H. Peter Anvin]

Theodore Tso wrote:
> 
> As far as whether or not it should be _mandatory_, to be able to pull
> out the version information from an arbitrary bzImage file, can folks
> agree that it would at least be a nice-to-have feature?  Sometimes
> when you're out in the field you don't know what you're faced with,
> especially if you're dealing with a customer who likes to build their
> own kernels, and who might not have, ah, a very well defined release
> process.  Sure, you can _call_ them incompetent, and it might even be
> true, but wouldn't be nice if there was an easy way to look at a
> bzImage file and be able to tell what kernel version it was built
> from?
> 

There is a documented procedure for doing exactly that.

See Documentation/i386/boot.txt for details; there is a pointer in the 
header which points to a cleartext string, even if the kernel is compressed.

	-hpa

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Linus Torvalds wrote:

> 
> 
> On Mon, 11 Dec 2006, Olaf Hering wrote:
> > 
> > Hmm, even moving this to linux_banner doesnt work, just because
> > __initdata is in a different section.
> 
> Heh. Let's just change the "version_read_proc" string to not trigger.
> 
> Something like this instead (which replaces the "Linux" with "%s" in 
> /proc, and just takes it from "utsname()->sysname" instead. So now you can 
> lie and call yourself "OS X" in your virtual partition if you want to ;)

Yes, that works. Thanks.

And if I'm really bored, I will dig up the reason why the very same
binary has to identify itself with different kernelreleases.

Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Kyle Moffett]

On Dec 11, 2006, at 13:26:13, Linus Torvalds wrote:
> On Mon, 11 Dec 2006, Olaf Hering wrote:
>> Hmm, even moving this to linux_banner doesnt work, just because  
>> __initdata is in a different section.
>
> Heh. Let's just change the "version_read_proc" string to not trigger.
>
> Something like this instead (which replaces the "Linux" with "%s"  
> in /proc, and just takes it from "utsname()->sysname" instead. So  
> now you can lie and call yourself "OS X" in your virtual partition  
> if you want to ;)

Funny you should mention that...  I'm currently tinkering with a  
binfmt_mach_o kernel module and associated Darwin syscall personality  
to try to load and run console Mach-O binaries natively under PPC or  
X86 linux (depending on what architectures the Mach-O binary supports).

I've basically got the binary loader part working; the Mach-O dynamic  
loader gets mapped into memory at the appropriate location, the  
kernel jumps to the specified location in the code... and then the  
program comes crashing to a halt when it starts calling invalid  
syscalls with bogus arguments.

So now I have to figure out how to set up a new syscall personality  
with a bunch of wrapper syscalls which reorder arguments and  
translate constant values before calling into the rest of the Linux  
code.  I'm fairly sure it's possible because you can run some Solaris  
binaries under Linux if you turn on the appropriate BINFMT_* config  
option(s), but I'm totally unsure as to _how_.

I'd much appreciate any advice you can give!

Cheers,
Kyle Moffett

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Linus Torvalds]

On Tue, 12 Dec 2006, Kyle Moffett wrote:
> 
> So now I have to figure out how to set up a new syscall personality with a
> bunch of wrapper syscalls which reorder arguments and translate constant
> values before calling into the rest of the Linux code.  I'm fairly sure it's
> possible because you can run some Solaris binaries under Linux if you turn on
> the appropriate BINFMT_* config option(s), but I'm totally unsure as to _how_.

What system call interface do Mach-O binaries use? Is it the old stupid 
"lcall 7,0" thing, or does it use "sysenter" or something like that?

If it's sysenter, it's going to be "interesting". That code currently 
doesn't support any kind of emulation, and the whole "sysenter" interface 
is pretty grotty at a CPU level (it doesn't even save eip etc). So you'd 
need to delve into x86 asm and arch/i386/kernel/entry.S (or the x86-64 
equivalent).

		Linus

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Kyle Moffett]

NOTE: If at all possible I'd like to keep the userspace stuff is set  
in stone; I want to just load some Mach-O binaries, libraries and  
dynamic linker onto the Linux system, "chroot /darwin" and go.  There  
may very well be huge game-over show-stoppers, but I might as well  
try, dammit! :-D

On Dec 12, 2006, at 11:23:58, Linus Torvalds wrote:
> On Tue, 12 Dec 2006, Kyle Moffett wrote:
>> So now I have to figure out how to set up a new syscall  
>> personality with a bunch of wrapper syscalls which reorder  
>> arguments and translate constant values before calling into the  
>> rest of the Linux code.  I'm fairly sure it's possible because you  
>> can run some Solaris binaries under Linux if you turn on the  
>> appropriate BINFMT_* config option(s), but I'm totally unsure as  
>> to _how_.
>
> What system call interface do Mach-O binaries use? Is it the old  
> stupid  "lcall 7,0" thing, or does it use "sysenter" or something  
> like that?
>
> If it's sysenter, it's going to be "interesting". That code  
> currently doesn't support any kind of emulation, and the whole  
> "sysenter" interface is pretty grotty at a CPU level (it doesn't  
> even save EIP etc). So you'd need to delve into x86 asm and arch/ 
> i386/kernel/entry.S (or the x86-64 equivalent).

Virtually all of my easily accessible computers right now are PowerPC  
and all of my assembly experience is PPC and MIPS, so as far as the  
x86-syscall support I have no clue whatsoever.  I hadn't even really  
considered it till you mentioned it.  I might get around to X86  
support at some point in the future assuming I get the PPC side to  
work, or I might just leave that for some enterprising person with a  
hell of a lot better understanding of X86 assembly fundamentals.

PPC seems to be a lot more straightforward and constrained than  
the... umm... "eccentric" privilege rings that x86 has (which I only  
minimally grasp).  I've only really studied RISC architectures in any  
detail.

With a little bit of Google and a lot of greping darwin sources I've  
been able to puzzle out that they don't use slow call gates and that  
they probably use sysenter/sysexit (Or possibly syscall/sysret if  
it's available?  I'm way over my head as far as x86 assembly and  
architecture goes)

The PPC syscall stuff on the other hand is fairly straightforward.   
The code loads the argument registers (which I _think_ follow the  
same syscall ABI on Linux and Darwin due to somebody having a flash  
of inspiration and putting that recommendation in the PPC spec  
documents) and runs the sc instruction.  Kernel code takes over,  
saves some state, loads some state, bumbles around with the argument  
registers a bit and looks up the syscall function in the syscall  
table (marked "asmlinkage"? what does that do?) and wanders off into  
the per-syscall handling code.

 From what I understand (for PPC at least), from the "sc" instruction  
till we actually call the in-kernel syscall-specific handler function  
the code is effectively the same.  We save the same state, set up  
similar kernel state, etc, so that the customary kernel services are  
available as soon as our syscall-handler-function starts.

So I guess all I have to do is:
   (A)  Write a bunch of new syscall handlers taking arguments of the  
same types as the Darwin syscall handlers,
   (B)  Figure out how to switch tables depending on the "syscall  
personality" of "current"
   (C)  Figure out how to set the "syscall personality" of "current"  
from my Mach-O binary format module.

(A) seems fairly straightforward, if unusually tedious and error- 
prone, but I'm totally in the dark for (B) and (C).  Any help would  
be much appreciated.

Cheers,
Kyle Moffett

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Linus Torvalds]

On Tue, 12 Dec 2006, Kyle Moffett wrote:
> 
> Virtually all of my easily accessible computers right now are PowerPC and all
> of my assembly experience is PPC and MIPS, so as far as the x86-syscall
> support I have no clue whatsoever.

Ok. On ppc, the issues are perhaps a bit more straightforward, because 
there is really just one way to do system calls: "sc".

That said, powerpc simply doesn't historically do any system call 
translation, so you'll just have to implement the same kind of translation 
layer that sparc has done, for example.

See: DoSyscall in arch/powerpc/kernel/entry_32.S. Right now it just does

		...
		cmplwi  0,r0,NR_syscalls
	        lis     r10,sys_call_table@h
	        ori     r10,r10,sys_call_table@l
	        slwi    r0,r0,2
	        bge-    66f
	        lwzx    r10,r10,r0      /* Fetch system call handler [ptr] */
	        mtlr    r10
	        addi    r9,r1,STACK_FRAME_OVERHEAD
	        PPC440EP_ERR42
	        blrl                    /* Call handler */
	        .globl  ret_from_syscall
	ret_from_syscall:
		...

ie it uses one global table ("sys_call_table" and "NR_syscalls") for 
everything, and you'd need to make it use different tables (and table 
sizes) conditionally on the "Apple binary flag"

Alternatively, you could perhaps fit it in the exception table itself 
(arch/powerpc/kernel/entry.S), but that gets just nastier and more 
low-level, so I doubt it's all that great an idea.

> So I guess all I have to do is:
>  (A)  Write a bunch of new syscall handlers taking arguments of the same types
> as the Darwin syscall handlers,

Yes. The big issue tends to be to translate all the errno's and the fcntl 
structure pointers etc. THAT can be quite painful indeed. You might ask 
David Miller and company about their SunOS stuff, and look at things like

	arch/sparc/kernel/{sys_sunos.c,sunos_ioctl.c}

for some sorry examples.

>  (B)  Figure out how to switch tables depending on the "syscall personality"
> of "current"

See above. "DoSyscall" is where you enter.

		Linus

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Kyle Moffett]

On Dec 12, 2006, at 13:20:19, Linus Torvalds wrote:
> That said, powerpc simply doesn't historically do any system call  
> translation, so you'll just have to implement the same kind of  
> translation layer that sparc has done, for example.

Thanks a lot for all your help.  I've got two last questions:  From  
the code in entry_32.s I can dig up "current" from ((struct  
paca_struct *)r13)->__current to read a personality flag from it,  
right?  Digging up offsets in assembly can't be very fun :-\   
Secondly, is there a preferred existing field into which I should  
stick said flag or just stuff it somewhere?

This part seems like the easiest so far; no icky binary format  
parsing, no confusing memory maps.  Thanks once again!

>> So I guess all I have to do is:
>>  (A)  Write a bunch of new syscall handlers taking arguments of  
>> the same types
>> as the Darwin syscall handlers,
>
> Yes. The big issue tends to be to translate all the errno's and the  
> fcntl structure pointers etc. THAT can be quite painful indeed. You  
> might ask David Miller and company about their SunOS stuff, and  
> look at things like
>
> 	arch/sparc/kernel/{sys_sunos.c,sunos_ioctl.c}
>
> for some sorry examples.

Ok, I figured it was going to be ugly; maybe not quite _that_ ugly  
but my hopes weren't high enough for you to dash to any real degree :-D.

Cheers,
Kyle Moffett

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Benjamin Herrenschmidt]

On Tue, 2006-12-12 at 17:34 -0500, Kyle Moffett wrote:
> On Dec 12, 2006, at 13:20:19, Linus Torvalds wrote:
> > That said, powerpc simply doesn't historically do any system call  
> > translation, so you'll just have to implement the same kind of  
> > translation layer that sparc has done, for example.
> 
> Thanks a lot for all your help.  I've got two last questions:  From  
> the code in entry_32.s I can dig up "current" from ((struct  
> paca_struct *)r13)->__current to read a personality flag from it,  
> right?  Digging up offsets in assembly can't be very fun :-\   

That's what asm-offset.c is for :-) It generates all the offsets you
need. In general though, you probably want to copy your personality flag
to thread_info, along with other bits in there (like 32 vs. 64 bits).
Look how it's done on ppc64.

> Secondly, is there a preferred existing field into which I should  
> stick said flag or just stuff it somewhere?

Yes, thread_info->flags.

> Ok, I figured it was going to be ugly; maybe not quite _that_ ugly  
> but my hopes weren't high enough for you to dash to any real degree :-D.

Well... it can't be pretty :-)

Ben.

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Linus Torvalds]

On Wed, 13 Dec 2006, Benjamin Herrenschmidt wrote:
> 
> > Secondly, is there a preferred existing field into which I should  
> > stick said flag or just stuff it somewhere?
> 
> Yes, thread_info->flags.

Well, it may actually make sense to actually stick the whole "syscall 
table pointer" in there, rather than a flag that says which pointer to 
choose.

We already load the thread_info pointer because we need the flags for 
syscall tracing, and since we have the thread_info pointer, it might be 
easier to load the syscall table pointer right off there, rather than 
loading it as a big constant with "lis + ori" (in fact, on ppc64, we 
currently load it off the TOC, which is really sad, since we already 
brought in the thread_info into the cache, and usign the TOC is not just a 
load, it's a load off a separate cacheline).

So on 64-bit ppc, it could actually speed things up to put the system call 
table pointer into thread_info, and make it more flexible at the same 
time, without any conditional flags.

		Linus

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Benjamin Herrenschmidt]

> The PPC syscall stuff on the other hand is fairly straightforward.   
> The code loads the argument registers (which I _think_ follow the  
> same syscall ABI on Linux and Darwin due to somebody having a flash  
> of inspiration and putting that recommendation in the PPC spec  
> documents) 

I wouldn't bet on that ... they might look the same but it's likely that
there will be subtle differences. There are definitely differences
between the PEF ABI used on MacOS < X (and useable in OS X with a
special loader) and the SysV ABI we use in Linux. The differences
generally are around those areas:

 - stack frame format (hopefully should be irrelevant for syscalls,
well, I hope so ...)
 - va_args format (same)
 - passing or returning function arguments larger than the native int
size (passing 64 bits values, passing structures by values)  (r3/r4 vs.
stack for example).
 - TOC/TLS/whatever is in r2, r12 and r13 ...

I would expect most of these but not all to be irrelevant for syscalls.

Now, I don't know precisely what the mach-o ABI looks like, we might be
lucky and it may be similar to ours. PEF is not, but then, PEF isn't
native in OS-X, they use a special loader/wrapper for it.

Also, beware that there are two different ABIs (both in linux and in
mach-o) for 32 and 64 bits binaries.

Ben.

Re: Mach-O binary format support and Darwin syscall personality [Was: uts banner changes]

[Pavel Machek]

Hi!

> So I guess all I have to do is:
>   (A)  Write a bunch of new syscall handlers taking 
>   arguments of the  same types as the Darwin syscall 
> handlers,
>   (B)  Figure out how to switch tables depending on the 
>   "syscall  personality" of "current"
>   (C)  Figure out how to set the "syscall personality" 
>   of "current"  from my Mach-O binary format module.
> 
> (A) seems fairly straightforward, if unusually tedious 
> and error- prone, but I'm totally in the dark for (B) 
> and (C).  Any help would  be much appreciated.

try strace osx_binary. If syscall interface is similar enough, perhaps
it is possible to do it with ptrace() :-).
							Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Linus Torvalds wrote: 
> What crud. I'm even slightly inclined to just let SLES9 be broken, just to 
> let people know how unacceptable it is to look for strings in kernel 
> binaries. But sadly, I don't think the poor users should be penalized for 
> some idiotic SLES developers bad taste.

SLES7 or SLES11 is not any different than SLES9 in that respect.
Suppose I send you some random vmlinux binary. How do you (you as in linus.sh)
know what 'uname -r' is inside this binary?
There are surely many many ways to pass that info. Having a string like
'Linux version 2.6.19-g9202f325-dirty' somewhere in the binary is the
most reliable one. Dont you agree?
Just think about it for a minute.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Arjan van de Ven]

On Mon, 2006-12-11 at 18:50 +0100, Olaf Hering wrote:
> On Mon, Dec 11, Linus Torvalds wrote: 
> > What crud. I'm even slightly inclined to just let SLES9 be broken, just to 
> > let people know how unacceptable it is to look for strings in kernel 
> > binaries. But sadly, I don't think the poor users should be penalized for 
> > some idiotic SLES developers bad taste.
> 
> SLES7 or SLES11 is not any different than SLES9 in that respect.
> Suppose I send you some random vmlinux binary. How do you (you as in linus.sh)
> know what 'uname -r' is inside this binary?
> There are surely many many ways to pass that info. Having a string like
> 'Linux version 2.6.19-g9202f325-dirty' somewhere in the binary is the
> most reliable one. Dont you agree?

it's for sure the most ugly one. I could see the use of having modinfo
work on the vmlinux, and have the vmlinux have a VERMAGIC as well. It's
only a simple elf section after all, and a heck of a lot more defined
and standard...



-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Arjan van de Ven wrote:

> it's for sure the most ugly one. I could see the use of having modinfo
> work on the vmlinux, and have the vmlinux have a VERMAGIC as well. It's
> only a simple elf section after all, and a heck of a lot more defined
> and standard...

Just go for it. Remember there is also that bzImage thingy, no idea how
its layed out, it has to work there too.

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Arjan van de Ven]

On Mon, 2006-12-11 at 19:00 +0100, Olaf Hering wrote:
> On Mon, Dec 11, Arjan van de Ven wrote:
> 
> > it's for sure the most ugly one. I could see the use of having modinfo
> > work on the vmlinux, and have the vmlinux have a VERMAGIC as well. It's
> > only a simple elf section after all, and a heck of a lot more defined
> > and standard...
> 
> Just go for it. Remember there is also that bzImage thingy, no idea how
> its layed out, it has to work there too.

strings doesn't work there, it's a compressed image!
(well the first few bytes might be readable due to how compression
works)


also... can't you just know which vmlinux it is in the first place?
(or in other words, why is SLES the only one with the problem?)

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Arjan van de Ven wrote:

> strings doesn't work there, it's a compressed image!
Thats why get_kernel_version calls gzip.

> also... can't you just know which vmlinux it is in the first place?

No, you cant.

> (or in other words, why is SLES the only one with the problem?)

Everyone has this "problem". Or how do you know what kernelrelease is
inside a random ELF or bzImage binary?

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Arjan van de Ven]

> 
> > (or in other words, why is SLES the only one with the problem?)
> 
> Everyone has this "problem". Or how do you know what kernelrelease is
> inside a random ELF or bzImage binary?

I doubt anyone else will let it come to the "random" stage


-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Jan Engelhardt]

On Dec 11 2006 19:14, Olaf Hering wrote:
>
>> (or in other words, why is SLES the only one with the problem?)
>
>Everyone has this "problem". Or how do you know what kernelrelease is
>inside a random ELF or bzImage binary?

Why would you even want to know that? (Stirring in the hornets nest,
just add a new mkinitrd option.)


	-`J'
-- 

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
> 
> SLES7 or SLES11 is not any different than SLES9 in that respect.
> Suppose I send you some random vmlinux binary. How do you (you as in linus.sh)
> know what 'uname -r' is inside this binary?
> There are surely many many ways to pass that info. Having a string like
> 'Linux version 2.6.19-g9202f325-dirty' somewhere in the binary is the
> most reliable one. Dont you agree?
> Just think about it for a minute.

YOU just think about it for a minute.

Your basic problem was much earlier:

	"Suppose I send you some random vmlinux binary."

THAT is the problem.

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Olaf Hering]

On Mon, Dec 11, Linus Torvalds wrote:

> 	"Suppose I send you some random vmlinux binary."
> 
> THAT is the problem.

Erm no, thats reality and happens every day. git-bisect a modular kernel
on one box and test it on another. The mkinitrd (and depmod) wants to know
where to look for modules.
Of course I could tell it every time what the kernelrelease is, but why
do I have to?

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Linus Torvalds]

On Mon, 11 Dec 2006, Olaf Hering wrote:
>
> Of course I could tell it every time what the kernelrelease is, but why
> do I have to?

Because right now, YOUR PIECE OF CRAP IS BUGGY.

Look here, I'm not going to bother explain it to you any more. Do the 

	git grep '".*Linux version .*"'

thing, and if you don't understand why your CRAP IS BUGGY, I don't know 
what else I can tell you.

So I'll make it real simple: I hopefully fixed it this time, but the next 
time somebody happens to have a string that contains "Linux version" in 
it, I simply won't bother. It's not a kernel bug. It's a bug in your 
broken and utterly idiotic process. 

End of discussion.

		Linus

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[Jan Engelhardt]

On Dec 11 2006 08:44, Linus Torvalds wrote:
>> 
>> Please revert this change.
>
>Well, that "get_kernel_version" is definitely buggered, and should be 
>fixed. And we do want the new behaviour for /proc/version.
>
>So I don't think we should revert it, but we should:
>
> - strongly encourage "get_kernel_version" users to just stop using that 
>   crap. Ask the build system for the version instead or something, don't 
>   expect to dig it out of the binary (if you create an RPM for any other 
>   package, you sure as _hell_ don't start doing strings on the binary and 
>   try to figure out what the kernel is - you do it as part of the build)
>
>What crud. I'm even slightly inclined to just let SLES9 be broken, just to 
>let people know how unacceptable it is to look for strings in kernel 
>binaries. But sadly, I don't think the poor users should be penalized for 
>some idiotic SLES developers bad taste.


If you fix it now, it will anyhow only be fixed for >= 2.6.20, hence
you don't break current/older SLES kernel builds.


	-`J'
-- 

Re: 2.6.19-git13: uts banner changes break SLES9 (at least)

[H. Peter Anvin]

Linus Torvalds wrote:
> 
>  - strongly encourage "get_kernel_version" users to just stop using that 
>    crap. Ask the build system for the version instead or something, don't 
>    expect to dig it out of the binary (if you create an RPM for any other 
>    package, you sure as _hell_ don't start doing strings on the binary and 
>    try to figure out what the kernel is - you do it as part of the build)
> 

This is (presumably) not just "strings" on the binary -- it's actually 
using the documented way to statically extract a version number string 
from a Linux kernel binary, even a compressed one.  A lot of things, 
including Grub, depends on it.  If they're doing something other than 
that, of course, then they deserve what they get.

Now, their problem is that they're making assumptions that are probably 
unwarranted about the contents of that string.

	-hpa