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

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

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.

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

Does this fix the problem?

Linus

----

index dc3e580..6a56c4f 100644
@@ -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);
 }

index e8bfac3..b0c4a05 100644
@@ -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)

index 036f97c..c783695 100644
@@ -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();

index 2a5dfcd..9d96d36 100644
@@ -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";
-

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";

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

index dc3e580..92ea774 100644
@@ -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);
 }
 

index e8bfac3..b0c4a05 100644
@@ -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)

index 036f97c..c783695 100644
@@ -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();

index 2a5dfcd..9d96d36 100644
@@ -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";
-

cool!

should definitely work for all 'known' cases

thanks,
Herbert

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

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'

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

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.

From: Paul Mackerras <[email protected]>
Date: Tue, 12 Dec 2006 09:04:41 +1100

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.

Hi,

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

cheers,

Gerd

Gerd Hoffmann <[email protected]>

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

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 ...

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

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

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

-apw

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
===================================================================
@@ -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
===================================================================
@@ -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;

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

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

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

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

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

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

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.

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

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

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

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

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.

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.

See above. "DoSyscall" is where you enter.

Linus

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!

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

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.

Yes, thread_info->flags.

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

Ben.

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

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.

Hi!

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.

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.

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

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

Thats why get_kernel_version calls gzip.

No, you cant.

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

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

	-`J'

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

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?

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

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'

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