kernel segv with 2.6.31-rc6 ?

kernel segv with 2.6.31-rc6 ?

[Helge Deller]

anyone else seeing this with 2.6.31-rc6 ?

<...system boots up...>
Waiting for /dev to be fully populated...
  
sysfs: cannot create duplicate filename '/module/ac97_bus/sections/.text'
------------[ cut here ]------------
Badness at fs/sysfs/dir.c:487
  
      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
PSW: 00000000000001000000000000001111 Not tainted
r00-03  0004000f 10669bd0 10204ff8 7ce58340
r04-07  7efcd000 ffffffef 7f881d74 7efcd000
r08-11  0008746c 00000000 7f1c6400 0008441c
r12-15  00000019 00084332 105a96c8 00000124
r16-19  0000fff1 00000017 00084abc ffffffff
r20-23  7eeff080 00000060 102f5898 10330dec
r24-27  ffffffff 0000000e 10669c04 10656670
r28-31  00000050 00000190 7ce583c0 10123988
sr00-03  00000000 00000000 00000000 00000008
sr04-07  00000000 00000000 00000000 00000000
  
IASQ: 00000000 00000000 IAOQ: 10204ff8 10204ffc
  IIR: 03ffe01f    ISR: 00000000  IOR: 00000000
  CPU:        0   CR30: 7ce58000 CR31: 11111111
  ORIG_R28: 00000001
  IAOQ[0]: sysfs_add_one+0xb8/0xd0
  IAOQ[1]: sysfs_add_one+0xbc/0xd0
  RP(r2): sysfs_add_one+0xb8/0xd0
Backtrace:
  [<102045b8>] sysfs_add_file_mode+0x60/0xc4
  [<1020748c>] internal_create_group+0xf0/0x1d8
  
Backtrace:
  [<1016f0f0>] load_module+0x10e8/0x1294
  

Kernel Fault: Code=26 regs=7ce58200 (Addr=00000030)
  
      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
PSW: 00000000000001100000000000001111 Tainted: G        W
r00-03  0006000f 7ce58200 1016f0f0 7ce58140
r04-07  00084444 00000019 00087424 7eddb660
r08-11  00084000 00000001 7f1c66a4 0008441c
r12-15  00000019 00084332 105a96c8 00000124
r16-19  0000fff1 00000017 00084abc 00000000
r20-23  00000000 1016d3e0 7eddb668 00000124
r24-27  105a96cc 00000000 7eddb660 10656670
r28-31  00000000 00000001 7ce58200 00000000
sr00-03  00000000 00000000 00000000 00000008
sr04-07  00000000 00000000 00000000 00000000
  
IASQ: 00000000 00000000 IAOQ: 1016f130 1016f134
  IIR: 4b940060    ISR: 00000000  IOR: 00000030
  CPU:        0   CR30: 7ce58000 CR31: 11111111
  ORIG_R28: 00084332
  IAOQ[0]: load_module+0x1128/0x1294
  IAOQ[1]: load_module+0x112c/0x1294
  RP(r2): load_module+0x10e8/0x1294
Backtrace:
  [<1016f0f0>] load_module+0x10e8/0x1294
  
Kernel panic - not syncing: Kernel Fault
Backtrace:
  [<1011ac28>] show_stack+0x18/0x28
  [<1013f3a0>] vprintk+0x19c/0x430
  

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Mon, 2009-08-17 at 23:31 +0200, Helge Deller wrote:
> anyone else seeing this with 2.6.31-rc6 ?
> 
> <...system boots up...>
> Waiting for /dev to be fully populated...
>   
> sysfs: cannot create duplicate filename '/module/ac97_bus/sections/.text'

Not on ion.  However, it could be linker discrepancies ... what does 

objdump -x

show for the module in question ... does it have two .text sections?

James

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Mon, 2009-08-17 at 23:31 +0200, Helge Deller wrote:
> anyone else seeing this with 2.6.31-rc6 ?
> 
> <...system boots up...>
> Waiting for /dev to be fully populated...
>   
> sysfs: cannot create duplicate filename '/module/ac97_bus/sections/.text'
> ------------[ cut here ]------------
> Badness at fs/sysfs/dir.c:487
>   
>       YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> PSW: 00000000000001000000000000001111 Not tainted
> r00-03  0004000f 10669bd0 10204ff8 7ce58340
> r04-07  7efcd000 ffffffef 7f881d74 7efcd000
> r08-11  0008746c 00000000 7f1c6400 0008441c
> r12-15  00000019 00084332 105a96c8 00000124
> r16-19  0000fff1 00000017 00084abc ffffffff
> r20-23  7eeff080 00000060 102f5898 10330dec
> r24-27  ffffffff 0000000e 10669c04 10656670
> r28-31  00000050 00000190 7ce583c0 10123988
> sr00-03  00000000 00000000 00000000 00000008
> sr04-07  00000000 00000000 00000000 00000000
>   
> IASQ: 00000000 00000000 IAOQ: 10204ff8 10204ffc
>   IIR: 03ffe01f    ISR: 00000000  IOR: 00000000
>   CPU:        0   CR30: 7ce58000 CR31: 11111111
>   ORIG_R28: 00000001
>   IAOQ[0]: sysfs_add_one+0xb8/0xd0
>   IAOQ[1]: sysfs_add_one+0xbc/0xd0
>   RP(r2): sysfs_add_one+0xb8/0xd0
> Backtrace:
>   [<102045b8>] sysfs_add_file_mode+0x60/0xc4
>   [<1020748c>] internal_create_group+0xf0/0x1d8
>   
> Backtrace:
>   [<1016f0f0>] load_module+0x10e8/0x1294
>   
> 
> Kernel Fault: Code=26 regs=7ce58200 (Addr=00000030)
>   
>       YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
> PSW: 00000000000001100000000000001111 Tainted: G        W
> r00-03  0006000f 7ce58200 1016f0f0 7ce58140
> r04-07  00084444 00000019 00087424 7eddb660
> r08-11  00084000 00000001 7f1c66a4 0008441c
> r12-15  00000019 00084332 105a96c8 00000124
> r16-19  0000fff1 00000017 00084abc 00000000
> r20-23  00000000 1016d3e0 7eddb668 00000124
> r24-27  105a96cc 00000000 7eddb660 10656670
> r28-31  00000000 00000001 7ce58200 00000000
> sr00-03  00000000 00000000 00000000 00000008
> sr04-07  00000000 00000000 00000000 00000000
>   
> IASQ: 00000000 00000000 IAOQ: 1016f130 1016f134
>   IIR: 4b940060    ISR: 00000000  IOR: 00000030
>   CPU:        0   CR30: 7ce58000 CR31: 11111111
>   ORIG_R28: 00084332
>   IAOQ[0]: load_module+0x1128/0x1294
>   IAOQ[1]: load_module+0x112c/0x1294
>   RP(r2): load_module+0x10e8/0x1294
> Backtrace:
>   [<1016f0f0>] load_module+0x10e8/0x1294
>   
> Kernel panic - not syncing: Kernel Fault
> Backtrace:
>   [<1011ac28>] show_stack+0x18/0x28
>   [<1013f3a0>] vprintk+0x19c/0x430

