From a598c45b0517916af3f07114526e8209a5a7d3ab Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Tue, 9 May 2017 11:06:41 +0900 Subject: [PATCH 1/2] romio314: update NFS read/write routines for large xfers When we updated UFS and others we left NFS alone. HDF group would like a fix, so here we go. Signed-off-by: Ken Raffenetti (back-ported from upstream commit pmodels/mpich@684df9f4c962c235b49eed0b18c4d2ebc92fb81b) Signed-off-by: Gilles Gouaillardet (cherry picked from commit open-mpi/ompi@02af10ce6e87e78e0c760d7addfc0bfde1294788) --- .../romio314/romio/adio/ad_nfs/ad_nfs_read.c | 85 ++++++++----------- .../romio314/romio/adio/ad_nfs/ad_nfs_write.c | 84 ++++++++---------- 2 files changed, 73 insertions(+), 96 deletions(-) diff --git a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c index 0a74dafe989..c9d980737e0 100644 --- a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c +++ b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c @@ -7,79 +7,68 @@ #include "ad_nfs.h" #include "adio_extern.h" +#ifdef HAVE_UNISTD_H +#include +#endif void ADIOI_NFS_ReadContig(ADIO_File fd, void *buf, int count, MPI_Datatype datatype, int file_ptr_type, ADIO_Offset offset, ADIO_Status *status, int *error_code) { - int err=-1; + ssize_t err=-1; MPI_Count datatype_size, len; + ADIO_Offset bytes_xfered=0; + size_t rd_count; static char myname[] = "ADIOI_NFS_READCONTIG"; + char *p; MPI_Type_size_x(datatype, &datatype_size); len = datatype_size * count; - if (file_ptr_type == ADIO_EXPLICIT_OFFSET) { - if (fd->fp_sys_posn != offset) { -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_a, 0, NULL ); -#endif - lseek(fd->fd_sys, offset, SEEK_SET); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_b, 0, NULL ); -#endif - } - if (fd->atomicity) - ADIOI_WRITE_LOCK(fd, offset, SEEK_SET, len); - else ADIOI_READ_LOCK(fd, offset, SEEK_SET, len); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_read_a, 0, NULL ); -#endif - err = read(fd->fd_sys, buf, len); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_read_b, 0, NULL ); -#endif - ADIOI_UNLOCK(fd, offset, SEEK_SET, len); - fd->fp_sys_posn = offset + err; - /* individual file pointer not updated */ - } - else { /* read from curr. location of ind. file pointer */ + if (file_ptr_type == ADIO_INDIVIDUAL) { offset = fd->fp_ind; - if (fd->fp_sys_posn != fd->fp_ind) { -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_a, 0, NULL ); -#endif - lseek(fd->fd_sys, fd->fp_ind, SEEK_SET); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_b, 0, NULL ); -#endif - } + } + + p = buf; + while (bytes_xfered < len ) { + rd_count = len - bytes_xfered; + /* FreeBSD and Darwin workaround: bigger than INT_MAX is an error */ + if (rd_count > INT_MAX) + rd_count = INT_MAX; if (fd->atomicity) - ADIOI_WRITE_LOCK(fd, offset, SEEK_SET, len); - else ADIOI_READ_LOCK(fd, offset, SEEK_SET, len); + ADIOI_WRITE_LOCK(fd, offset+bytes_xfered, SEEK_SET, rd_count); + else ADIOI_READ_LOCK(fd, offset+bytes_xfered, SEEK_SET, rd_count); #ifdef ADIOI_MPE_LOGGING MPE_Log_event( ADIOI_MPE_read_a, 0, NULL ); #endif - err = read(fd->fd_sys, buf, len); + err = pread(fd->fd_sys, p, rd_count, offset+bytes_xfered); + /* --BEGIN ERROR HANDLING-- */ + if (err == -1) { + *error_code = MPIO_Err_create_code(MPI_SUCCESS, + MPIR_ERR_RECOVERABLE, myname, __LINE__, MPI_ERR_IO, + "**io", "**io %s", strerror(errno)); + } + /* --END ERROR HANDLING-- */ #ifdef ADIOI_MPE_LOGGING MPE_Log_event( ADIOI_MPE_read_b, 0, NULL ); #endif - ADIOI_UNLOCK(fd, offset, SEEK_SET, len); - fd->fp_ind += err; - fd->fp_sys_posn = fd->fp_ind; + ADIOI_UNLOCK(fd, offset+bytes_xfered, SEEK_SET, rd_count); + if (err == 0) { + /* end of file */ + break; + } + bytes_xfered += err; + p += err; } - /* --BEGIN ERROR HANDLING-- */ - if (err == -1) { - *error_code = MPIO_Err_create_code(MPI_SUCCESS, MPIR_ERR_RECOVERABLE, - myname, __LINE__, MPI_ERR_IO, - "**io", "**io %s", strerror(errno)); - return; + fd->fp_sys_posn = offset + bytes_xfered; + if (file_ptr_type == ADIO_INDIVIDUAL) { + fd->fp_ind += bytes_xfered; } /* --END ERROR HANDLING-- */ #ifdef HAVE_STATUS_SET_BYTES - MPIR_Status_set_bytes(status, datatype, err); + if (err != -1) MPIR_Status_set_bytes(status, datatype, bytes_xfered); #endif *error_code = MPI_SUCCESS; diff --git a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c index b41488036e5..5337ada5971 100644 --- a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c +++ b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c @@ -7,76 +7,64 @@ #include "ad_nfs.h" #include "adio_extern.h" +#ifdef HAVE_UNISTD_H +#include +#endif void ADIOI_NFS_WriteContig(ADIO_File fd, const void *buf, int count, MPI_Datatype datatype, int file_ptr_type, ADIO_Offset offset, ADIO_Status *status, int *error_code) { - int err=-1; + ssize_t err=-1; MPI_Count datatype_size, len; + ADIO_Offset bytes_xfered=0; + size_t wr_count; static char myname[] = "ADIOI_NFS_WRITECONTIG"; + char *p; MPI_Type_size_x(datatype, &datatype_size); - len = datatype_size * count; + len = datatype_size * (ADIO_Offset)count; - if (file_ptr_type == ADIO_EXPLICIT_OFFSET) { - if (fd->fp_sys_posn != offset) { -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_a, 0, NULL ); -#endif - lseek(fd->fd_sys, offset, SEEK_SET); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_b, 0, NULL ); -#endif - } - ADIOI_WRITE_LOCK(fd, offset, SEEK_SET, len); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_write_a, 0, NULL ); -#endif - err = write(fd->fd_sys, buf, len); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_write_b, 0, NULL ); -#endif - ADIOI_UNLOCK(fd, offset, SEEK_SET, len); - fd->fp_sys_posn = offset + err; - /* individual file pointer not updated */ - } - else { /* write from curr. location of ind. file pointer */ + if (file_ptr_type == ADIO_INDIVIDUAL) { offset = fd->fp_ind; - if (fd->fp_sys_posn != fd->fp_ind) { -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_a, 0, NULL ); -#endif - lseek(fd->fd_sys, fd->fp_ind, SEEK_SET); -#ifdef ADIOI_MPE_LOGGING - MPE_Log_event( ADIOI_MPE_lseek_b, 0, NULL ); -#endif - } - ADIOI_WRITE_LOCK(fd, offset, SEEK_SET, len); + } + + p = (char *)buf; + while (bytes_xfered < len) { #ifdef ADIOI_MPE_LOGGING MPE_Log_event( ADIOI_MPE_write_a, 0, NULL ); #endif - err = write(fd->fd_sys, buf, len); + wr_count = len - bytes_xfered; + /* work around FreeBSD and OS X defects*/ + if (wr_count > INT_MAX) + wr_count = INT_MAX; + + ADIOI_WRITE_LOCK(fd, offset+bytes_xfered, SEEK_SET, wr_count); + err = pwrite(fd->fd_sys, p, wr_count, offset+bytes_xfered); + /* --BEGIN ERROR HANDLING-- */ + if (err == -1) { + *error_code = MPIO_Err_create_code(MPI_SUCCESS, + MPIR_ERR_RECOVERABLE, + myname, __LINE__, MPI_ERR_IO, "**io", + "**io %s", strerror(errno)); + fd->fp_sys_posn = -1; + return; + } + /* --END ERROR HANDLING-- */ #ifdef ADIOI_MPE_LOGGING MPE_Log_event( ADIOI_MPE_write_b, 0, NULL ); #endif - ADIOI_UNLOCK(fd, offset, SEEK_SET, len); - fd->fp_ind += err; - fd->fp_sys_posn = fd->fp_ind; + ADIOI_UNLOCK(fd, offset+bytes_xfered, SEEK_SET, wr_count); + bytes_xfered += err; + p += err; } - /* --BEGIN ERROR HANDLING-- */ - if (err == -1) { - *error_code = MPIO_Err_create_code(MPI_SUCCESS, MPIR_ERR_RECOVERABLE, - myname, __LINE__, MPI_ERR_IO, - "**io", - "**io %s", strerror(errno)); - return; + if (file_ptr_type == ADIO_INDIVIDUAL) { + fd->fp_ind += bytes_xfered; } - /* --END ERROR HANDLING-- */ #ifdef HAVE_STATUS_SET_BYTES - MPIR_Status_set_bytes(status, datatype, err); + MPIR_Status_set_bytes(status, datatype, bytes_xfered); #endif *error_code = MPI_SUCCESS; From 06197972ef62b1b38c0d72e28c9bc1319044b22a Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Tue, 9 May 2017 11:11:12 +0900 Subject: [PATCH 2/2] romio314: adio/ad_nfs: fix buffer overflows in ADIOI_NFS_{Read,Write}Strided Refs: models/mpich#2338 Refs: models/mpich#2617 Signed-off-by: Rob Latham (back-ported from upstream commit pmodels/mpich@642db576487394440776b2b7216faa1a822b875b) Signed-off-by: Gilles Gouaillardet (cherry picked from commit open-mpi/ompi@eaf050cfe1505801211b5087cad9fd9522912b56) --- .../io/romio314/romio/adio/ad_nfs/ad_nfs_read.c | 17 ++++++++++------- .../romio314/romio/adio/ad_nfs/ad_nfs_write.c | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c index c9d980737e0..48543ea0cee 100644 --- a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c +++ b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_read.c @@ -157,8 +157,9 @@ void ADIOI_NFS_ReadStrided(ADIO_File fd, void *buf, int count, /* offset is in units of etype relative to the filetype. */ ADIOI_Flatlist_node *flat_buf, *flat_file; - int i, j, k, err=-1, brd_size, frd_size=0, st_index=0; - int bufsize, num, size, sum, n_etypes_in_filetype, size_in_filetype; + int i, j, k, err=-1, brd_size, st_index=0; + int num, size, sum, n_etypes_in_filetype, size_in_filetype; + MPI_Count bufsize; int n_filetypes, etype_in_filetype; ADIO_Offset abs_off_in_filetype=0; int req_len, partial_read; @@ -168,8 +169,9 @@ void ADIOI_NFS_ReadStrided(ADIO_File fd, void *buf, int count, ADIO_Offset userbuf_off; ADIO_Offset off, req_off, disp, end_offset=0, readbuf_off, start_off; char *readbuf, *tmp_buf, *value; - int st_frd_size, st_n_filetypes, readbuf_len; - int new_brd_size, new_frd_size, err_flag=0, info_flag, max_bufsize; + int st_n_filetypes, readbuf_len; + ADIO_Offset frd_size=0, new_frd_size, st_frd_size; + int new_brd_size, err_flag=0, info_flag, max_bufsize; static char myname[] = "ADIOI_NFS_READSTRIDED"; @@ -449,12 +451,13 @@ void ADIOI_NFS_ReadStrided(ADIO_File fd, void *buf, int count, else { /* noncontiguous in memory as well as in file */ + ADIO_Offset i; ADIOI_Flatten_datatype(datatype); flat_buf = ADIOI_Flatlist; while (flat_buf->type != datatype) flat_buf = flat_buf->next; k = num = buf_count = 0; - i = (int) (flat_buf->indices[0]); + i = flat_buf->indices[0]; j = st_index; off = offset; n_filetypes = st_n_filetypes; @@ -499,8 +502,8 @@ void ADIOI_NFS_ReadStrided(ADIO_File fd, void *buf, int count, k = (k + 1)%flat_buf->count; buf_count++; - i = (int) (buftype_extent*(buf_count/flat_buf->count) + - flat_buf->indices[k]); + i = buftype_extent*(buf_count/flat_buf->count) + + flat_buf->indices[k]; new_brd_size = flat_buf->blocklens[k]; if (size != frd_size) { off += size; diff --git a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c index 5337ada5971..0a4636bb9e1 100644 --- a/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c +++ b/ompi/mca/io/romio314/romio/adio/ad_nfs/ad_nfs_write.c @@ -260,8 +260,9 @@ void ADIOI_NFS_WriteStrided(ADIO_File fd, const void *buf, int count, /* offset is in units of etype relative to the filetype. */ ADIOI_Flatlist_node *flat_buf, *flat_file; - int i, j, k, err=-1, bwr_size, fwr_size=0, st_index=0; - int bufsize, num, size, sum, n_etypes_in_filetype, size_in_filetype; + int i, j, k, err=-1, bwr_size, st_index=0; + int num, size, sum, n_etypes_in_filetype, size_in_filetype; + MPI_Count bufsize; int n_filetypes, etype_in_filetype; ADIO_Offset abs_off_in_filetype=0; int req_len; @@ -271,8 +272,9 @@ void ADIOI_NFS_WriteStrided(ADIO_File fd, const void *buf, int count, ADIO_Offset userbuf_off; ADIO_Offset off, req_off, disp, end_offset=0, writebuf_off, start_off; char *writebuf=NULL, *value; - int st_fwr_size, st_n_filetypes, writebuf_len, write_sz; - int new_bwr_size, new_fwr_size, err_flag=0, info_flag, max_bufsize; + int st_n_filetypes, writebuf_len, write_sz; + ADIO_Offset fwr_size = 0, new_fwr_size, st_fwr_size; + int new_bwr_size, err_flag=0, info_flag, max_bufsize; static char myname[] = "ADIOI_NFS_WRITESTRIDED"; ADIOI_Datatype_iscontig(datatype, &buftype_is_contig); @@ -553,12 +555,13 @@ void ADIOI_NFS_WriteStrided(ADIO_File fd, const void *buf, int count, else { /* noncontiguous in memory as well as in file */ + ADIO_Offset i; ADIOI_Flatten_datatype(datatype); flat_buf = ADIOI_Flatlist; while (flat_buf->type != datatype) flat_buf = flat_buf->next; k = num = buf_count = 0; - i = (int) (flat_buf->indices[0]); + i = flat_buf->indices[0]; j = st_index; off = offset; n_filetypes = st_n_filetypes; @@ -604,8 +607,8 @@ void ADIOI_NFS_WriteStrided(ADIO_File fd, const void *buf, int count, k = (k + 1)%flat_buf->count; buf_count++; - i = (int) (buftype_extent*(buf_count/flat_buf->count) + - flat_buf->indices[k]); + i = buftype_extent*(buf_count/flat_buf->count) + + flat_buf->indices[k]; new_bwr_size = flat_buf->blocklens[k]; if (size != fwr_size) { off += size;