2007-08-16 16:26:40

by Jeff Dike

[permalink] [raw]
Subject: [PATCH] UML - Add a .note.SuSE section

[ This is both 2.6.24 and -stable material ]

SuSE seems to require that binaries have a .note.SuSE section.
Without it, UML segfaults if any parameters are passed on the command
line.

Signed-off-by: Jeff Dike <[email protected]>
--
arch/um/kernel/dyn.lds.S | 1 +
1 file changed, 1 insertion(+)

Index: linux-2.6.22/arch/um/kernel/dyn.lds.S
===================================================================
--- linux-2.6.22.orig/arch/um/kernel/dyn.lds.S 2007-08-15 10:39:39.000000000 -0400
+++ linux-2.6.22/arch/um/kernel/dyn.lds.S 2007-08-15 12:07:10.000000000 -0400
@@ -10,6 +10,7 @@ SECTIONS
PROVIDE (__executable_start = START);
. = START + SIZEOF_HEADERS;
.interp : { *(.interp) }
+ .note.SuSE : { *(.note.SuSE) }
__binary_start = .;
. = ALIGN(4096); /* Init code and data */
_text = .;


2007-08-16 16:36:09

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] UML - Add a .note.SuSE section


On Thu, 2007-08-16 at 12:24 -0400, Jeff Dike wrote:
> [ This is both 2.6.24 and -stable material ]
>
> SuSE seems to require that binaries have a .note.SuSE section.
> Without it, UML segfaults if any parameters are passed on the command
> line.


this sounds like something really stupid and bad... why would the kernel
need to have a per-distro note section???


2007-08-16 16:41:18

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [PATCH] UML - Add a .note.SuSE section

* Jeff Dike ([email protected]) wrote:
> [ This is both 2.6.24 and -stable material ]
>
> SuSE seems to require that binaries have a .note.SuSE section.
> Without it, UML segfaults if any parameters are passed on the command
> line.

Huh!? Why do we need a SuSE section?
thanks,
-chris

2007-08-16 16:56:18

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] UML - Add a .note.SuSE section

On Thu, Aug 16, 2007 at 09:30:56AM -0700, Arjan van de Ven wrote:
>
> On Thu, 2007-08-16 at 12:24 -0400, Jeff Dike wrote:
> > [ This is both 2.6.24 and -stable material ]
> >
> > SuSE seems to require that binaries have a .note.SuSE section.
> > Without it, UML segfaults if any parameters are passed on the command
> > line.
>
>
> this sounds like something really stupid and bad... why would the kernel
> need to have a per-distro note section???

I agree, what did we mess up in the SuSE kernel to require such a hack?
:)

thanks,

greg k-h

2007-08-16 18:36:27

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] UML - Add a .note.SuSE section

Jeff Dike <[email protected]> writes:

> [ This is both 2.6.24 and -stable material ]
>
> SuSE seems to require that binaries have a .note.SuSE section.
> Without it, UML segfaults if any parameters are passed on the command
> line.

This doesn't make any sense. You must have misanalyzed this.

-Andi

2007-08-16 19:28:42

by Jeff Dike

[permalink] [raw]
Subject: Re: [PATCH] UML - Add a .note.SuSE section

On Thu, Aug 16, 2007 at 09:30:56AM -0700, Arjan van de Ven wrote:
> this sounds like something really stupid and bad... why would the kernel
> need to have a per-distro note section???

On Thu, Aug 16, 2007 at 09:39:06AM -0700, Chris Wright wrote:
> Huh!? Why do we need a SuSE section?

On Thu, Aug 16, 2007 at 09:54:55AM -0700, Greg KH wrote:
> I agree, what did we mess up in the SuSE kernel to require such a hack?

Beats the crap out of me.

Drop this patch - it looks like it might just be papering over
symptoms rather than fixing the real problem - see below.

What I do know is that current UML doesn't run when built on a SuSE
host, the UML commit which caused it to break is
c35e584c087381aaa5f1ed40a28b978535c18fb2

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=c35e584c087381aaa5f1ed40a28b978535c18fb2;hp=a5bd1786fb30abe663b904f6d79bba413e9ba883

and the difference between a working UML binary and a broken one is
this:

+ 1 .note.ABI-tag 00000020 0000000060000254 0000000060000254 00000254 2**2
+ CONTENTS, ALLOC, LOAD, READONLY, DATA
+ 2 .note.SuSE 00000018 0000000060000274 0000000060000274 00000274 2**2
+ CONTENTS, ALLOC, LOAD, READONLY, DATA

and the .note.SuSE section makes the difference.

Looking into it a bit further, the contents of the section are:

objdump --section=.note.SuSE -s uml8796-linux-good

uml8796-linux-good: file format elf64-x86-64

Contents of section .note.SuSE:
60000274 05000000 04000000 53755345 53755345 ........SuSESuSE
60000284 00000000 01000a02 ........

which seems kind of pointless, but also harmless.

The crash is in this section:

__uml_setup_start = .;
.uml.setup.init : { *(.uml.setup.init) }
__uml_setup_end = .;

with &__uml_setup_start being 8 bytes before the start of the first
16-byte structure in .uml.setup.init, so the structures are misaligned
wrt the start symbol.

I don't see any connection between the presence of a section at the
start of the binary and this misalignment, so the patch is probably wrong.

Jeff

--
Work email - jdike at linux dot intel dot com

