[PATCH] UML - Add a .note.SuSE section

[PATCH] UML - Add a .note.SuSE section

[Jeff Dike]

[ 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 <jdike@linux.intel.com>
--
 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 = .;

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

[Arjan van de Ven]

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

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

[Chris Wright]

* Jeff Dike (jdike@addtoit.com) 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

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

[Greg KH]

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

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

[Jeff Dike]

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

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

[Andi Kleen]

Jeff Dike <jdike@addtoit.com> 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

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

[Sam Ravnborg]

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

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

[Jeff Dike]

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

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

[Blaisorblade]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Jeff Dike]

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

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

[Blaisorblade]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]