The root cause is a duplicate section name (.text); is this legal?

However, there's a problem with commit
6d76013381ed28979cd122eb4b249a88b5e384fa in that if you fail to allocate
a mod->sect_attrs (in this case it's null because of the duplication),
it still gets used without checking in add_notes_attrs()

This should fix it

Signed-off-by: James Bottomley <James.Bottomley@suse.de>

---

diff --git a/kernel/module.c b/kernel/module.c
index fd14114..a703c49 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2353,7 +2353,8 @@ static noinline struct module *load_module(void __user *umod,
 	if (err < 0)
 		goto unlink;
 	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
-	add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
+	if (mod->sect_attrs)
+		add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
 
 	/* Get rid of temporary copy */
 	vfree(hdr);

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> The root cause is a duplicate section name (.text); is this legal?

It's a complicated subject, but I don't think they should ever actually
exist in a .ko.  The linker script for making .ko's should take care of
that.  But if modules do some crazy things like using section groups
(e.g. for COMDAT, i.e. compile C++ code into modules) then I'm not sure how
easy it is to get ld to combine things ideally as we'd like.

Suffice it to say that any occurrence of this merits further investigation.
It is certainly a red flag at first blush (as it were).

> However, there's a problem with commit
> 6d76013381ed28979cd122eb4b249a88b5e384fa in that if you fail to allocate
> a mod->sect_attrs (in this case it's null because of the duplication),
> it still gets used without checking in add_notes_attrs()
> 
> This should fix it

Yes, good catch.  Sorry about that complete failure of defensive programming.

Acked-by: Roland McGrath <roland@redhat.com>


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[Rusty Russell]

On Tue, 18 Aug 2009 08:19:36 am James Bottomley wrote:
> The root cause is a duplicate section name (.text); is this legal?

I'd be happy to fail to load it.  There might be sysfs issues with it
too.

> However, there's a problem with commit
> 6d76013381ed28979cd122eb4b249a88b5e384fa in that if you fail to allocate
> a mod->sect_attrs (in this case it's null because of the duplication),
> it still gets used without checking in add_notes_attrs()
> 
> This should fix it

No, the real problem is that it ignores failure.  I'd much rather fail
the module load than various features mysteriously MIA.

Which brings us to "patches which don't go thru the maintainer" (or
perhaps, non-responsive maintainers who get bypassed).

Thanks,
Rusty.

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Tue, 2009-08-18 at 12:48 +0930, Rusty Russell wrote:
> On Tue, 18 Aug 2009 08:19:36 am James Bottomley wrote:
> > The root cause is a duplicate section name (.text); is this legal?
> 
> I'd be happy to fail to load it.  There might be sysfs issues with it
> too.

Well, that's why I want clarification.  The bad code has been in since
2007 so it looks like a recent change, probably the one to more default
linker scripts is the cause.

> > However, there's a problem with commit
> > 6d76013381ed28979cd122eb4b249a88b5e384fa in that if you fail to allocate
> > a mod->sect_attrs (in this case it's null because of the duplication),
> > it still gets used without checking in add_notes_attrs()
> > 
> > This should fix it
> 
> No, the real problem is that it ignores failure.  I'd much rather fail
> the module load than various features mysteriously MIA.

There are two separate problems.  One is why the the module has
duplicate sections.  Under my reading of the ELF spec, they seem to be
allowable ... however we control the linker scripts and I believe we
shouldn't have generated them.

The other is the missing error handling in the module loader.

The question of whether this is a generic failure that needs load
refusal or an expected artifact of our new linker scripts needs
investigating.

> Which brings us to "patches which don't go thru the maintainer" (or
> perhaps, non-responsive maintainers who get bypassed).

Well, the original code was in 2007, so it's probably a bit late for a
postmortem.

James

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> I'd be happy to fail to load it.  There might be sysfs issues with it too.

That sounds reasonable to me.  And I'd be happy to at least look a little
and maybe give some advice to anybody who finds themself building such a
(free) module, doesn't know why or how it got that way, and wants to ask.

> No, the real problem is that it ignores failure.  I'd much rather fail
> the module load than various features mysteriously MIA.

In that regard, I just made add_notes_attrs() follow the model of
add_sect_attrs(), which (gracefully) ignores all its failures.  I don't
know what the thought behind that was.  My only guess was that since this
is all CONFIG_KALLSYMS-only features, that someone thought turning on
CONFIG_KALLSYMS should not add new ways to lose that weren't there before,
only new ways to lose the new features that weren't there before either.
Having these other alloc/sysfs failures cause the module load to fail would
certainly be fine with me.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

[resending, fluffed reply-all]

On Mon, 2009-08-17 at 22:06 -0700, Roland McGrath wrote:
> > I'd be happy to fail to load it.  There might be sysfs issues with it too.
> 
> That sounds reasonable to me.  And I'd be happy to at least look a little
> and maybe give some advice to anybody who finds themself building such a
> (free) module, doesn't know why or how it got that way, and wants to ask.

Actually, for parisc, its not reasonable.  It's expected that our
modules have multiple text sections (we have to use -ffunction-sections
to generate them in order that the PCREL17 jump stubs can be
interleaved).

The problem looks to be that some linker error gave the one of the named
function text sections a duplicate name.  Helge, can you post he objdump
info that shows which section had a duplicate name?

Even with the duplicate name, though, the module should be perfectly
loadable.

James

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Actually, for parisc, its not reasonable.  It's expected that our modules
> have multiple text sections (we have to use -ffunction-sections to
> generate them in order that the PCREL17 jump stubs can be interleaved).

I don't think you need what you think you need.  Having lots of sections in
your .o's when you compile is fine.  These should be combined by the linker
script that creates the .ko, however.  Unless I am missing something, there
is no purpose to this section distinction at insmod time--it's only
important for the relative layout of the parts of the .ko's text, which
winds up contiguous whether laid out that way at ld -r (.ko creation) time
or at insmod time.

> Even with the duplicate name, though, the module should be perfectly
> loadable.

But its /sys/module/foo/sections/ virtual directory becomes useless,
as a single name space can no longer describe what sections it has.
So perhaps it is then proper for add_sect_attrs() to punt on it.
But that reduces the functionality you get from CONFIG_KALLSYMS.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Tue, 2009-08-18 at 17:14 -0700, Roland McGrath wrote:
> > Actually, for parisc, its not reasonable.  It's expected that our modules
> > have multiple text sections (we have to use -ffunction-sections to
> > generate them in order that the PCREL17 jump stubs can be interleaved).
> 
> I don't think you need what you think you need.  Having lots of sections in
> your .o's when you compile is fine.  These should be combined by the linker
> script that creates the .ko, however.  Unless I am missing something, there
> is no purpose to this section distinction at insmod time--it's only
> important for the relative layout of the parts of the .ko's text, which
> winds up contiguous whether laid out that way at ld -r (.ko creation) time
> or at insmod time.

Actually, I think we do; the module loader is a runtime linker, after
all.  We have specific module bugs we fixed by inserting relocation
stubs between the text sections.

Our specific problem is that the standard pa relocation is PCREL17 ...
as you can appreciate, 17 bits of relative jump isn't a lot.  If the
target isn't within the 17 bits, we have to insert a relocation stub to
do an indirect absolute jump thought the GOT.  Our problems occur when a
text section is wider than the 17 bits, which happens a lot in the
larger modules ... with nowhere to put the stub within range of the
relocation we can't load them.  Our fix is to split the text sections
with -ffunction-sections so we can be pretty much assured of having a
stub location within the 17 bits.

We don't care what they're called or anything.  We just care that
the .text is split up into multiple separately relocateable sections so
we can get the stubs within range of the jumps.

Now, of course, if the final linker could be persuaded to sprinkle
needed stubs through the text section and all we have to do is GOT
relocations, we don't need all the jiggery-pokery ... but I'm told this
can't be done.

James

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Actually, I think we do; the module loader is a runtime linker, after
> all.  [...]

Indeed you do.  I've just read some of the parts of ld that normally
address this issue for HPPA.  They don't run for ld -r.  So this is just
another fine example of the lunacy of the ET_REL .ko madness that would be
naturally avoided by a sensible tweaked ET_DYN scheme.  But that battle was
lost way, way back in the long, long ago, so long ago they were probably
even still making HPPA machines then.

> Now, of course, if the final linker could be persuaded to sprinkle
> needed stubs through the text section and all we have to do is GOT
> relocations, we don't need all the jiggery-pokery ... but I'm told this
> can't be done.

Not with ld -r as it is today.  That's what ld does for you in proper final
links.  It looks to me like you might be able to enable some special mode
("finalish link" for -r) with a hack to HPPA ld to apply this stub-creation
logic based on the assumption that the symbols in the relocs will be
resolved to themselves, and barf on you if they're used for SHN_UNDEF symbols.
But nobody cares enough to fiddle with ld.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Tue, 2009-08-18 at 18:31 -0700, Roland McGrath wrote:
> > Actually, I think we do; the module loader is a runtime linker, after
> > all.  [...]
> 
> Indeed you do.  I've just read some of the parts of ld that normally
> address this issue for HPPA.  They don't run for ld -r.  So this is just
> another fine example of the lunacy of the ET_REL .ko madness that would be
> naturally avoided by a sensible tweaked ET_DYN scheme.

Using ET_DYN would have made our life easier when trying to code the
kernel module loader as well.  The basic problem is, of course, that
this is simple on an x86, so it didn't matter that much for the initial
implementation.  It just becomes less simple on anything else.

>   But that battle was
> lost way, way back in the long, long ago, so long ago they were probably
> even still making HPPA machines then.

Well, since they're still making parisc machines, I assume you mean
further back than when the production lines knocked off today?

> > Now, of course, if the final linker could be persuaded to sprinkle
> > needed stubs through the text section and all we have to do is GOT
> > relocations, we don't need all the jiggery-pokery ... but I'm told this
> > can't be done.
> 
> Not with ld -r as it is today.  That's what ld does for you in proper final
> links.  It looks to me like you might be able to enable some special mode
> ("finalish link" for -r) with a hack to HPPA ld to apply this stub-creation
> logic based on the assumption that the symbols in the relocs will be
> resolved to themselves, and barf on you if they're used for SHN_UNDEF symbols.
> But nobody cares enough to fiddle with ld.

So that leaves us stuck with the current implementation and still
needing a solution for the duplicate section names?

James

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Using ET_DYN would have made our life easier when trying to code the
> kernel module loader as well.  The basic problem is, of course, that
> this is simple on an x86, so it didn't matter that much for the initial
> implementation.  It just becomes less simple on anything else.

There are generic ways that ET_DYN is better for all, too.  
A little birdie told me it was all the fault of mips.
Anyway, water under the bridge.

> Well, since they're still making parisc machines, I assume you mean
> further back than when the production lines knocked off today?

I'm pretty sure you knew what I meant.

> So that leaves us stuck with the current implementation and still
> needing a solution for the duplicate section names?

Something like that.  Your fix makes module loading work again, so we
could just let /sys/module/*/{sections,notes}/ be missing on parisc
and see how long it takes anybody to complain, assuming you talk Rusty
out of his preference to change those error cases to refuse to load
the module.  OTOH, you said earlier that it was only some mistake that
produced multiple same-named sections to begin with, so you could just
try to fix that and then forget about it.

Finally, we could decide to really support the general case including
duplicate section names in these features.  That's really not very
hard to do, if we just completely change the interfaces for those
features.  e.g. instead of a /sys/module/*/sections directory, it
could just be a single file that contains lines of:

SHNDX 0xADDR NAME

For /sys/module/*/notes, no consumer really cares about the section
distinctions at all.  It would be easy enough just to have a single
pseudo-file that delivers all note sections concatenated together.
(In practice, there is usually only one note section at most anyway.)

Of course, such changes would be a new incompatible change to the
userland sysfs ABI, require compatibility concerns, etc.  It seems
unlikely anyone really wants to bother with all that.  It would be
more natural presentation of the info IMHO, but it hardly matters.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[Rusty Russell]

On Wed, 19 Aug 2009 11:08:36 am James Bottomley wrote:
> On Tue, 2009-08-18 at 18:31 -0700, Roland McGrath wrote:
> > > Actually, I think we do; the module loader is a runtime linker, after
> > > all.  [...]
> > 
> > Indeed you do.  I've just read some of the parts of ld that normally
> > address this issue for HPPA.  They don't run for ld -r.  So this is just
> > another fine example of the lunacy of the ET_REL .ko madness that would be
> > naturally avoided by a sensible tweaked ET_DYN scheme.
> 
> Using ET_DYN would have made our life easier when trying to code the
> kernel module loader as well.  The basic problem is, of course, that
> this is simple on an x86, so it didn't matter that much for the initial
> implementation.  It just becomes less simple on anything else.

Actually, x86 was one of the archs which fucked us.  Richard Henderson and
I *had* this, but ld -shared without -fPIC helpfully tells you "you're doing
it wrong" on x86-64.

There were other issues, ISTR MIPS was a showstopper.  Google finds the
following summary I wrote when this stuff was fresher:

http://lkml.org/lkml/2003/1/12/271 :

	While ET_DYN modules are a reasonably serious win for ia64 (and
	probably hppa) (ie. -300 lines or so), they're a minor win for alpha
	and ppc64 (-100 lines or so), and no real change for arm, i386, ppc,
	sparc, and sparc64.  It's a lose for x86_64 (toolchain fixes, unless
	they want to use -fPIC for modules), mips and mips64 (major toolchain
	fixes, unless they want to use -fPIC for modules and stop using r28
	for current inside modules).

> >   But that battle was
> > lost way, way back in the long, long ago, so long ago they were probably
> > even still making HPPA machines then.

This isn't quite true; userspace should handle ET_DYN fine (at least, it
was supposed to).

So you could change any arch to use that, but it's a fair refactor if we leave
some archs behind.

If anyone's really interested, I can dig out the bits I have...

> So that leaves us stuck with the current implementation and still
> needing a solution for the duplicate section names?

If this is not a "don't do that" bug, we could try hacking around it in
parisc's module_arch_frob_sections?

Rusty.

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Actually, x86 was one of the archs which fucked us.  Richard Henderson and
> I *had* this, but ld -shared without -fPIC helpfully tells you "you're doing
> it wrong" on x86-64.

This complaint could easily be (have been) made optional.

> There were other issues, ISTR MIPS was a showstopper.

Richard told me it's all MIPS' fault, but I was being discreet.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[Rusty Russell]

On Wed, 26 Aug 2009 04:54:32 am Roland McGrath wrote:
> > Actually, x86 was one of the archs which fucked us.  Richard Henderson and
> > I *had* this, but ld -shared without -fPIC helpfully tells you "you're doing
> > it wrong" on x86-64.
> 
> This complaint could easily be (have been) made optional.

Yep, I even had a patch.  And meanwhile we could have lived with -fPIC modules
with some loss of performance (I think).

But at some level the problems accumulate until you go "nice idea, let's go
do something else" :)

But don't let me discourage you!
Rusty.

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> But at some level the problems accumulate until you go "nice idea, let's go
> do something else" :)

Yeah, done been gone.

> But don't let me discourage you!

I've already spent too much time making ET_REL .ko DWARF parsing work to be
very motivated to do things right so all that work would never have been
necessary like I've been grumbling about the whole time. ;-)


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[Helge Deller]

> On Mon, 2009-08-17 at 22:06 -0700, Roland McGrath wrote:
> > > I'd be happy to fail to load it.  There might be sysfs issues wit=
h it
> too.
> >=20
> > That sounds reasonable to me.  And I'd be happy to at least look a
> little
> > and maybe give some advice to anybody who finds themself building s=
uch a
> > (free) module, doesn't know why or how it got that way, and wants t=
o
> ask.
>=20
> Actually, for parisc, its not reasonable.  It's expected that our
> modules have multiple text sections (we have to use -ffunction-sectio=
ns
> to generate them in order that the PCREL17 jump stubs can be
> interleaved).
>=20
> The problem looks to be that some linker error gave the one of the na=
med
> function text sections a duplicate name.  Helge, can you post he objd=
ump
> info that shows which section had a duplicate name?
>=20
> Even with the duplicate name, though, the module should be perfectly
> loadable.


It's the ac97_bus kernel module:
-rw-r--r-- 1 root root 3.0K 2009-08-19 12:25 ac97_bus.ko=20


"objdump -x ac97_bus.ko" shows two .text sections:

ac97_bus.ko:     file format elf32-hppa-linux
ac97_bus.ko                                 =20
architecture: hppa1.1, flags 0x00000011:    =20
HAS_RELOC, HAS_SYMS                         =20
start address 0x00000000                    =20

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .note.gnu.build-id 00000024  00000000  00000000  00000034  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA           =20
  1 .text         00000000  00000000  00000000  00000058  2**0    =20
                  CONTENTS, ALLOC, LOAD, READONLY, CODE           =20
  2 .text.ac97_bus_match 0000001c  00000000  00000000  00000058  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE             =20
  3 .exit.text    00000030  00000000  00000000  00000074  2**2      =20
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE      =20
  4 .init.text    00000030  00000000  00000000  000000a4  2**2      =20
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE      =20
  5 .text         00000000  00000000  00000000  000000d4  2**0      =20
                  CONTENTS, ALLOC, LOAD, READONLY, CODE             =20
  6 .rodata.str1.4 00000008  00000000  00000000  000000d4  2**2     =20
                  CONTENTS, ALLOC, LOAD, READONLY, DATA             =20
  7 .modinfo      00000040  00000000  00000000  000000dc  2**2      =20
                  CONTENTS, ALLOC, LOAD, READONLY, DATA             =20
  8 __ksymtab     00000008  00000000  00000000  0000011c  2**2      =20
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA      =20
  9 __ksymtab_strings 0000000e  00000000  00000000  00000124  2**0  =20
                  CONTENTS, ALLOC, LOAD, READONLY, DATA             =20
 10 .PARISC.unwind 00000030  00000000  00000000  00000134  2**2     =20
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA      =20
 11 .data         00000034  00000000  00000000  00000164  2**2      =20
                  CONTENTS, ALLOC, LOAD, RELOC, DATA                =20
 12 .gnu.linkonce.this_module 00000160  00000000  00000000  00000198  2=
**2
                  CONTENTS, ALLOC, LOAD, RELOC, DATA, LINK_ONCE_DISCARD=
  =20
 13 .bss          00000000  00000000  00000000  000002f8  2**0         =
  =20
                  ALLOC                                                =
  =20
 14 .comment      0000003a  00000000  00000000  000002f8  2**0         =
  =20
                  CONTENTS, READONLY                                   =
  =20
SYMBOL TABLE:                                                          =
  =20
00000000 l    d  .note.gnu.build-id     00000000 .note.gnu.build-id    =
  =20
00000000 l    d  .text  00000000 .text                                 =
  =20
00000000 l    d  .text.ac97_bus_match   00000000 .text.ac97_bus_match  =
  =20
00000000 l    d  .exit.text     00000000 .exit.text                    =
  =20
00000000 l    d  .init.text     00000000 .init.text                    =
  =20
00000000 l    d  .text  00000000 .text                                 =
  =20
00000000 l    d  .rodata.str1.4 00000000 .rodata.str1.4                =
  =20
00000000 l    d  .modinfo       00000000 .modinfo                      =
  =20
00000000 l    d  __ksymtab      00000000 __ksymtab                     =
  =20
00000000 l    d  __ksymtab_strings      00000000 __ksymtab_strings     =
  =20
00000000 l    d  .PARISC.unwind 00000000 .PARISC.unwind                =
  =20
00000000 l    d  .data  00000000 .data                                 =
  =20
00000000 l    d  .gnu.linkonce.this_module      00000000 .gnu.linkonce.=
this_module
00000000 l    d  .bss   00000000 .bss                                  =
          =20
00000000 l    d  .comment       00000000 .comment                      =
          =20
00000000 l     F .text.ac97_bus_match   0000001c ac97_bus_match        =
          =20
00000000 l     F .exit.text     00000030 ac97_bus_exit                 =
          =20
00000000 l     F .init.text     00000030 ac97_bus_init                 =
          =20
00000000 l     O .modinfo       0000000c __mod_license77               =
          =20
00000000 l     O __ksymtab      00000008 __ksymtab_ac97_bus_type       =
          =20
00000000 l     O __ksymtab_strings      0000000e __kstrtab_ac97_bus_typ=
e         =20
0000000c l     O .modinfo       00000009 __module_depends              =
          =20
00000018 l     O .modinfo       00000026 __mod_vermagic5               =
          =20
00000000 g     O .gnu.linkonce.this_module      00000160 __this_module =
          =20
00000000 g     F .exit.text     00000030 cleanup_module                =
          =20
00000000 g     F .init.text     00000030 init_module                   =
          =20
00000000         *UND*  00000000 bus_unregister                        =
          =20
00000000 g     O .data  00000034 ac97_bus_type                         =
          =20
00000000         *UND*  00000000 bus_register                          =
          =20


RELOCATION RECORDS FOR [.exit.text]:
OFFSET   TYPE              VALUE
00000010 R_PARISC_DPREL21L  ac97_bus_type
00000014 R_PARISC_DPREL14R  ac97_bus_type
00000018 R_PARISC_PCREL17F  bus_unregister


RELOCATION RECORDS FOR [.init.text]:
OFFSET   TYPE              VALUE
00000010 R_PARISC_DPREL21L  ac97_bus_type
00000014 R_PARISC_DPREL14R  ac97_bus_type
00000018 R_PARISC_PCREL17F  bus_register


RELOCATION RECORDS FOR [__ksymtab]:
OFFSET   TYPE              VALUE
00000000 R_PARISC_DIR32    ac97_bus_type
00000004 R_PARISC_DIR32    __ksymtab_strings


RELOCATION RECORDS FOR [.PARISC.unwind]:
OFFSET   TYPE              VALUE
00000000 R_PARISC_SEGREL32  ac97_bus_match
00000004 R_PARISC_SEGREL32  .text.ac97_bus_match+0x00000018
00000010 R_PARISC_SEGREL32  ac97_bus_exit
00000014 R_PARISC_SEGREL32  .exit.text+0x0000002c
00000020 R_PARISC_SEGREL32  ac97_bus_init
00000024 R_PARISC_SEGREL32  .init.text+0x0000002c


RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE
00000000 R_PARISC_DIR32    .rodata.str1.4
00000010 R_PARISC_PLABEL32  ac97_bus_match


RELOCATION RECORDS FOR [.gnu.linkonce.this_module]:
OFFSET   TYPE              VALUE
000000d4 R_PARISC_PLABEL32  init_module
00000150 R_PARISC_PLABEL32  cleanup_module


Helge
--=20
GRATIS f=FCr alle GMX-Mitglieder: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel segv with 2.6.31-rc6 ?

[Helge Deller]

> > On Mon, 2009-08-17 at 22:06 -0700, Roland McGrath wrote:
> > > > I'd be happy to fail to load it.  There might be sysfs issues w=
ith
> it
> > too.
> > >=20
> > > That sounds reasonable to me.  And I'd be happy to at least look =
a
> > little
> > > and maybe give some advice to anybody who finds themself building=
 such
> a
> > > (free) module, doesn't know why or how it got that way, and wants=
 to
> > ask.
> >=20
> > Actually, for parisc, its not reasonable.  It's expected that our
> > modules have multiple text sections (we have to use -ffunction-sect=
ions
> > to generate them in order that the PCREL17 jump stubs can be
> > interleaved).
> >=20
> > The problem looks to be that some linker error gave the one of the =
named
> > function text sections a duplicate name.  Helge, can you post he ob=
jdump
> > info that shows which section had a duplicate name?
> >=20
> > Even with the duplicate name, though, the module should be perfectl=
y
> > loadable.
>=20
>=20
> It's the ac97_bus kernel module:
> -rw-r--r-- 1 root root 3.0K 2009-08-19 12:25 ac97_bus.ko=20

That's actually a problem of all kernel modules on hppa when using a ne=
wer compiler, not only the ac97_bus module.

The reason seems to be, that something in the newer gcc compilers chang=
ed to generate multiple sections all named ".text" for the PCREL17 relo=
cations.
Older compilers named those sections ".text.1", ".text.2", ".text.3" an=
d so forth.

"objdump -x ac97_bus.ko" with current (newer) compiler gives:
> Sections:
> Idx Name          Size      VMA       LMA       File off  Algn
>   0 .note.gnu.build-id 00000024  00000000  00000000  00000034  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA           =20
>   1 .text         00000000  00000000  00000000  00000058  2**0    =20
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE           =20
>   2 .text.ac97_bus_match 0000001c  00000000  00000000  00000058  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE             =20
>   3 .exit.text    00000030  00000000  00000000  00000074  2**2      =20
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE      =20
>   4 .init.text    00000030  00000000  00000000  000000a4  2**2      =20
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE      =20
>   5 .text         00000000  00000000  00000000  000000d4  2**0      =20
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE             =20
>...

older compiler produced:
Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00000000  00000000  00000000  00000034  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  1 .text.ac97_bus_match 0000001c  00000000  00000000  00000034  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .exit.text    00000030  00000000  00000000  00000050  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
  3 .init.text    00000030  00000000  00000000  00000080  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
  4 .text.1       00000000  00000000  00000000  000000b0  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
=2E..

Helge
--=20
Neu: GMX Doppel-FLAT mit Internet-Flatrate + Telefon-Flatrate
f=FCr nur 19,99 Euro/mtl.!* http://portal.gmx.net/de/go/dsl01
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel segv with 2.6.31-rc6 ?

[John David Anglin]

> The reason seems to be, that something in the newer gcc compilers changed to generate multiple sections all named ".text" for the PCREL17 relocations.
> Older compilers named those sections ".text.1", ".text.2", ".text.3" and so forth.

GCC has never generated ".text.1", etc, on parisc linux as far as I know.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: kernel segv with 2.6.31-rc6 ?

[Helge Deller]

On 08/20/2009 08:55 PM, John David Anglin wrote:
>> The reason seems to be, that something in the newer gcc compilers changed to generate multiple sections all named ".text" for the PCREL17 relocations.
>> Older compilers named those sections ".text.1", ".text.2", ".text.3" and so forth.
>
> GCC has never generated ".text.1", etc, on parisc linux as far as I know.

Hmm, I don't like to disagree with an gcc-expert like you,but
I did pasted an objdump in my last mail, which shows that gcc did
generated .text.1, .text.2 and so on:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
   0 .text         00000000  00000000  00000000  00000034  2**0
                   CONTENTS, ALLOC, LOAD, READONLY, CODE
...
   4 .text.1       00000000  00000000  00000000  000000b0  2**0
                   CONTENTS, ALLOC, LOAD, READONLY, CODE

Helge

Re: kernel segv with 2.6.31-rc6 ?

[James Bottomley]

On Thu, 2009-08-20 at 23:45 +0200, Helge Deller wrote:
> On 08/20/2009 08:55 PM, John David Anglin wrote:
> >> The reason seems to be, that something in the newer gcc compilers changed to generate multiple sections all named ".text" for the PCREL17 relocations.
> >> Older compilers named those sections ".text.1", ".text.2", ".text.3" and so forth.
> >
> > GCC has never generated ".text.1", etc, on parisc linux as far as I know.
> 
> Hmm, I don't like to disagree with an gcc-expert like you,but
> I did pasted an objdump in my last mail, which shows that gcc did
> generated .text.1, .text.2 and so on:
> 
> Sections:
> Idx Name          Size      VMA       LMA       File off  Algn
>    0 .text         00000000  00000000  00000000  00000034  2**0
>                    CONTENTS, ALLOC, LOAD, READONLY, CODE
> ...
>    4 .text.1       00000000  00000000  00000000  000000b0  2**0
>                    CONTENTS, ALLOC, LOAD, READONLY, CODE

Since this is a relinked object, might it be possible that ld rather
than gcc is the culprit?

James

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Since this is a relinked object, might it be possible that ld rather
> than gcc is the culprit?

I don't think ld -r does any of that sort of magic.

Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[John David Anglin]

> On Thu, 2009-08-20 at 23:45 +0200, Helge Deller wrote:
> > On 08/20/2009 08:55 PM, John David Anglin wrote:
> > >> The reason seems to be, that something in the newer gcc compilers changed to generate multiple sections all named ".text" for the PCREL17 relocations.
> > >> Older compilers named those sections ".text.1", ".text.2", ".text.3" and so forth.
> > >
> > > GCC has never generated ".text.1", etc, on parisc linux as far as I know.
> > 
> > Hmm, I don't like to disagree with an gcc-expert like you,but
> > I did pasted an objdump in my last mail, which shows that gcc did
> > generated .text.1, .text.2 and so on:
> > 
> > Sections:
> > Idx Name          Size      VMA       LMA       File off  Algn
> >    0 .text         00000000  00000000  00000000  00000034  2**0
> >                    CONTENTS, ALLOC, LOAD, READONLY, CODE
> > ...
> >    4 .text.1       00000000  00000000  00000000  000000b0  2**0
> >                    CONTENTS, ALLOC, LOAD, READONLY, CODE
> 
> Since this is a relinked object, might it be possible that ld rather
> than gcc is the culprit?

I think it must be from binutils.  It might be a result of broken
section merging.  Possibly, we don't merge .text sections when
doing ld -r so that stub sections could be interleaved.  Alan Modra
wrote that code and I'm not that familiar with it.

The GCC definition of TEXT_SECTION_ASM_OP in pa-linux.h has never changed.
The PA backend in GCC doesn't play any section games on linux targets,
so it's hard to see how this could happen.  Check the assembler output
from GCC if you don't believe me.

At some time, GCC may start a new .text section at the beginning of
each function.  We do this on hpux but not linux.  This will cause
multiple .text sections to be present in an object and allow finer grained
stub placement.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> At some time, GCC may start a new .text section at the beginning of
> each function.  We do this on hpux but not linux.  This will cause
> multiple .text sections to be present in an object and allow finer grained
> stub placement.

arch/parisc/Makefile:

	# Without this, "ld -r" results in .text sections that are too big
	# (> 0x40000) for branches to reach stubs.
	ifndef CONFIG_FUNCTION_TRACER
	  cflags-y	+= -ffunction-sections
	endif

Re: kernel segv with 2.6.31-rc6 ?

[John David Anglin]

> arch/parisc/Makefile:
> 
> 	# Without this, "ld -r" results in .text sections that are too big
> 	# (> 0x40000) for branches to reach stubs.
> 	ifndef CONFIG_FUNCTION_TRACER
> 	  cflags-y	+= -ffunction-sections
> 	endif

Thus, the .text.* sections must be coming from .S files, or possibly
from dwarf2 debug output.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: kernel segv with 2.6.31-rc6 ?

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 876 bytes --]

On Thursday 20 August 2009 08:25:37 Helge Deller wrote:
> The reason seems to be, that something in the newer gcc compilers changed
> to generate multiple sections all named ".text" for the PCREL17
> relocations. Older compilers named those sections ".text.1", ".text.2",
> ".text.3" and so forth.

we saw this on Blackfin some time ago and it was tracked back to attribute mismatches in
assembly files:
http://blackfin.uclinux.org/gf/tracker/3638
http://blackfin.uclinux.org/gf/project/linux-kernel/scmsvn/?action=browse&path=/trunk/arch/blackfin/mach-
common/entry.S&r1=3898&r2=3897&pathrev=3898

when `ld -r` ran, the sections named .text couldnt be combined due to different attributes so ld just added the .# 
suffix for us

if you have scanelf installed (from pax-utils), that might help track back the source object ... `scanelf -qrk .text.1 ./`
-mike

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

Re: kernel segv with 2.6.31-rc6 ?

[Rusty Russell]

On Wed, 19 Aug 2009 09:39:24 am James Bottomley wrote:
> Even with the duplicate name, though, the module should be perfectly
> loadable.

In a perfect world, yes, but there are places which assume they'll be
unique and I always thought that reasonable.

Front of my mind is /sys/module/<MODNAME/sections/ which has one file
per section.

Let's figure out how it happened tho; I'd rather fail cleanly than break
subtly and horribly later...

Thanks,
Rusty.

Re: kernel segv with 2.6.31-rc6 ?

[Helge Deller]

* Rusty Russell <rusty@rustcorp.com.au>:
> On Wed, 19 Aug 2009 09:39:24 am James Bottomley wrote:
> > Even with the duplicate name, though, the module should be perfectly
> > loadable.
> 
> In a perfect world, yes, but there are places which assume they'll be
> unique and I always thought that reasonable.
> 
> Front of my mind is /sys/module/<MODNAME/sections/ which has one file
> per section.

I just noticed, that on parisc the duplicate sections do have a
size of "0 bytes". Objdump shows that:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  1 .text         00000000  00000000  00000000  00000058  2**0     
                  CONTENTS, ALLOC, LOAD, READONLY, CODE            
  5 .text         00000000  00000000  00000000  000000d4  2**0       
                  CONTENTS, ALLOC, LOAD, READONLY, CODE    

> Let's figure out how it happened tho; I'd rather fail cleanly than break
> subtly and horribly later...


Since the sections are empty anyway, I don't see any reason
why those section names need to be exported via sysfs.

Let's just drop them, which works for parisc (and should for any
other architecture too).

A patch is attached to
http://bugzilla.kernel.org/show_bug.cgi?id=14703

and here:


[PATCH] modules: don't export section names of empty sections via sysfs

This patch fixes a "Badness at fs/sysfs/dir.c:487" warning on parisc, where
duplicate section names in kernel modules are common.

Signed-off-by: Helge Deller <deller@gmx.de>
CC: rusty@rustcorp.com.au
CC: James.Bottomley@HansenPartnership.com
CC: roland@redhat.com
CC: dave@hiauly1.hia.nrc.ca

diff --git a/kernel/module.c b/kernel/module.c
index 8b7d880..5842a71 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1187,7 +1187,8 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
 
 	/* Count loaded sections and allocate structures */
 	for (i = 0; i < nsect; i++)
-		if (sechdrs[i].sh_flags & SHF_ALLOC)
+		if (sechdrs[i].sh_flags & SHF_ALLOC
+		    && sechdrs[i].sh_size)
 			nloaded++;
 	size[0] = ALIGN(sizeof(*sect_attrs)
 			+ nloaded * sizeof(sect_attrs->attrs[0]),
@@ -1207,6 +1208,8 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
 	for (i = 0; i < nsect; i++) {
 		if (! (sechdrs[i].sh_flags & SHF_ALLOC))
 			continue;
+		if (!sechdrs[i].sh_size)
+			continue;
 		sattr->address = sechdrs[i].sh_addr;
 		sattr->name = kstrdup(secstrings + sechdrs[i].sh_name,
 					GFP_KERNEL);

Re: kernel segv with 2.6.31-rc6 ?

[Roland McGrath]

> Since the sections are empty anyway, I don't see any reason
> why those section names need to be exported via sysfs.
> 
> Let's just drop them, which works for parisc (and should for any
> other architecture too).

That seems fine to me.


Thanks,
Roland

Re: kernel segv with 2.6.31-rc6 ?

[Helge Deller]

> The root cause is a duplicate section name (.text); is this legal?
> 
> However, there's a problem with commit
> 6d76013381ed28979cd122eb4b249a88b5e384fa in that if you fail to allocate
> a mod->sect_attrs (in this case it's null because of the duplication),
> it still gets used without checking in add_notes_attrs()
> 
> This should fix it
> 
> Signed-off-by: James Bottomley <James.Bottomley@suse.de>


Thanks!
I tested it, and it does at least fix the kernel crash.

Tested-by: Helge Deller <deller@gmx.de>


 
> diff --git a/kernel/module.c b/kernel/module.c
> index fd14114..a703c49 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2353,7 +2353,8 @@ static noinline struct module *load_module(void
> __user *umod,
>  	if (err < 0)
>  		goto unlink;
>  	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
> -	add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
> +	if (mod->sect_attrs)
> +		add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
>  
>  	/* Get rid of temporary copy */
>  	vfree(hdr);
> 

-- 
Jetzt kostenlos herunterladen: Internet Explorer 8 und Mozilla Firefox 3 -
sicherer, schneller und einfacher! http://portal.gmx.net/de/go/atbrowser