2007-08-16 20:03:58

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] UML - Add a .note.SuSE section

On Thu, Aug 16, 2007 at 03:26:39PM -0400, Jeff Dike wrote:
>
> The crash is in this section:
>
> __uml_setup_start = .;
> .uml.setup.init : { *(.uml.setup.init) }
> __uml_setup_end = .;

This looks like a classic bug.
You wanted this:
.uml.setup.init : {
__uml_setup_start = .;
*(.uml.setup.init)
__uml_setup_end = .;
}

In this way you are sure __uml_setup_start has same address as start
of section.
With the original code the linker will align the .uml.setup.init section
to the alignment required by the included sections and thus
the label and the actual start address may differ.

Sam

2007-08-16 21:07:25

by Jeff Dike

[permalink] [raw]
Subject: Re: [PATCH] UML - Add a .note.SuSE section

On Thu, Aug 16, 2007 at 10:04:55PM +0200, Sam Ravnborg wrote:
> On Thu, Aug 16, 2007 at 03:26:39PM -0400, Jeff Dike wrote:
> >
> > The crash is in this section:
> >
> > __uml_setup_start = .;
> > .uml.setup.init : { *(.uml.setup.init) }
> > __uml_setup_end = .;
>
> This looks like a classic bug.
> You wanted this:
> .uml.setup.init : {
> __uml_setup_start = .;
> *(.uml.setup.init)
> __uml_setup_end = .;
> }

Ooh, this sounds promising, thanks.

Jeff

--
Work email - jdike at linux dot intel dot com

2007-08-21 17:12:54

by Blaisorblade

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] UML - Add a .note.SuSE section

On gioved? 16 agosto 2007, Jeff Dike wrote:
> On Thu, Aug 16, 2007 at 10:04:55PM +0200, Sam Ravnborg wrote:
> > On Thu, Aug 16, 2007 at 03:26:39PM -0400, Jeff Dike wrote:
> > > The crash is in this section:
> > >
> > > __uml_setup_start = .;
> > > .uml.setup.init : { *(.uml.setup.init) }
> > > __uml_setup_end = .;
> >
> > This looks like a classic bug.
> > You wanted this:
> > .uml.setup.init : {
> > __uml_setup_start = .;
> > *(.uml.setup.init)
> > __uml_setup_end = .;
> > }
>
> Ooh, this sounds promising, thanks.

It's not the first time we hit effects of such bugs, is it? The .note.ABI-tag
fix, time ago, may be about the same problem. Can you double-check all UML
linker scripts for more instances of this bug?

Bye
--
"Doh!" (cit.), I've made another another mistake!
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


Attachments:
(No filename) (903.00 B)
signature.asc (189.00 B)
This is a digitally signed message part.
Download all attachments

2007-08-22 16:37:55

by Jeff Dike

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] UML - Add a .note.SuSE section

On Tue, Aug 21, 2007 at 07:05:53PM +0200, Blaisorblade wrote:
> It's not the first time we hit effects of such bugs, is it?

I don't remember seeing this before.

> The .note.ABI-tag fix, time ago, may be about the same problem.

Are you referring to
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=c35e584c087381aaa5f1ed40a28b978535c18fb2;hp=a5bd1786fb30abe663b904f6d79bba413e9ba883?
If so, I never understood that - it just came in saying "this fixes
static building", so I sent it along. BTW, that commit was singled
out by git-bisect as "causing" this particular problem.

> Can you
> double-check all UML linker scripts for more instances of this bug?

I did, I have a patch, and it's been verified to fix the problem.

Jeff

--
Work email - jdike at linux dot intel dot com

2007-08-23 14:50:56

by Blaisorblade

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] UML - Add a .note.SuSE section

On mercoled? 22 agosto 2007, Jeff Dike wrote:
> On Tue, Aug 21, 2007 at 07:05:53PM +0200, Blaisorblade wrote:
> > It's not the first time we hit effects of such bugs, is it?
>
> I don't remember seeing this before.
>
> > The .note.ABI-tag fix, time ago, may be about the same problem.
>
> Are you referring to
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdi
>ff;h=c35e584c087381aaa5f1ed40a28b978535c18fb2;hp=a5bd1786fb30abe663b904f6d79
>bba413e9ba883?

Yes.

>If so, I never understood that - it just came in saying "this
> fixes static building", so I sent it along.

In this case, I'm referring to the patch which had a typo, which is yours:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7632fc8f809a97f9d82ce125e8e3e579390ce2e5
Description follows:
"During a static link, ld has started putting a .note section in the
.uml.setup.init section. This has the result that the UML setups begin
with 32 bytes of garbage and UML crashes immediately on boot.

This patch creates a specific .note section for ld to drop this stuff
into."

My patch only made your change work for real - IIRC you had fixed that exact
typo too, but you forgot to run quilt refresh before sending the patch (btw,
quilt pop -a will force you to refresh all patches to succeed - I do it
frequently).

> BTW, that commit was singled
> out by git-bisect as "causing" this particular problem.
>
> > Can you
> > double-check all UML linker scripts for more instances of this bug?
>
> I did, I have a patch, and it's been verified to fix the problem.

In this case, we _may_ want to remove the .note section altogether - even if
it is likely to shake out more problems.

Good bye!
--
"Doh!" (cit.), I've made another another mistake!
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


Attachments:
(No filename) (1.84 kB)
signature.asc (189.00 B)
This is a digitally signed message part.
Download all attachments