Skip to content

Commit 8871bdb

Browse files
committed
fcoll/two_phase: fix coverity issues
Fix CID 72296: Resource leak (RESOURCE_LEAK): Changed code to goto exit instead of returning to ensure memory is freed. Fix CID 712589: Out-of-bounds read (OVERRUN): In this loop i and j are identical and always less than iov_count. The CID was triggered because i was incremented if i was < iov_count. This meant that if the loop did go on the next iteration would access an invalid index. Fix CID 741363: Uninitialized scalar variable (UNINIT): Allocate tmp_len with calloc to insure every index is initialized. Fix CID 741364: Uninitialized pointer read (UNINIT): Allocate recv_types with calloc to ensure all indices are always initialized. Also added a check to not loop and destroy if recv_types is NULL. Also added a NULL check on the allocation of decoded iov. This is not the cause of CID 126784 but should be fixed. Fix CID 712588: Out-of-bounds read (OVERRUN): Similar to CID 712589. Should silence the issue. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent 60a3eb1 commit 8871bdb

File tree

2 files changed

+24
-23
lines changed

2 files changed

+24
-23
lines changed

ompi/mca/fcoll/two_phase/fcoll_two_phase_file_read_all.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -277,15 +277,9 @@ mca_fcoll_two_phase_file_read_all (mca_io_ompio_file_t *fh,
277277
}
278278
}
279279
flat_buf->count = local_size;
280-
i=0;j=0;
281-
while(j < local_size){
282-
flat_buf->indices[j] = (OMPI_MPI_OFFSET_TYPE)(intptr_t)decoded_iov[i].iov_base;
283-
flat_buf->blocklens[j] = decoded_iov[i].iov_len;
284-
285-
if(i < (int)iov_count)
286-
i+=1;
287-
288-
j+=1;
280+
for (j = 0 ; j < local_size ; ++j) {
281+
flat_buf->indices[j] = (OMPI_MPI_OFFSET_TYPE)(intptr_t)decoded_iov[j].iov_base;
282+
flat_buf->blocklens[j] = decoded_iov[j].iov_len;
289283
}
290284

291285
#if DEBUG

ompi/mca/fcoll/two_phase/fcoll_two_phase_file_write_all.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Copyright (c) 2008-2014 University of Houston. All rights reserved.
1414
* Copyright (c) 2015 Research Organization for Information Science
1515
* and Technology (RIST). All rights reserved.
16-
* Copyright (c) 2015 Los Alamos National Security, LLC. All rights
16+
* Copyright (c) 2015-2016 Los Alamos National Security, LLC. All rights
1717
* reserved.
1818
* $COPYRIGHT$
1919
*
@@ -194,6 +194,10 @@ mca_fcoll_two_phase_file_write_all (mca_io_ompio_file_t *fh,
194194
if ( 0 < iov_count ) {
195195
decoded_iov = (struct iovec *)malloc
196196
(iov_count * sizeof(struct iovec));
197+
if (NULL == decoded_iov) {
198+
ret = OMPI_ERR_OUT_OF_RESOURCE;
199+
goto exit;
200+
}
197201
}
198202
for (ti = 0; ti < iov_count; ti ++){
199203
decoded_iov[ti].iov_base = (IOVBASE_TYPE *)(
@@ -320,19 +324,15 @@ mca_fcoll_two_phase_file_write_all (mca_io_ompio_file_t *fh,
320324
}
321325
}
322326
flat_buf->count = local_size;
323-
i=0;j=0;
324-
while(j < local_size){
327+
for (j = 0 ; j < local_size ; ++j) {
325328
if ( 0 < max_data ) {
326-
flat_buf->indices[j] = (OMPI_MPI_OFFSET_TYPE)(intptr_t)decoded_iov[i].iov_base;
327-
flat_buf->blocklens[j] = decoded_iov[i].iov_len;
329+
flat_buf->indices[j] = (OMPI_MPI_OFFSET_TYPE)(intptr_t)decoded_iov[j].iov_base;
330+
flat_buf->blocklens[j] = decoded_iov[j].iov_len;
328331
}
329332
else {
330333
flat_buf->indices[j] = 0;
331334
flat_buf->blocklens[j] = 0;
332335
}
333-
if(i < (int)iov_count)
334-
i+=1;
335-
j+=1;
336336
}
337337

338338
#if DEBUG_ON
@@ -967,14 +967,14 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
967967

968968

969969
recv_types = (ompi_datatype_t **)
970-
malloc (( nprocs_recv + 1 ) * sizeof(ompi_datatype_t *));
970+
calloc (( nprocs_recv + 1 ), sizeof(ompi_datatype_t *));
971971

972972
if ( NULL == recv_types ){
973973
ret = OMPI_ERR_OUT_OF_RESOURCE;
974974
goto exit;
975975
}
976976

977-
tmp_len = (int *) malloc(fh->f_size*sizeof(int));
977+
tmp_len = (int *) calloc(fh->f_size, sizeof(int));
978978

979979
if ( NULL == tmp_len ) {
980980
ret = OMPI_ERR_OUT_OF_RESOURCE;
@@ -1005,15 +1005,13 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
10051005

10061006
if ( NULL == srt_off ){
10071007
ret = OMPI_ERR_OUT_OF_RESOURCE;
1008-
free(tmp_len);
10091008
goto exit;
10101009
}
10111010

10121011
srt_len = (int *) malloc((sum+1)*sizeof(int));
10131012

10141013
if ( NULL == srt_len ) {
10151014
ret = OMPI_ERR_OUT_OF_RESOURCE;
1016-
free(tmp_len);
10171015
free(srt_off);
10181016
goto exit;
10191017
}
@@ -1029,6 +1027,7 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
10291027
}
10301028

10311029
free(tmp_len);
1030+
tmp_len = NULL;
10321031

10331032
*hole = 0;
10341033
if (off != srt_off[0]){
@@ -1059,7 +1058,8 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
10591058
(sizeof(mca_io_ompio_io_array_t));
10601059
if (NULL == fh->f_io_array) {
10611060
opal_output(1, "OUT OF MEMORY\n");
1062-
return OMPI_ERR_OUT_OF_RESOURCE;
1061+
ret = OMPI_ERR_OUT_OF_RESOURCE;
1062+
goto exit;
10631063
}
10641064
fh->f_io_array[0].offset =(IOVBASE_TYPE *)(intptr_t) off;
10651065
fh->f_num_of_io_entries = 1;
@@ -1182,7 +1182,13 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
11821182
#endif
11831183

11841184
exit:
1185-
for (i=0; i<nprocs_recv; i++) ompi_datatype_destroy(recv_types+i);
1185+
if (recv_types) {
1186+
for (i=0; i<nprocs_recv; i++) {
1187+
if (recv_types[i]) {
1188+
ompi_datatype_destroy(recv_types+i);
1189+
}
1190+
}
1191+
}
11861192
free (recv_types);
11871193

11881194
free (requests);
@@ -1193,6 +1199,7 @@ static int two_phase_exchage_data(mca_io_ompio_file_t *fh,
11931199

11941200
free (send_buf);
11951201
}
1202+
free (tmp_len);
11961203

11971204
return ret;
11981205
}

0 commit comments

Comments
 (0)