Bug 12161 - Unconforming ELF file causes SIGSEGV in avr-ld
Summary: Unconforming ELF file causes SIGSEGV in avr-ld
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.18
: P2 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
: 13612 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-10-25 18:51 UTC by meiermic
Modified: 2014-05-28 19:45 UTC (History)
5 users (show)

See Also:
Host:
Target: avr
Build:
Last reconfirmed:


Attachments
ELF file which causes avr-ld to crash (185 bytes, application/octet-stream)
2010-10-25 18:51 UTC, meiermic
Details
Delete elf32_avr_check_relocs function (883 bytes, patch)
2010-10-27 13:25 UTC, Nick Clifton
Details | Diff
Add extra check for bad reloc. (1.30 KB, patch)
2010-11-01 16:22 UTC, Nick Clifton
Details | Diff
Load relocs if necessary (517 bytes, patch)
2012-01-04 17:08 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description meiermic 2010-10-25 18:51:57 UTC
Created attachment 5085 [details]
ELF file which causes avr-ld to crash

The attached ELF file causes avr-ld to crash:

$ avr-ld govrout.o 
Segmentation fault



The ELF file is not a valid ELF file, the .e_info field of the .symtab section header has the value 1, though the last local symbol is at index 1 and thus the .e_info should be 2.

I think ld should at least print an error message instead of crashing.



I am running avr-ld on Linux, see below:

$ avr-ld --version
GNU ld (GNU Binutils) 2.18.0.20080103 (WinAVR 20080610)

(that is the Debian 5.0 package)

$ uname -a
Linux thecomputer 2.6.26-2-amd64 #1 SMP Thu Sep 16 15:56:38 UTC 2010 x86_64 GNU/Linux
Comment 1 Nick Clifton 2010-10-27 13:25:57 UTC
Created attachment 5090 [details]
Delete elf32_avr_check_relocs function
Comment 2 Nick Clifton 2010-10-27 13:27:36 UTC
Hi,

  Please could you try out the uploaded patch ?

  It seems that the bug is in the elf32_avr_check_relocs() function, but since this function does not actually do anything there is no reason to actually keep it in the sources.

Cheers
  Nick
Comment 3 meiermic 2010-10-31 18:16:44 UTC
Hi,

>   Please could you try out the uploaded patch ?

I applied it to the 2.18 sources, the problem still persists. I had trouble getting the Debian sources, but I can get that another shot.
Comment 4 Nick Clifton 2010-11-01 16:22:48 UTC
Created attachment 5102 [details]
Add extra check for bad reloc.
Comment 5 Nick Clifton 2010-11-01 16:24:48 UTC
Hi,

> I applied it to the 2.18 sources, the problem still persists.

My bad - I was using the current mainline binutils sources.  The old 2.18 sources have two places that need to be fixed, so please try out the newly uploaded "extra" patch.  (Despite its name, this patch should be applied instead of the delete elf32_avr_check_relocs patch).

Cheers
  Nick
