Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat

[Jerry D <jvdelisle2@gmail.com>]

On 3/5/24 1:51 PM, Harald Anlauf wrote:
> Hi Jerry,
> 
> on further thought, do we sanitize 'child_iomsg'?
> We pass it to snprintf as format.
> 
> Wouldn't a strncpy be sufficient?
> 
> Harald
> 
> 

Just to be safe I will bump char message[IOMSG_LEN] to char 
message[IOMSG_LEN + 1]

This is like a C string vs a Fortran string length situation. snprintf 
guarantees we don't exceed the child_iomsg_len and null terminates it.

I added 1 to:
  child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1

Because snprintf was chopping off the last character of the fortran 
string to put the null in. (zero based vs one based char array). I test 
this with a very long string which exceeded the length and then reduced 
it until I could see the desired end.

I have not tried running a test case with sanitize. I did check with 
valgrind.  I will try the sanitize flags to see if we get a problem.  If 
not will push.

Thanks for comments,

Jerry -

> On 3/5/24 22:37, Harald Anlauf wrote:
>> Hi Jerry,
>>
>> I think there is the risk of buffer overrun in the following places:
>>
>> +             char message[IOMSG_LEN];
>> +             child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg)
>> + 1;
>>                free_line (dtp);
>>                snprintf (message, child_iomsg_len, child_iomsg);
>>                generate_error (&dtp->common, dtp->u.p.child_saved_iostat,
>>
>> plus several more.  Wouldn't it be better to increase the size of
>> message by one?
>>
>> Thanks,
>> Harald
>>
>>
>> On 3/5/24 04:15, Jerry D wrote:
>>> On 3/1/24 11:24 AM, rep.dot.nop@gmail.com wrote:
>>>> Hi Jerry and Steve,
>>>>
>>>> On 29 February 2024 19:28:19 CET, Jerry D <jvdelisle2@gmail.com> wrote:
>>>>> On 2/29/24 10:13 AM, Steve Kargl wrote:
>>>>>> On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote:
>>>>>>> On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote:
>>>>>>>
>>>>>>>> And, just for my own education, the length limitation of iomsg to
>>>>>>>> 255
>>>>>>>> chars is not backed by the standard AFAICS, right? It's just our
>>>>>>>> STRERR_MAXSZ?
>>>>>>>
>>>>>>> Yes, its what we have had for a long lone time. Once you throw an
>>>>>>> error
>>>>>>> things get very processor dependent. I found MSGLEN set to 100 and
>>>>>>> IOMSG_len
>>>>>>> to 256. Nothing magic about it.
>>>>>>>
>>>>>>
>>>>>> There is no restriction on the length for the iomsg-variable
>>>>>> that receives the generated error message.  In fact, if the
>>>>>> iomsg-variable has a deferred-length type parameter, then
>>>>>> (re)-allocation to the exact length is expected.
>>>>>>
>>>>>>     F2023
>>>>>>
>>>>>>     12.11.6 IOMSG= specifier
>>>>>>
>>>>>>     If an error, end-of-file, or end-of-record condition occurs 
>>>>>> during
>>>>>>     execution of an input/output statement, iomsg-variable is 
>>>>>> assigned
>>>>>>     an explanatory message, as if by intrinsic assignment. If no such
>>>>>>     condition occurs, the definition status and value of
>>>>>> iomsg-variable
>>>>>>     are unchanged.
>>>>>>    character(len=23) emsg
>>>>>> read(fd,*,iomsg=emsg)
>>>>>>
>>>>>> Here, the generated iomsg is either truncated to a length of 23
>>>>>> or padded with blanks to a length of 23.
>>>>>>
>>>>>> character(len=:), allocatable :: emsg
>>>>>> read(fd,*,iomsg=emsg)
>>>>>>
>>>>>> Here, emsg should have the length of whatever error message was
>>>>>> generated.
>>>>>>    HTH
>>>>>>
>>>>>
>>>>> Well, currently, if someone uses a larger string than 256 we are
>>>>> going to chop it off.
>>>>>
>>>>> Do we want to process this differently now?
>>>>
>>>> Yes. There is some odd hunk about discrepancy of passed len and
>>>> actual len afterwards in 22-007-r1, IIRC. Didn't look closely though.
>>>>
>>> --- snip ---
>>>
>>> Attached is the revised patch using the already available
>>> string_len_trim function.
>>>
>>> This hunk is only executed if a user has not passed an iostat or iomsg
>>> variable in the parent I/O statement and an error is triggered which
>>> terminates execution of the program. In this case, the iomsg string is
>>> provided in the usual error message in a "processor defined" way.
>>>
>>> (F2023):
>>>
>>> 12.6.4.8.3 Executing defined input/output data transfers
>>> ---
>>> 11 If the iostat argument of the defined input/output procedure has a
>>> nonzero value when that procedure returns, and the processor therefore
>>> terminates execution of the program as described in 12.11, the
>>> processor shall make the value of the iomsg argument available in a
>>> processor-dependent manner.
>>> ---
>>>
>>> OK for trunk?
>>>
>>> Regards,
>>>
>>> Jerry
>>>
>>>
>>
>>
>>
>