* Re: patch for readelf - dumping NOTE sections
@ 1999-11-25 3:06 Nick Clifton
1999-11-25 8:26 ` fnf
0 siblings, 1 reply; 3+ messages in thread
From: Nick Clifton @ 1999-11-25 3:06 UTC (permalink / raw)
To: fnf; +Cc: binutils, fnf
Hi Fred,
: + 1999-11-24 Fred Fish <fnf@cygnus.com>
: +
: + * readelf.c (process_note): Change arg from Elf_External_Note
: + to Elf32_Internal_Note, which also turns the function body
: + into little more than a call to printf.
: + (process_corefile_note_segment): Substantially rewritten
: + to properly handle case where target and host are different
: + endianness, handle note sections with padding, and add some
: + cruft to handle notes with unterminated name data.
: +
Thanks for this patch. I do however have a few comments:
* The normal convention is just submit the ChangeLog entry as
straight text, not a context diff. This is because ChangeLogs
change so rapidly that it is often impossible to apply a ChangeLog
patch automatically.
* Your patch contained a potential memory leak, in that if the
malloc of temp failed the code would return without freeing
pnotes. I have fixed this.
* The was also a potential memory corruption bug in that you used
strcpy() to copy the un-null-terminated name string into temp.
This could easily extend over the end of the memory allocated for
temp. I have fixed this by using strncpy() to copy the name and
then forcing the last byte of the array to zero.
Apart from that the patch was fine and I have gone ahead and checked
it in.
Cheers
Nick
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: patch for readelf - dumping NOTE sections
1999-11-25 3:06 patch for readelf - dumping NOTE sections Nick Clifton
@ 1999-11-25 8:26 ` fnf
0 siblings, 0 replies; 3+ messages in thread
From: fnf @ 1999-11-25 8:26 UTC (permalink / raw)
To: Nick Clifton; +Cc: fnf, binutils, fnf
> * The normal convention is just submit the ChangeLog entry as
> straight text, not a context diff.
Yup, I neglected to do that, even though I already knew that.
> * Your patch contained a potential memory leak ...
> * The was also a potential memory corruption bug ...
Thanks. Those were partly due to my rushed fix once I discovered the
problem with unterminated names on Linux. I should have taken more
time to review that last minute change.
Sorry about the sloppy work. :-(
-Fred
^ permalink raw reply [flat|nested] 3+ messages in thread
* patch for readelf - dumping NOTE sections
@ 1999-11-24 15:01 fnf
0 siblings, 0 replies; 3+ messages in thread
From: fnf @ 1999-11-24 15:01 UTC (permalink / raw)
To: binutils; +Cc: fnf
While attempting to dump a corefile created on a big endian machine
using a copy of readelf compiled on a little endian host, I ran into a
problem where readelf would try to malloc a huge chunk of memory, and
when that failed, eventually coredump.
I believe this patch fixes the endianness problem, as well as a couple
other minor nits I noticed. One of the nits is that the namesz and
descsz fields do not include the size of any padding needed to align
the note descriptors and also align the next note entry.
While investigating this problem, I discovered that core files
generated on Linux appear to violate the ELF standard that I have, by
not properly null terminating the name field and including the null
byte in the namesz field. This patch still accomodates that, but the
warning to diagnose the problem is currently commented out.
-Fred
Index: ChangeLog
===================================================================
RCS file: /cvs/binutils/binutils/binutils/ChangeLog,v
retrieving revision 1.89
diff -c -r1.89 ChangeLog
*** ChangeLog 1999/11/22 09:42:42 1.89
--- ChangeLog 1999/11/24 22:50:08
***************
*** 1,3 ****
--- 1,13 ----
+ 1999-11-24 Fred Fish <fnf@cygnus.com>
+
+ * readelf.c (process_note): Change arg from Elf_External_Note
+ to Elf32_Internal_Note, which also turns the function body
+ into little more than a call to printf.
+ (process_corefile_note_segment): Substantially rewritten
+ to properly handle case where target and host are different
+ endianness, handle note sections with padding, and add some
+ cruft to handle notes with unterminated name data.
+
1999-11-22 Nick Clifton <nickc@cygnus.com>
* objcopy.c (copy_usage): Reformat.
Index: readelf.c
===================================================================
RCS file: /cvs/binutils/binutils/binutils/readelf.c,v
retrieving revision 1.28
diff -c -r1.28 readelf.c
*** readelf.c 1999/10/04 18:59:13 1.28
--- readelf.c 1999/11/24 22:50:59
***************
*** 200,206 ****
static const char * get_osabi_name PARAMS ((unsigned char));
static int guess_is_rela PARAMS ((unsigned long));
static char * get_note_type PARAMS ((unsigned int));
! static int process_note PARAMS ((Elf_External_Note *));
static int process_corefile_note_segment PARAMS ((FILE *, unsigned long, unsigned long));
static int process_corefile_note_segments PARAMS ((FILE *));
static int process_corefile_contents PARAMS ((FILE *));
--- 200,206 ----
static const char * get_osabi_name PARAMS ((unsigned char));
static int guess_is_rela PARAMS ((unsigned long));
static char * get_note_type PARAMS ((unsigned int));
! static int process_note PARAMS ((Elf32_Internal_Note *));
static int process_corefile_note_segment PARAMS ((FILE *, unsigned long, unsigned long));
static int process_corefile_note_segments PARAMS ((FILE *));
static int process_corefile_contents PARAMS ((FILE *));
***************
*** 6605,6634 ****
}
}
static int
process_note (pnote)
! Elf_External_Note * pnote;
{
- Elf32_Internal_Note * internal;
- char * pname;
-
- internal = (Elf32_Internal_Note *) pnote;
- pname = malloc (internal->namesz + 1);
-
- if (pname == NULL)
- {
- error (_("Out of memory\n"));
- return 0;
- }
-
- memcpy (pname, pnote->name, internal->namesz);
- pname[internal->namesz] = '\0';
-
printf (" %s\t\t0x%08lx\t%s\n",
! pname, internal->descsz, get_note_type (internal->type));
!
! free (pname);
!
return 1;
}
--- 6605,6623 ----
}
}
+ /* Note that by the ELF standard, the name field is already null byte
+ terminated, and namesz includes the terminating null byte.
+ I.E. the value of namesz for the name "FSF" is 4.
+
+ If the value of namesz is zero, there is no name present. */
+
static int
process_note (pnote)
! Elf32_Internal_Note * pnote;
{
printf (" %s\t\t0x%08lx\t%s\n",
! pnote->namesz ? pnote -> namedata : "(NONE)",
! pnote->descsz, get_note_type (pnote->type));
return 1;
}
***************
*** 6640,6672 ****
{
Elf_External_Note * pnotes;
Elf_External_Note * external;
- Elf32_Internal_Note* internal;
- unsigned int notesz;
- unsigned int nlength;
- unsigned char * p;
int res = 1;
if (length <= 0)
return 0;
GET_DATA_ALLOC (offset, length, pnotes, Elf_External_Note *, "notes");
-
external = pnotes;
- p = (unsigned char *) pnotes;
- nlength = length;
printf (_("\nNotes at offset 0x%08lx with length 0x%08lx:\n"), offset, length);
printf (_(" Owner\t\tData size\tDescription\n"));
! while (nlength > 0)
{
! res &= process_note (external);
! internal = (Elf32_Internal_Note *) p;
! notesz = 3 * sizeof(unsigned long) + internal->namesz + internal->descsz;
! nlength -= notesz;
! p += notesz;
! external = (Elf_External_Note *) p;
}
free (pnotes);
--- 6629,6682 ----
{
Elf_External_Note * pnotes;
Elf_External_Note * external;
int res = 1;
if (length <= 0)
return 0;
GET_DATA_ALLOC (offset, length, pnotes, Elf_External_Note *, "notes");
external = pnotes;
printf (_("\nNotes at offset 0x%08lx with length 0x%08lx:\n"), offset, length);
printf (_(" Owner\t\tData size\tDescription\n"));
! while (external < ((char *) pnotes + length))
{
! Elf32_Internal_Note inote;
! char *temp = NULL;
! inote.type = BYTE_GET (external->type);
! inote.namesz = BYTE_GET (external->namesz);
! inote.namedata = external->name;
! inote.descsz = BYTE_GET (external->descsz);
! inote.descdata = inote.namedata + align_power (inote.namesz, 2);
! inote.descpos = offset + (inote.descdata - (char *) pnotes);
! external = inote.descdata + align_power (inote.descsz, 2);
!
! /* Verify that name is null terminated. It appears that at least
! one version of Linux (RedHat 6.0) generates corefiles that don't
! comply with the ELF spec by failing to include the null byte in
! namesz. */
! if (inote.namedata[inote.namesz] != '\0')
! {
! temp = malloc (inote.namesz + 1);
! if (temp == NULL)
! {
! error (_("Out of memory\n"));
! return (0);
! }
! strcpy (temp, inote.namedata);
! /* warn (_("'%s' NOTE name not properly null terminated\n"), temp); */
! inote.namedata = temp;
! }
!
! res &= process_note (&inote);
!
! if (temp != NULL)
! {
! free (temp);
! temp = NULL;
! }
}
free (pnotes);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~1999-11-25 8:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-11-25 3:06 patch for readelf - dumping NOTE sections Nick Clifton
1999-11-25 8:26 ` fnf
-- strict thread matches above, loose matches on Subject: below --
1999-11-24 15:01 fnf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).