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
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.
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
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";
-
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.
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
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.
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
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
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?
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";
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
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";
-
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";
> -
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?
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.
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
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'
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
>
> > (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
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
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 [email protected]
> 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
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 [email protected]
> >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
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'
--
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
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'
--
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;
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
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
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?
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
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
> 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
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
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
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.
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
From: Paul Mackerras <[email protected]>
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.
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 <[email protected]>
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
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
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
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
> 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.
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
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.
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
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.