Skip to content

Commit d610024

Browse files
committed
Ensure the buffer provided to MPAS_io_get_var_generic is large enough.
A fixed size array is provided as an output buffer when reading a 0d-char character variable. Call MPAS_io_inq_var prior to the read to get the size of the variable, and only proceed with the read if the size of the variable will fit in the provided array. Return an error code if the variable value is larger than the provided output buffer.
1 parent 55c737f commit d610024

File tree

5 files changed

+288
-29
lines changed

5 files changed

+288
-29
lines changed

src/core_test/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ OBJS = mpas_test_core.o \
1212
mpas_test_core_dmpar.o \
1313
mpas_test_core_stream_inquiry.o \
1414
mpas_test_openacc.o \
15-
mpas_test_core_stream_list.o
15+
mpas_test_core_stream_list.o \
16+
mpas_test_core_io.o \
1617

1718
all: core_test
1819

src/core_test/mpas_test_core.F

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ function test_core_run(domain) result(iErr)!{{{
9797
use test_core_string_utils, only : mpas_test_string_utils
9898
use mpas_test_core_dmpar, only : mpas_test_dmpar
9999
use mpas_test_core_stream_inquiry, only : mpas_test_stream_inquiry
100+
use test_core_io, only : test_core_io_test
100101
use mpas_test_core_openacc, only : mpas_test_openacc
101102

102103
implicit none
@@ -224,6 +225,17 @@ function test_core_run(domain) result(iErr)!{{{
224225

225226
call mpas_stream_mgr_write(domain % streamManager, forceWriteNow=.true.)
226227

228+
!
229+
! Run io tests
230+
!
231+
call mpas_log_write('')
232+
call test_core_io_test(domain, threadErrs, iErr)
233+
if (iErr == 0) then
234+
call mpas_log_write('All tests PASSED')
235+
else
236+
call mpas_log_write('$i tests FAILED', intArgs=[iErr])
237+
end if
238+
call mpas_log_write('')
227239
!
228240
! Run mpas_test_openacc
229241
!

src/core_test/mpas_test_core_io.F

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
! Copyright (c) 2013, Los Alamos National Security, LLC (LANS)
2+
! and the University Corporation for Atmospheric Research (UCAR).
3+
!
4+
! Unless noted otherwise source code is licensed under the BSD license.
5+
! Additional copyright and license information can be found in the LICENSE file
6+
! distributed with this code, or at http://mpas-dev.github.com/license.html
7+
!
8+
module test_core_io
9+
10+
#define ERROR_WRITE(M) call mpas_log_write( M , messageType=MPAS_LOG_ERR)
11+
#define ERROR_WRITE_ARGS(M, ARGS) call mpas_log_write( M , ARGS, messageType=MPAS_LOG_ERR)
12+
use mpas_log
13+
use mpas_io
14+
15+
implicit none
16+
private
17+
public :: test_core_io_test
18+
19+
contains
20+
21+
!***********************************************************************
22+
!
23+
! routine close_file_with_message
24+
!
25+
!> \brief closes the provided file handle and writes an error message.
26+
!-----------------------------------------------------------------------
27+
subroutine close_file_with_message(fileHandle, message, args)
28+
type(MPAS_IO_Handle_type), intent(inout) :: fileHandle
29+
character (len=*), intent(in), optional :: message
30+
integer, dimension(:), intent(in), optional :: args
31+
32+
integer :: local_ierr
33+
34+
! log an error message
35+
if (present(message)) then
36+
if (present(args)) then
37+
ERROR_WRITE_ARGS(message, intArgs=args)
38+
else
39+
ERROR_WRITE(message)
40+
end if
41+
end if
42+
43+
! close the provided file
44+
call MPAS_io_close(fileHandle, local_ierr)
45+
if (local_ierr /= MPAS_IO_NOERR) then
46+
ERROR_WRITE_ARGS('MPAS_io_close failed with error code:$i', intArgs=(/local_ierr/))
47+
return
48+
endif
49+
50+
end subroutine close_file_with_message
51+
52+
!***********************************************************************
53+
!
54+
! routine test_read_string_buffer_check
55+
!
56+
!> \brief verifies attempts to read strings into buffers which are too small
57+
!> to hold the value fails safely.
58+
!> \details
59+
!> Run these tests with valgrind to ensure there are no buffer overflows when
60+
!> attempting to read strings into undersized buffers.
61+
!-----------------------------------------------------------------------
62+
subroutine test_read_string_buffer_check(domain, threadErrs, ierr)
63+
64+
type (domain_type), intent(inout) :: domain
65+
integer, dimension(:), intent(out) :: threadErrs
66+
integer, intent(out) :: ierr
67+
68+
integer :: local_ierr, i
69+
type(MPAS_IO_Handle_type) :: fileHandle
70+
character (len=StrKIND), dimension(1), parameter :: dimNamesString = ['StrLen']
71+
character (len=StrKIND), dimension(2), parameter :: dimNamesStringTime = ['StrLen', 'Time ']
72+
character (len=32), parameter :: varName1 = 'stringVar'
73+
character (len=32), parameter :: varName2 = 'stringTimeVar'
74+
character (len=*), parameter :: varValue1 = 'This is a string'
75+
character (len=32), dimension(2), parameter :: varNames = [varName1, varName2]
76+
integer, parameter :: bufferSize=128
77+
integer, parameter :: smallBufferSize=bufferSize/2
78+
character (len=bufferSize) :: buffer
79+
character (len=smallBufferSize) :: smallBuffer
80+
character (len=*), parameter :: filename = 'char_data.nc'
81+
82+
ierr = 0
83+
84+
! open a file to write char variables to
85+
fileHandle = MPAS_io_open(filename, MPAS_IO_WRITE, MPAS_IO_NETCDF4, domain % ioContext, &
86+
clobber_file=.true., truncate_file=.true., ierr=local_ierr)
87+
if (local_ierr /= MPAS_IO_NOERR) then
88+
ierr = 1
89+
ERROR_WRITE('Error opening file ' // trim(filename))
90+
return
91+
end if
92+
93+
! define dimensions and char variables
94+
call MPAS_io_def_dim(fileHandle, dimNamesStringTime(1), bufferSize, local_ierr)
95+
if (local_ierr /= MPAS_IO_NOERR) then
96+
ierr = 1
97+
call close_file_with_message(fileHandle, 'Error defining '//trim(dimNamesStringTime(1))//', error=$i', (/local_ierr/))
98+
return
99+
end if
100+
call MPAS_io_def_dim(fileHandle, dimNamesStringTime(2), MPAS_IO_UNLIMITED_DIM, local_ierr)
101+
if (local_ierr /= MPAS_IO_NOERR) then
102+
ierr = 1
103+
call close_file_with_message(fileHandle, 'Error defining '//trim(dimNamesStringTime(2))//', error=$i', (/local_ierr/))
104+
return
105+
end if
106+
call MPAS_io_def_var(fileHandle, varNames(1), MPAS_IO_CHAR, dimNamesString, ierr=local_ierr)
107+
if (local_ierr /= MPAS_IO_NOERR) then
108+
ierr = 1
109+
call close_file_with_message(fileHandle, 'Error defining var "'//trim(varNames(1))//'" error=$i', (/local_ierr/))
110+
return
111+
end if
112+
call MPAS_io_def_var(fileHandle, varNames(2), MPAS_IO_CHAR, dimNamesStringTime, ierr=local_ierr)
113+
if (local_ierr /= MPAS_IO_NOERR) then
114+
ierr = 1
115+
call close_file_with_message(fileHandle, 'Error defining var "'//trim(varNames(2))//'" error=$i', (/local_ierr/))
116+
return
117+
end if
118+
119+
! write the string values
120+
do i=1,2
121+
call MPAS_io_put_var_char0d(fileHandle, varNames(i), varValue1, local_ierr)
122+
if (local_ierr /= MPAS_IO_NOERR) then
123+
ierr = 1
124+
call close_file_with_message(fileHandle, 'Error writing "'//trim(varNames(i))// &
125+
'", error=$i', (/local_ierr/))
126+
return
127+
end if
128+
129+
! verify the strings are read into buffers which are large enough for the strin values
130+
call MPAS_io_get_var_char0d(fileHandle, varNames(i), buffer, local_ierr)
131+
if (local_ierr /= MPAS_IO_NOERR) then
132+
ierr = 1
133+
call close_file_with_message(fileHandle, 'Error reading "'//trim(varNames(i))// &
134+
'", error=$i', (/local_ierr/))
135+
return
136+
end if
137+
end do
138+
139+
! verify attempts to read strings into buffers which are too small generates an error
140+
call mpas_log_write(' ')
141+
call mpas_log_write('Expect to see the following error:')
142+
call MPAS_io_err_mesg(domain % ioContext, MPAS_IO_ERR_INSUFFICIENT_ARG, .false.)
143+
call mpas_log_write(' ')
144+
do i=1,2
145+
! this should return an error
146+
call MPAS_io_get_var_char0d(fileHandle, varNames(i), smallBuffer, local_ierr)
147+
call mpas_log_write(' ')
148+
149+
if (local_ierr /= MPAS_IO_ERR_INSUFFICIENT_ARG) then
150+
ierr = 1
151+
if (local_ierr == MPAS_IO_NOERR) then
152+
call close_file_with_message(fileHandle, 'Expected MPAS_IO_ERR_INSUFFICIENT_ARG ($i)'&
153+
//' but recieved no error reading "'//trim(varName1), (/local_ierr/))
154+
else
155+
call close_file_with_message(fileHandle, 'Expected MPAS_IO_ERR_INSUFFICIENT_ARG ($i)'&
156+
//' but recieved error $i reading "'//trim(varName1)//'"', &
157+
(/MPAS_IO_ERR_INSUFFICIENT_ARG, local_ierr/))
158+
end if
159+
return
160+
end if
161+
end do
162+
call close_file_with_message(fileHandle)
163+
164+
end subroutine test_read_string_buffer_check
165+
166+
167+
!***********************************************************************
168+
! Subroutine test_core_io_test
169+
!
170+
!> \brief Core test suite for I/O
171+
!>
172+
!> \details This subroutine tests mpas_io features.
173+
!> It calls individual tests for I/O operations.
174+
!> See the subroutine body for details.
175+
!> The results of each test are logged with a success or failure message.
176+
!>
177+
!> \param domain The domain object that contains the I/O context
178+
!> \param threadErrs An array to store any errors encountered during
179+
!> the test.
180+
!> \param ierr The error code that indicates the result of the test.
181+
!
182+
!-----------------------------------------------------------------------
183+
subroutine test_core_io_test(domain, threadErrs, ierr)
184+
185+
use mpas_log
186+
187+
type (domain_type), intent(inout) :: domain
188+
integer, dimension(:), intent(out) :: threadErrs
189+
integer, intent(out) :: ierr
190+
191+
integer :: test_status
192+
193+
ierr = 0
194+
test_status = 0
195+
196+
call mpas_log_write('Testing char-0 buffer reads')
197+
call test_read_string_buffer_check(domain, threadErrs, test_status)
198+
if (test_status == 0) then
199+
call mpas_log_write('char-0 buffer tests: SUCCESS')
200+
else
201+
call mpas_log_write('char-0 buffer tests: FAILURE', MPAS_LOG_ERR)
202+
ierr = ierr + abs(test_status)
203+
end if
204+
205+
206+
end subroutine test_core_io_test
207+
208+
end module test_core_io

src/framework/mpas_io.F

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
!
88
module mpas_io
99

10+
#define IO_DEBUG_WRITE(M, ARGS) !call mpas_log_write(M, ARGS)
11+
#define IO_ERROR_WRITE(M, ARGS) call mpas_log_write( M, ARGS, messageType=MPAS_LOG_ERR)
1012
use mpas_derived_types
1113
use mpas_attlist
1214
use mpas_dmpar
@@ -1847,6 +1849,13 @@ subroutine MPAS_io_get_var_generic(handle, fieldname, intVal, intArray1d, intArr
18471849
character (len=:), pointer :: charVal_p
18481850
character (len=:), dimension(:), pointer :: charArray1d_p
18491851

1852+
! local variables returned from MPAS_io_inq_var
1853+
integer :: fieldtype
1854+
integer :: ndims
1855+
integer, dimension(:), pointer :: dimsizes
1856+
character (len=StrKIND), dimension(:), pointer :: dimnames
1857+
character (len=StrKind) :: message
1858+
18501859
#ifdef MPAS_SMIOL_SUPPORT
18511860
type (SMIOLf_decomp), pointer :: null_decomp
18521861

@@ -1984,22 +1993,40 @@ subroutine MPAS_io_get_var_generic(handle, fieldname, intVal, intArray1d, intArr
19841993
! call mpas_log_write(' value is char')
19851994

19861995
charVal_p => charVal
1996+
1997+
! get the dimension of the char variable to ensure the provided output buffer is large enough
1998+
call MPAS_io_inq_var(handle, fieldname, fieldtype, ndims, dimnames, dimsizes, local_ierr)
1999+
do i = 1, ndims
2000+
message = ' MPAS_io_get_var_generic len(charVal):$i var "'//trim(fieldname)// &
2001+
'" type is $i dim $i is '// trim(dimnames(i))//' size is $i'
2002+
IO_DEBUG_WRITE(message , intArgs=(/len(charVal), fieldtype, i, dimsizes(i)/))
2003+
end do
2004+
! because charVal is provided, assume dimension 1 is the string length
2005+
if (dimsizes(1) > len(charVal)) then
2006+
local_ierr = MPAS_IO_ERR_INSUFFICIENT_ARG
2007+
message = ' MPAS_io_get_var_generic var "'//trim(fieldname)// &
2008+
'" len too big, len=$i buflen=$i'
2009+
IO_ERROR_WRITE(message, intArgs=(/dimsizes(1), len(charVal)/))
2010+
else
19872011
#ifdef MPAS_SMIOL_SUPPORT
1988-
local_ierr = SMIOLf_get_var(handle % smiol_file, trim(fieldname), null_decomp, charVal_p)
2012+
local_ierr = SMIOLf_get_var(handle % smiol_file, trim(fieldname), null_decomp, charVal_p)
19892013
#endif
19902014

19912015
#ifdef MPAS_PIO_SUPPORT
1992-
if (field_cursor % fieldhandle % has_unlimited_dim) then
1993-
count2(1) = field_cursor % fieldhandle % dims(1) % dimsize
1994-
pio_ierr = PIO_get_var(handle % pio_file, field_cursor % fieldhandle % fieldid, start2, count2, tempchar)
1995-
charVal(1:count2(1)) = tempchar(1)(1:count2(1))
1996-
else
1997-
start1(1) = 1
1998-
count1(1) = field_cursor % fieldhandle % dims(1) % dimsize
1999-
pio_ierr = PIO_get_var(handle % pio_file, field_cursor % fieldhandle % fieldid, start1, count1, tempchar)
2000-
charVal(1:count1(1)) = tempchar(1)(1:count1(1))
2001-
end if
2016+
if (field_cursor % fieldhandle % has_unlimited_dim) then
2017+
count2(1) = field_cursor % fieldhandle % dims(1) % dimsize
2018+
pio_ierr = PIO_get_var(handle % pio_file, field_cursor % fieldhandle % fieldid, start2, count2, tempchar)
2019+
charVal(1:count2(1)) = tempchar(1)(1:count2(1))
2020+
else
2021+
start1(1) = 1
2022+
count1(1) = field_cursor % fieldhandle % dims(1) % dimsize
2023+
pio_ierr = PIO_get_var(handle % pio_file, field_cursor % fieldhandle % fieldid, start1, count1, tempchar)
2024+
charVal(1:count1(1)) = tempchar(1)(1:count1(1))
2025+
end if
20022026
#endif
2027+
end if
2028+
deallocate(dimsizes)
2029+
deallocate(dimnames)
20032030
else if (present(charArray1d)) then
20042031
! call mpas_log_write(' value is char1')
20052032
#ifdef MPAS_PIO_SUPPORT
@@ -2765,28 +2792,34 @@ subroutine MPAS_io_get_var_generic(handle, fieldname, intVal, intArray1d, intArr
27652792
end if
27662793

27672794
! call mpas_log_write('Checking for error')
2795+
if (local_ierr == MPAS_IO_ERR_INSUFFICIENT_ARG) then
2796+
call MPAS_io_err_mesg(handle % ioContext, local_ierr, .false.)
2797+
io_global_err = local_ierr
2798+
if (present(ierr)) ierr = local_ierr
2799+
else
27682800
#ifdef MPAS_PIO_SUPPORT
2769-
if (pio_ierr /= PIO_noerr) then
2770-
io_global_err = pio_ierr
2771-
if (present(ierr)) ierr = MPAS_IO_ERR_BACKEND
2772-
return
2773-
end if
2801+
if (pio_ierr /= PIO_noerr) then
2802+
io_global_err = pio_ierr
2803+
if (present(ierr)) ierr = MPAS_IO_ERR_BACKEND
2804+
return
2805+
end if
27742806
#endif
27752807

27762808
#ifdef MPAS_SMIOL_SUPPORT
2777-
if (local_ierr /= SMIOL_SUCCESS) then
2778-
call mpas_log_write('SMIOLf_get_var failed with error $i', intArgs=[local_ierr], messageType=MPAS_LOG_ERR)
2779-
if (local_ierr == SMIOL_LIBRARY_ERROR) then
2780-
call mpas_log_write(trim(SMIOLf_lib_error_string(handle % ioContext % smiol_context)), messageType=MPAS_LOG_ERR)
2781-
else
2782-
call mpas_log_write(trim(SMIOLf_error_string(local_ierr)), messageType=MPAS_LOG_ERR)
2783-
end if
2809+
if (local_ierr /= SMIOL_SUCCESS) then
2810+
call mpas_log_write('SMIOLf_get_var failed with error $i', intArgs=[local_ierr], messageType=MPAS_LOG_ERR)
2811+
if (local_ierr == SMIOL_LIBRARY_ERROR) then
2812+
call mpas_log_write(trim(SMIOLf_lib_error_string(handle % ioContext % smiol_context)), messageType=MPAS_LOG_ERR)
2813+
else
2814+
call mpas_log_write(trim(SMIOLf_error_string(local_ierr)), messageType=MPAS_LOG_ERR)
2815+
end if
27842816

2785-
io_global_err = local_ierr
2786-
if (present(ierr)) ierr = MPAS_IO_ERR_BACKEND
2787-
return
2788-
end if
2817+
io_global_err = local_ierr
2818+
if (present(ierr)) ierr = MPAS_IO_ERR_BACKEND
2819+
return
2820+
end if
27892821
#endif
2822+
end if
27902823

27912824
end subroutine MPAS_io_get_var_generic
27922825

@@ -6498,6 +6531,10 @@ subroutine MPAS_io_err_mesg(ioContext, ierr, fatal)
64986531
call mpas_log_write('MPAS IO Error: Would clobber existing file', MPAS_LOG_ERR)
64996532
case (MPAS_IO_ERR_NOEXIST_READ)
65006533
call mpas_log_write('MPAS IO Error: Attempting to read a file which does not exist.', MPAS_LOG_ERR)
6534+
case (MPAS_IO_ERR_MISSING_DIM)
6535+
call mpas_log_write('MPAS IO Error: Attempting to read a dimension which does not exist.', MPAS_LOG_ERR)
6536+
case (MPAS_IO_ERR_INSUFFICIENT_ARG)
6537+
call mpas_log_write('MPAS IO Error: Attempting to read a string into a buffer which is too small.', MPAS_LOG_ERR)
65016538
case default
65026539
call mpas_log_write('MPAS IO Error: Unrecognized error code...', MPAS_LOG_ERR)
65036540
end select

src/framework/mpas_io_types.inc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@
6565
MPAS_IO_ERR_UNIMPLEMENTED = -18, &
6666
MPAS_IO_ERR_WOULD_CLOBBER = -19, &
6767
MPAS_IO_ERR_NOEXIST_READ = -20, &
68-
MPAS_IO_ERR_MISSING_DIM = -21
68+
MPAS_IO_ERR_MISSING_DIM = -21, &
69+
MPAS_IO_ERR_INSUFFICIENT_ARG = -22
6970

7071
type MPAS_IO_Handle_type
7172
logical :: initialized = .false.

0 commit comments

Comments
 (0)