This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

SREC reader tweaks


Last year, I needed to back up a firmware image over a serial
connection using RedBoot.  I found two problems with the SREC
parser.

One was that we completely ignore checksums.  Serial lines are not
necessarily reliable.  So I felt that if I wanted to trust my backup,
and the format already had a checksum in it, bfd ought to let me know
if the file was corrupt.

The other was that the memory-dump-as-srec did not include a
termination record.  The srec reader was written to expect
bfd_error_file_truncated at the end of the file and nothing
set it.  I decided that cache_bread was a sensible place
to leave a truncation marker, since the read routine had
apparently done this at some point in BFD history.  As you
can see from the comments just above the bit in my patch:

  nread = fread (buf, 1, nbytes, f);
  /* Set bfd_error if we did not read as much data as we expected.  If
     the read failed due to an error set the bfd_error_system_call,
     else set bfd_error_file_truncated.  */
  if (nread < nbytes && ferror (f))
    {
      bfd_set_error (bfd_error_system_call);
      return -1;
    }

But nothing actually sets bfd_error_file_truncated.

OK to commit?

-- 
Daniel Jacobowitz
CodeSourcery

2008-02-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* cache.c (cache_bread): Set bfd_error_file_truncated if EOF
	was reached.
	* srec.c (srec_scan): Calculate the checksum.  Complain on mismatch.

Index: cache.c
===================================================================
RCS file: /cvs/src/src/bfd/cache.c,v
retrieving revision 1.30
diff -u -p -r1.30 cache.c
--- cache.c	3 Jul 2007 14:26:39 -0000	1.30
+++ cache.c	6 Aug 2007 18:25:24 -0000
@@ -309,6 +309,10 @@ cache_bread (struct bfd *abfd, void *buf
       return -1;
     }
 #endif
+  if (nread < nbytes)
+    /* This may or may not be an error, but in case the calling code
+       bails out because of it, set the right error code.  */
+    bfd_set_error (bfd_error_file_truncated);
   return nread;
 }
 
Index: srec.c
===================================================================
RCS file: /cvs/src/src/bfd/srec.c,v
retrieving revision 1.45
diff -u -p -r1.45 srec.c
--- srec.c	26 Jul 2007 18:45:13 -0000	1.45
+++ srec.c	6 Aug 2007 18:25:24 -0000
@@ -458,6 +458,7 @@ srec_scan (bfd *abfd)
 	    unsigned int bytes;
 	    bfd_vma address;
 	    bfd_byte *data;
+	    unsigned char check_sum;
 
 	    /* Starting an S-record.  */
 
@@ -476,7 +477,7 @@ srec_scan (bfd *abfd)
 		goto error_return;
 	      }
 
-	    bytes = HEX (hdr + 1);
+	    check_sum = bytes = HEX (hdr + 1);
 	    if (bytes * 2 > bufsize)
 	      {
 		if (buf != NULL)
@@ -505,18 +506,22 @@ srec_scan (bfd *abfd)
 		break;
 
 	      case '3':
+		check_sum += HEX (data);
 		address = HEX (data);
 		data += 2;
 		--bytes;
 		/* Fall through.  */
 	      case '2':
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
 		--bytes;
 		/* Fall through.  */
 	      case '1':
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
 		bytes -= 2;
@@ -548,25 +553,56 @@ srec_scan (bfd *abfd)
 		    sec->size = bytes;
 		    sec->filepos = pos;
 		  }
+
+		while (bytes > 0)
+		  {
+		    check_sum += HEX (data);
+		    data += 2;
+		    bytes--;
+		  }
+		check_sum = 255 - (check_sum & 0xff);
+		if (check_sum != HEX (data))
+		  {
+		    (*_bfd_error_handler)
+		      (_("%B:%d: Bad checksum in S-record file\n"),
+		       abfd, lineno);
+		    bfd_set_error (bfd_error_bad_value);
+		    goto error_return;
+		  }
+
 		break;
 
 	      case '7':
+		check_sum += HEX (data);
 		address = HEX (data);
 		data += 2;
 		/* Fall through.  */
 	      case '8':
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
 		/* Fall through.  */
 	      case '9':
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
+		check_sum += HEX (data);
 		address = (address << 8) | HEX (data);
 		data += 2;
 
 		/* This is a termination record.  */
 		abfd->start_address = address;
 
+		check_sum = 255 - (check_sum & 0xff);
+		if (check_sum != HEX (data))
+		  {
+		    (*_bfd_error_handler)
+		      (_("%B:%d: Bad checksum in S-record file\n"),
+		       abfd, lineno);
+		    bfd_set_error (bfd_error_bad_value);
+		    goto error_return;
+		  }
+
 		if (buf != NULL)
 		  free (buf);
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]