Re: [RFA/PATCH] Fix recognition of NT_PRXFREG notes

[Mark Kettenis <kettenis at chello dot nl>]

   Date: Fri, 5 Jul 2002 11:00:29 +0930
   From: Alan Modra <amodra@bigpond.net.au>

   On Thu, Jul 04, 2002 at 05:03:22PM +0200, Mark Kettenis wrote:
   > Index: elf.c
   > ===================================================================
   > RCS file: /cvs/src/src/bfd/elf.c,v
   > retrieving revision 1.152
   > diff -u -p -r1.152 elf.c
   > --- elf.c 4 Jul 2002 13:26:30 -0000 1.152
   > +++ elf.c 4 Jul 2002 14:54:36 -0000
   > @@ -6718,8 +6718,7 @@ elfcore_grok_note (abfd, note)
   >  #endif
   >  
   >      case NT_PRXFPREG:		/* Linux SSE extension */
   > -      if (note->namesz == 5
   > -	  && ! strcmp (note->namedata, "LINUX"))
   > +      if (strncmp (note->namedata, "LINUX", 5) == 0)
   >  	return elfcore_grok_prxfpreg (abfd, note);
   >        else
   >  	return true;

   Don't the alignment rules require that the name be padded out to a
   multiple of 4 chars? (or 8 on 64 bit ELF files).  So you should have
   'L','I','N','U','X','\0','\0','\0' and thus can use strcmp.  Also,
   it's a good idea to check namesz before accessing namedata.  I'm
   sure I can make your strncmp segfault by carefully crafting a
   non-compliant note.

Hmm, I modelled my code after the bits that check for "NetBSD-CORE"
notes.  Therefore I'm pretty sure you won't crash on *my* strncmp ;-).

That said, the current code isn't really robust.  There is no place
where we check wheter namesz and descsz actually make any sense.  This
is addressed in the attached revision of my patch.

   Hmm, on re-reading the ELF standard, I see the pad char isn't
   specified.  :-(  But obviously the original strcmp worked, so the
   pad from the kernel is zero.  Please use

	 if (note->namesz >= 5
	     && strcmp (note->namedata, "LINUX") == 0)

Good news, Linus included my patch to make the Linux kernel emit
proper note entries in 2.5.25.  I'll see if I can get it into 2.4.x
too.  I also learned that the kernel patch that makes Linux write out
the SSE register into a note section isn't widely used (I knew it
wasn't in the standard kernel).  Therefore I think it's cleaner to
forget backwards compatibility with these broken notes and just use

      if (note->namesz == 6
	  && strcmp (note->namedata, "LINUX") == 0)

Unfortunately this patch is getting too long for inclusion without a
copyright assignment, and that's still not arranged (for BFD) as a
result of my own lameness :-(.

Is it OK if I just change the two lines mentioned above, and leave the
rest of this stuff on the backburner?

Mark

Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.152
diff -u -p -r1.152 elf.c
--- elf.c 4 Jul 2002 13:26:30 -0000 1.152
+++ elf.c 8 Jul 2002 21:35:40 -0000
@@ -6718,8 +6718,8 @@ elfcore_grok_note (abfd, note)
 #endif
 
     case NT_PRXFPREG:		/* Linux SSE extension */
-      if (note->namesz == 5
-	  && ! strcmp (note->namedata, "LINUX"))
+      if (note->namesz == 6
+	  && strcmp (note->namedata, "LINUX") == 0)
 	return elfcore_grok_prxfpreg (abfd, note);
       else
 	return true;
@@ -7071,11 +7071,17 @@ elfcore_read_notes (abfd, offset, size)
       in.namesz = H_GET_32 (abfd, xnp->namesz);
       in.namedata = xnp->name;
 
+      if (in.namedata + in.namesz >= buf + size)
+	goto error;
+
       in.descsz = H_GET_32 (abfd, xnp->descsz);
       in.descdata = in.namedata + BFD_ALIGN (in.namesz, 4);
       in.descpos = offset + (in.descdata - buf);
 
-      if (strncmp (in.namedata, "NetBSD-CORE", 11) == 0)
+      if (in.descdata + in.descsz >= buf + size)
+	goto error;
+
+      if (in.namesz == 12 && strcmp (in.namedata, "NetBSD-CORE") == 0)
         {
           if (! elfcore_grok_netbsd_note (abfd, &in))
             goto error;