DEVEL15-avoid-buffer-overflow-on-rx-fixed-size-array-return-20090402
authorSimon Wilkinson <sxw@inf.ed.ac.uk>
Mon, 6 Apr 2009 23:53:02 +0000 (23:53 +0000)
committerDerrick Brashear <shadow@dementia.org>
Mon, 6 Apr 2009 23:53:02 +0000 (23:53 +0000)
LICENSE IPL10
FIXES 124579

avoid potentially writing beyond allocated memory if a return is larger than expected

(cherry picked from commit 62bca1123fb471ca1eec58b448fd39f7f797638a)

src/afs/VNOPS/afs_vnop_lookup.c
src/sys/rmtsysc.c

index eeac33b..002998e 100644 (file)
@@ -636,8 +636,6 @@ afs_DoBulkStat(struct vcache *adp, long dirCookie, struct vrequest *areqp)
     int nskip;                 /* # of slots in the LRU queue to skip */
     struct vcache *lruvcp;     /* vcache ptr of our goal pos in LRU queue */
     struct dcache *dcp;                /* chunk containing the dir block */
-    char *statMemp;            /* status memory block */
-    char *cbfMemp;             /* callback and fid memory block */
     afs_size_t temp;           /* temp for holding chunk length, &c. */
     struct AFSFid *fidsp;      /* file IDs were collecting */
     struct AFSCallBack *cbsp;  /* call back pointers */
@@ -698,13 +696,11 @@ afs_DoBulkStat(struct vcache *adp, long dirCookie, struct vrequest *areqp)
      * one for fids and callbacks, and one for stat info.  Well set
      * up our pointers to the memory from there, too.
      */
-    statMemp = osi_AllocLargeSpace(nentries * sizeof(AFSFetchStatus));
-    statsp = (struct AFSFetchStatus *)statMemp;
-    cbfMemp =
-       osi_AllocLargeSpace(nentries *
-                           (sizeof(AFSCallBack) + sizeof(AFSFid)));
-    fidsp = (AFSFid *) cbfMemp;
-    cbsp = (AFSCallBack *) (cbfMemp + nentries * sizeof(AFSFid));
+    statsp = (AFSFetchStatus *) 
+           osi_Alloc(AFSCBMAX * sizeof(AFSFetchStatus));
+    fidsp = (AFSFid *) osi_AllocLargeSpace(nentries * sizeof(AFSFid));
+    cbsp = (AFSCallBack *) 
+           osi_Alloc(AFSCBMAX * sizeof(AFSCallBack));
 
     /* next, we must iterate over the directory, starting from the specified
      * cookie offset (dirCookie), and counting out nentries file entries.
@@ -1192,8 +1188,9 @@ afs_DoBulkStat(struct vcache *adp, long dirCookie, struct vrequest *areqp)
        code = 0;
     }
   done2:
-    osi_FreeLargeSpace(statMemp);
-    osi_FreeLargeSpace(cbfMemp);
+    osi_FreeLargeSpace((char *)fidsp);
+    osi_Free((char *)statsp, AFSCBMAX * sizeof(AFSFetchStatus));
+    osi_Free((char *)cbsp, AFSCBMAX * sizeof(AFSCallBack));
     return code;
 }
 
index bc04243..5e01f79 100644 (file)
@@ -239,8 +239,14 @@ pioctl(char *path, afs_int32 cmd, struct ViceIoctl *data, afs_int32 follow)
     InData.rmtbulk_len = data->in_size;
     InData.rmtbulk_val = inbuffer;
     inparam_conversion(cmd, InData.rmtbulk_val, 0);
-    OutData.rmtbulk_len = data->out_size;
-    OutData.rmtbulk_val = data->out;
+
+    OutData.rmtbulk_len = MAXBUFFERLEN * sizeof(*OutData.rmtbulk_val);
+    OutData.rmtbulk_val = malloc(OutData.rmtbulk_len); 
+    if (!OutData.rmtbulk_val) {
+       free(inbuffer);
+       return -1;
+    }
+
     /* We always need to pass absolute pathnames to the remote pioctl since we
      * lose the current directory value when doing an rpc call. Below we
      * prepend the current absolute path directory, if the name is relative */
@@ -277,8 +283,15 @@ pioctl(char *path, afs_int32 cmd, struct ViceIoctl *data, afs_int32 follow)
     if (!errorcode) {
        /* Do the conversions back to the host order; store the results back
         * on the same buffer */
-       outparam_conversion(cmd, OutData.rmtbulk_val, 1);
+       if (data->out_size < OutData.rmtbulk_len) {
+           errno = EINVAL;
+           errorcode = -1;
+       } else {
+           memcpy(data->out, OutData.rmtbulk_val, data->out_size);
+           outparam_conversion(cmd, data->out, 1);
+       }
     }
+    free(OutData.rmtbulk_val);
     free(inbuffer);
     return errorcode;
 }