diff mbox

[libgfortran] PR67367 Program crashes on READ

Message ID 55E14093.1070407@charter.net
State New
Headers show

Commit Message

Jerry DeLisle Aug. 29, 2015, 5:18 a.m. UTC
I found that in read_buf where raw_read is called, no checks for errors were
being made,  raw_read returns the number of bytes read or an error code.  In the
test case, an error occurs and we proceeded to use the resulting error code as
if it were the number of bytes read.

The attached patch fixes this.  Regression tested on x86_64.  New test case
provided.

OK for trunk?

Regards,

Jerry

2015-08-28 Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/67367
	* io/unix.c (buf_read): Check for error condition and if found
	return the error code.

Comments

FX Aug. 29, 2015, 6:59 a.m. UTC | #1
Hi Jerry,

The patch is OK, but I’m a bit puzzled about what the testcase does.

It tests that we can OPEN a directory, but not READ from it? I didn’t know that was expected (to be able to OPEN a directory), and I find it somewhat puzzling. Can you shed light on that?

Thanks,
FX
Jerry DeLisle Aug. 29, 2015, 3:36 p.m. UTC | #2
On 08/28/2015 11:59 PM, FX wrote:
> Hi Jerry,
> 
> The patch is OK, but I’m a bit puzzled about what the testcase does.
> 
> It tests that we can OPEN a directory, but not READ from it? I didn’t know that was expected (to be able to OPEN a directory), and I find it somewhat puzzling. Can you shed light on that?
> 
> Thanks,
> FX
> 

According to my man pages, open is used to open a directory as read only and
readdir must be used to actually read it.  It is not allowed to open a directory
with write access.  The test case is checking that the open works as expected
and the attempt to read using 'read' results in the error (no segfault)

Jerry

PS thanks for the review.
Jerry DeLisle Aug. 29, 2015, 3:56 p.m. UTC | #3
On 08/28/2015 10:18 PM, Jerry DeLisle wrote:
> I found that in read_buf where raw_read is called, no checks for errors were
> being made,  raw_read returns the number of bytes read or an error code.  In the
> test case, an error occurs and we proceeded to use the resulting error code as
> if it were the number of bytes read.
> 
> The attached patch fixes this.  Regression tested on x86_64.  New test case
> provided.
> 
> OK for trunk?
> 

Committed.  I should mention, the test case may fail on some platforms since the
system call may be dependent. I will xfail those as we learn which. Test case
committed revision 227321.

Regards,

Jerry
diff mbox

Patch

Index: unix.c
===================================================================
--- unix.c	(revision 227314)
+++ unix.c	(working copy)
@@ -529,16 +529,26 @@  buf_read (unix_stream * s, void * buf, ssize_t nby
       if (to_read <= BUFFER_SIZE/2)
         {
           did_read = raw_read (s, s->buffer, BUFFER_SIZE);
-          s->physical_offset += did_read;
-          s->active = did_read;
-          did_read = (did_read > to_read) ? to_read : did_read;
-          memcpy (p, s->buffer, did_read);
+	  if (likely (did_read >= 0))
+	    {
+	      s->physical_offset += did_read;
+	      s->active = did_read;
+	      did_read = (did_read > to_read) ? to_read : did_read;
+	      memcpy (p, s->buffer, did_read);
+	    }
+	  else
+	    return did_read;
         }
       else
         {
           did_read = raw_read (s, p, to_read);
-          s->physical_offset += did_read;
-          s->active = 0;
+	  if (likely (did_read >= 0))
+	    {
+	      s->physical_offset += did_read;
+	      s->active = 0;
+	    }
+	  else
+	    return did_read;
         }
       nbyte = did_read + nread;
     }