Comment 6 Eric Weddington 2010-11-01 19:28:28 UTC
(In reply to comment #0)
> Created attachment 5085 [details]
> ELF file which causes avr-ld to crash
> 
> The attached ELF file causes avr-ld to crash:
> 
> $ avr-ld govrout.o 
> Segmentation fault

Typically, avr-ld is not called directly, but through the driver (avr-gcc). Plus you must also specify the device on the linker command line: -mmcu=<device>.

Do you still get a crash when you do this?

Eric Weddington
Atmel
Comment 7 Nick Clifton 2010-11-17 10:56:37 UTC
Hi Guys,

  I have decided to go ahead and check this patch in.  Please reopen this PR if the problem resurfaces.

Cheers
  Nick
Comment 8 Pitchumani 2012-01-04 12:49:51 UTC
(In reply to comment #7)
> Hi Guys,
> 
>   I have decided to go ahead and check this patch in.  Please reopen this PR if
> the problem resurfaces.
> 
> Cheers
>   Nick

Hi Nick,

Function 'elf32_avr_check_relocs(elf32-avr.c)' is referred in elflink.c:4801

(-- snip from elflink.c --)
 4799   if (! dynamic
 4800       && is_elf_hash_table (htab)
 4801       && bed->check_relocs != NULL
 4802       && elf_object_id (abfd) == elf_hash_table_id (htab)
 4803       && (*bed->relocs_compatible) (abfd->xvec, info->output_bfd->xvec))
(-- snip end --)

As 'bed->check_relocs' is NULL in 2.22, code that follows this check not executed.
This caused a segmentation fault (NULL pointer access) in elf32-avr.c:1528.

Test case:
-- c source --
void func2(int a) { }

void func1(void)
{
  func2(10);
}

int main(void)
{
  func1();
}
-- c source --

command line options:
avr-gcc segfault.c -ffunction-sections -mmcu=atmega128 -c -o segfault.o
avr-ld segfault.o -m avr51 -relax

Please correct if I am missing something.

Thanks,
Pitchumani
Comment 9 Nick Clifton 2012-01-04 17:08:05 UTC
Created attachment 6145 [details]
Load relocs if necessary
Comment 10 Nick Clifton 2012-01-04 17:09:07 UTC
Hi Pitchumani,

  Please try out the uploaded patch ("Load relocs if necessary") and let me know if you have any problems with it.

Cheers
  Nick
Comment 11 Pitchumani 2012-01-05 07:21:02 UTC
(In reply to comment #10)
> Hi Pitchumani,
> 
>   Please try out the uploaded patch ("Load relocs if necessary") and let me
> know if you have any problems with it.
> 
> Cheers
>   Nick

This change solves the issue.

Thanks
Pitchumani
Comment 12 Sourceware Commits 2012-01-05 09:57:22 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2012-01-05 09:57:18

Modified files:
	bfd            : ChangeLog elf32-avr.c 

Log message:
	PR ld/12161
	* elf32-avr.c (elf32_avr_relax_delete_bytes): Read in relocs if
	necessary.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5572&r2=1.5573
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-avr.c.diff?cvsroot=src&r1=1.51&r2=1.52
Comment 13 Nick Clifton 2012-01-05 10:08:28 UTC
Patch committed,
Comment 14 Georg-Johann Lay 2012-01-22 14:31:25 UTC
*** Bug 13612 has been marked as a duplicate of this bug. ***
Comment 15 Georg-Johann Lay 2012-01-29 18:53:37 UTC
(In reply to comment #13)
> Patch committed,

Hi Nick,

I don't quite get it from the ChangeLogs... Is this only a fix for HEAD or will it be backported to 2.22, too?
Comment 16 Nick Clifton 2012-01-30 11:41:07 UTC
Hi Georg-Johann,

> Is this only a fix for HEAD or will it be backported to 2.22, too?

It was only checked in to HEAD, although the patch will work with the 2.22 sources as well.  I have now checked it in to the 2.22 branch. so if there is a 2.22 point release then the patch will be included in that.

Cheers
  Nick
Comment 17 Georg-Johann Lay 2012-01-30 12:42:09 UTC
(In reply to comment #16)
> Hi Georg-Johann,
> 
> > Is this only a fix for HEAD or will it be backported to 2.22, too?
> 
> It was only checked in to HEAD, although the patch will work with the 2.22
> sources as well.  I have now checked it in to the 2.22 branch. so if there is a
> 2.22 point release then the patch will be included in that.

Thanks for taking care of the matter and for the quick response.

If there is the decision to have no more 2.22 release and thus your work won't be integrated in any 2.22 release, then the release notes in
http://sourceware.org/binutils/
should mention that avr-binutils are broken for target avr and unusable.

Even if avr is no "important" target it would be great to have a working 2.22 avr-binutils and not a version that's completely broken...
Comment 18 Sourceware Commits 2012-02-01 17:07:45 UTC
CVSROOT:	/cvs/src
Module name:	src
Branch: 	binutils-2_22-branch
Changes by:	nickc@sourceware.org	2012-02-01 17:07:37

Modified files:
	bfd            : ChangeLog elf32-avr.c 

Log message:
	* Import this patch from the mainline:
	2012-01-05  Nick Clifton  <nickc@redhat.com>
	
	PR ld/12161
	* elf32-avr.c (elf32_avr_relax_delete_bytes): Read in relocs if
	necessary.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&only_with_tag=binutils-2_22-branch&r1=1.5473.2.35&r2=1.5473.2.36
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-avr.c.diff?cvsroot=src&only_with_tag=binutils-2_22-branch&r1=1.51&r2=1.51.2.1
Comment 19 Jackie Rosen 2014-02-16 17:46:44 UTC Comment hidden (spam)