United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-4847367 : FREE_AND_RETURN_NULL target in readLoc() in zip_util.c is missing null check

Details
Type:
Bug
Submit Date:
2003-04-11
Status:
Resolved
Updated Date:
2003-08-15
Project Name:
JDK
Resolved Date:
2003-08-15
Component:
core-libs
OS:
generic
Sub-Component:
java.util.jar
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
1.4.2
Fixed Versions:
5.0 (tiger)

Related Reports
Backport:
Relates:

Sub Tasks

Description
The code for FREE_AND_RETURN_NULL is missing a null check when ze is 
never initialized.  It is currently dereferencing a null pointer to free one
of it's fields.  ###@###.###'s suggested fix for this problem and some
additional changes to address a potential Thread.interrupt during class loaded
are provided in the "Suggested Fix".

-- iag@sfbay 2003-04-11

                                    

Comments
SUGGESTED FIX

The following changes were suggested by ###@###.### during the
investigation of a potential fix for bug 4766057.

*** /tmp/geta18542      Fri Feb 21 10:55:43 2003
--- zip_util.c  Fri Feb 21 10:53:49 2003
***************
*** 70,76 ****
       while (len > 0) {
         jint n = JVM_Read(fd, (char *)bp, len);
         if (n <= 0) {
!           return -1;
         }
         bp += n;
         len -= n;
--- 70,80 ----
       while (len > 0) {
         jint n = JVM_Read(fd, (char *)bp, len);
         if (n <= 0) {
!           if (errno == EINTR) {
!               /* retry after EINTR */
!               continue;
!           }
!             return -1;
         }
         bp += n;
         len -= n;
***************
*** 771,777 ****
   {
       unsigned char *locbuf;
       jint nlen, elen;
!     jzentry *ze;
       jint start, end;

   #ifdef USE_MMAP
--- 775,781 ----
   {
       unsigned char *locbuf;
       jint nlen, elen;
!     jzentry *ze = NULL;
       jint start, end;

   #ifdef USE_MMAP
***************
*** 935,946 ****

    FREE_AND_RETURN_NULL:
   #ifndef USE_MMAP
!     if (ze->extra != NULL)
           free(ze->extra);
!     if (ze->name != NULL)
           free(ze->name);
!     if (ze != NULL)
!         free(ze);
       if (locbuf != NULL)
           free(locbuf);
   #endif
--- 939,951 ----

    FREE_AND_RETURN_NULL:
   #ifndef USE_MMAP
!     if (ze != NULL) {
!       if (ze->extra != NULL)
           free(ze->extra);
!       if (ze->name != NULL)
           free(ze->name);
!       free(ze);
!     }
       if (locbuf != NULL)
           free(locbuf);
   #endif
***************
*** 1106,1112 ****
       /* Seek to beginning of entry data and read bytes */
       n = jlong_to_jint(JVM_Lseek(zip->fd, entry->pos + pos, SEEK_SET));
       if (n != -1) {
!       n = JVM_Read(zip->fd, buf, len);
       }

       return n;
--- 1111,1121 ----
       /* Seek to beginning of entry data and read bytes */
       n = jlong_to_jint(JVM_Lseek(zip->fd, entry->pos + pos, SEEK_SET));
       if (n != -1) {
!         if (readFully(zip->fd, buf, len) == -1) {
!           n = -1;
!         } else {
!           n = len;
!         }
       }

       return n;

-- iag@sfbay 2003-04-11

The actual fix is similar, but not identical:
(see http://nio/rev/4847367/)

*** /u/martin/ws/env4/webrev/src/share/native/java/util/zip/zip_util.c- Mon Aug 11 13:20:11 2003
--- zip_util.c  Sun Aug 10 21:15:04 2003

*** 67,81 ****
  static jint readFully(jint fd, void *buf, jint len)
  {
      unsigned char *bp = buf;
      while (len > 0) {
        jint n = JVM_Read(fd, (char *)bp, len);
!       if (n <= 0) {
!           return -1;
!       }
        bp += n;
        len -= n;
      }
      return 0;
  }
  
  /*
--- 67,84 ----
  static jint readFully(jint fd, void *buf, jint len)
  {
      unsigned char *bp = buf;
      while (len > 0) {
        jint n = JVM_Read(fd, (char *)bp, len);
!       if (n > 0) {
            bp += n;
            len -= n;
+       } else if (n == JVM_IO_ERR && errno == EINTR) {
+           continue;   /* Retry after EINTR */
+       } else {        /* EOF or IO error */
+           return -1;
+       }
      }
      return 0;
  }
  
  /*

*** 770,780 ****
  static jzentry *
  readLOC(jzfile *zip, jzcell *zc)
  {
      unsigned char *locbuf;
      jint nlen, elen;
!     jzentry *ze;
      jint start, end;
  
  #ifdef USE_MMAP
      locbuf = zip->maddr + zc->pos;
  #else
--- 773,783 ----
  static jzentry *
  readLOC(jzfile *zip, jzcell *zc)
  {
      unsigned char *locbuf;
      jint nlen, elen;
!     jzentry *ze = NULL;
      jint start, end;
  
  #ifdef USE_MMAP
      locbuf = zip->maddr + zc->pos;
  #else

*** 934,949 ****
  #endif
      return ze;
  
   FREE_AND_RETURN_NULL:
  #ifndef USE_MMAP
      if (ze->extra != NULL)
          free(ze->extra);
      if (ze->name != NULL)
          free(ze->name);
-     if (ze != NULL)
          free(ze);
      if (locbuf != NULL)
          free(locbuf);
  #endif
      return NULL;
  }
--- 937,953 ----
  #endif
      return ze;
  
   FREE_AND_RETURN_NULL:
  #ifndef USE_MMAP
+     if (ze != NULL) {
        if (ze->extra != NULL)
            free(ze->extra);
        if (ze->name != NULL)
            free(ze->name);
          free(ze);
+     }
      if (locbuf != NULL)
          free(locbuf);
  #endif
      return NULL;
  }

*** 1103,1113 ****
   * number of bytes read, or -1 if an error occurred. If err->msg != 0
   * then a zip error occurred and err->msg contains the error text.
   */
  jint ZIP_Read(jzfile *zip, jzentry *entry, jint pos, void *buf, jint len)
  {
!     jint n, avail, size;
  #ifdef USE_MMAP
      jint start, end;
  #endif 
      /* Clear previous zip error */
      zip->msg = 0;
--- 1107,1117 ----
   * number of bytes read, or -1 if an error occurred. If err->msg != 0
   * then a zip error occurred and err->msg contains the error text.
   */
  jint ZIP_Read(jzfile *zip, jzentry *entry, jint pos, void *buf, jint len)
  {
!     jint avail, size;
  #ifdef USE_MMAP
      jint start, end;
  #endif 
      /* Clear previous zip error */
      zip->msg = 0;

*** 1135,1150 ****
      }  
      memcpy(buf, zip->maddr + start, len);
      return len;
  #else
      /* Seek to beginning of entry data and read bytes */
!     n = jlong_to_jint(JVM_Lseek(zip->fd, entry->pos + pos, SEEK_SET));
!     if (n != -1) {
!       n = JVM_Read(zip->fd, buf, len);
      }
! 
!     return n;
  #endif
  }
  
  
  /* The maximum size of a stack-allocated buffer.
--- 1139,1157 ----
      }  
      memcpy(buf, zip->maddr + start, len);
      return len;
  #else
      /* Seek to beginning of entry data and read bytes */
!     if (JVM_Lseek(zip->fd, entry->pos + pos, SEEK_SET) == JVM_IO_ERR) {
!       zip->msg = "ZIP_Read: JVM_Lseek failed";
!       return -1;
      }
!     if (readFully(zip->fd, buf, len) == -1) {
!       zip->msg = "ZIP_Read: error reading zip file";
!       return -1;
!     }
!     return len;
  #endif
  }
  
  
  /* The maximum size of a stack-allocated buffer.

###@###.### 2003-08-11
                                     
2003-08-11
EVALUATION

Will fix for Tiger.

-- iag@sfbay 2003-04-11

There is no regression test for this because
it too hard to create a valid test case.

We only check for errno == EINTR if JVM_Read got JVM_IO_ERR,
not EOF.
###@###.### 2003-08-11
                                     
2003-08-11
CONVERTED DATA

BugTraq+ Release Management Values

COMMIT TO FIX:
tiger

FIXED IN:
tiger

INTEGRATED IN:
tiger
tiger-b16


                                     
2004-06-14



Hardware and Software, Engineered to Work Together