Bug 140954 - bad elf check in module-verify.c
Summary: bad elf check in module-verify.c
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 3
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Howells
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On:
Blocks: 167126
TreeView+ depends on / blocked
 
Reported: 2004-11-26 22:46 UTC by Yannick Heneault
Modified: 2007-11-30 22:10 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2005-08-30 19:06:14 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Yannick Heneault 2004-11-26 22:46:59 UTC
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (compatible; Konqueror/3.3; Linux) (KHTML, like Gecko)

Description of problem:
The patch linux-2.6.7-modsign-core.patch that is part of the kernel srpms contain a false check. At line 286 in kernel/module-verify.c:

relcheck(ELF_R_SYM(rel->r_info) > 0);

introduce a false check. According to the elf specification, it is possible the set the symbol index of the relocation table to 0. 

We are in a situation were our gcc toolchains generates this case. This prevent us to modprobe our module on a system with fc3 installed.

If someone can remove this line for the next update it will be appreciated. In fact, the loader of the kernel is already able to deal with this case (=0), so there is no need for this check.



Version-Release number of selected component (if applicable):


How reproducible:
Always

Steps to Reproduce:
1.
2.
3.
    

Additional info:

Comment 1 David Howells 2004-11-29 14:42:02 UTC
Out of interest, what does that relocate against? The zeroth element is meant 
to be a "NULL" symbol isn't it? 
 
I'm not saying you're wrong, but how do you deal with it? What does it mean? I 
wonder if the kernel's relocator will get it right... 
 
David 

Comment 2 David Howells 2004-11-29 14:51:00 UTC
OIC... When calculating a relocation, symbol #0 is explicitly defined to be 
zero by the standard. 

Comment 3 Yannick Heneault 2004-11-29 15:03:00 UTC
Any good elf loader, like the one in the kernel, will simply ignore them. These  
are useless relocation unit that are kept by the linker. In our case, it  
happens when we inline a function in multiple .o files. The linker will keep  
only one copy of the code by selecting one the of the object files and will use  
its sections if the inline functions need relocation (like a reference to  
the .rodata segment by a switch statement). In the other .o files that declared  
the inlined function, the relocation units are conserved but are not used  
anymore since they are not referenced, (i.e. the function was removed).  

Comment 5 Jason 2004-12-28 16:23:36 UTC
Has this been fixed in any of the errata kernels?

Comment 6 Jason Watson 2005-01-07 21:46:10 UTC
(In reply to comment #5)
> Has this been fixed in any of the errata kernels?

Looks like it is fixed in kernel-2.6.9-1.724_FC3.src.rpm

diff kernel-2.6.9-1.681_FC3.src/linux-2.6.7-modsign-core.patch
kernel-2.6.9-1.724_FC3.src/linux-2.6.7-modsign-core.patch
114c114
< @@ -0,0 +1,341 @@
---
> @@ -0,0 +1,340 @@
400d399
< +                             relcheck(ELF_R_SYM(rel->r_info) > 0);


Comment 7 Adam Goode 2005-08-27 02:24:58 UTC
There seems to be a new bug here, possibly/probably triggered by the new
glibc-2.3.5-10.3.

I've been getting "Verify ELF error (assertion 110)" on all modules compiled
recently, on i386 and x86_64. This results in rpmbuild --rebuild
kernel...src.rpm producing an unusable kernel.

Lines 109--110 of module-verify.c is:

	tmp = (size_t) hdr->e_shentsize * (size_t) hdr->e_shnum;
	elfcheck(tmp < size - hdr->e_shoff);


Shouldn't that be 

	elfcheck(tmp <= size - hdr->e_shoff);
                    ~~~~
instead?

With readelf -h on a module compiled a few days ago, I get this:

  Start of section headers:          455448 (bytes into file)
  Size of section headers:           40 (bytes)
  Number of section headers:         25
(file size 665312)

Section headers (40*25=1000 bytes) < file size (665312) - section header offset
(455448 bytes), and it verifies.



Here is the same module newly compiled:

  Start of section headers:          667532 (bytes into file)
  Size of section headers:           40 (bytes)
  Number of section headers:         25
(file size 668532)

Section headers (40*25=1000 bytes) = file size (668532) - section header offset
(667532 bytes), which should be valid, but isn't, since less than is used in the
kernel. This should be less than or equal to.


Note You need to log in before you can comment on or make changes to this bug.