From 62bca1123fb471ca1eec58b448fd39f7f797638a Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Mon, 6 Apr 2009 23:52:52 +0000 Subject: [PATCH] avoid-buffer-overflow-on-rx-fixed-size-array-return-20090402 LICENSE IPL10 FIXES 124579 avoid potentially writing beyond allocated memory if a return is larger than expected --- src/afs/VNOPS/afs_vnop_lookup.c | 19 ++++++++----------- src/sys/rmtsysc.c | 19 ++++++++++++++++--- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/afs/VNOPS/afs_vnop_lookup.c b/src/afs/VNOPS/afs_vnop_lookup.c index eeac33b2dc..002998ef8c 100644 --- a/src/afs/VNOPS/afs_vnop_lookup.c +++ b/src/afs/VNOPS/afs_vnop_lookup.c @@ -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; } diff --git a/src/sys/rmtsysc.c b/src/sys/rmtsysc.c index bc04243c69..5e01f79545 100644 --- a/src/sys/rmtsysc.c +++ b/src/sys/rmtsysc.c @@ -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; }