RFR: 8049695: nsk/jdb/options/connect/connect003 fails with "Launched jdb could not attach to debuggee during 300000 milliseconds"

[Alex Menkov]

Hi Chris,

On 03/14/2018 13:42, Chris Plummer wrote:
> Hi Alex,
> 
> I don't think prefix -> basename is what David had in mind. Those 
> basically mean the same thing. The buffer is being used for the full 
> name, which is why neither is really appropriate. So maybe just call it 
> fullname, or even just name. createConnection() has a similar prefix 
> reference that should be fixed.

Ok, I don't like "fullname", "name" is already used there, so I made 
them "objectName" (for mutex/event names) and streamName (for stream 
name in createConnection()).

updated webrev:
http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.03/

--alex

> 
> thanks,
> 
> Chris
> 
> On 3/14/18 12:43 PM, Alex Menkov wrote:
>>
>> Updated fix:
>> http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.02/
>>
>> The changes:
>> - createTransport function is fixed;
>> - "prefix" variable is renamed to "baseName".
>>
>> --alex
>>
>> On 03/14/2018 09:45, Alex Menkov wrote:
>>> Hi David,
>>>
>>>
>>> On 03/13/2018 17:46, David Holmes wrote:
>>>> Hi Alex,
>>>>
>>>> On 14/03/2018 9:14 AM, Alex Menkov wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review a small fix for
>>>>> https://bugs.openjdk.java.net/browse/JDK-8049695
>>>>> webrev: 
>>>>> http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open/
>>>>>
>>>>> Root cause of the issue is jbd hungs as a result of the buffer 
>>>>> overflow.
>>>>>
>>>>> In the beginning of the shmemBase.c:
>>>>>
>>>>> #define MAX_IPC_PREFIX 50   /* user-specified or generated name for */
>>>>>                              /* shared memory seg and prefix for 
>>>>> other IPC */
>>>>> #define MAX_IPC_SUFFIX 25   /* suffix to shmem name for other IPC 
>>>>> names */
>>>>> #define MAX_IPC_NAME   (MAX_IPC_PREFIX + MAX_IPC_SUFFIX)
>>>>>
>>>>> buffer (char prefix[]) in function createStream is used to generate 
>>>>> base name for mutex/events, so MAX_IPC_PREFIX is not big enough.
>>>>
>>>> Good catch! But overall this code seems to be missing bounds checks 
>>>> everywhere. You made the "prefix" (poor name?) buffer bigger 
>>>> (MAX_IPC_NAME) but do we know the incoming name plus the appended 
>>>> descriptive string will fit in it?
>>>
>>> Yes, the possible values can be added to the shmem name (which is 
>>> restricted by 49 chars):
>>> ".mutex"
>>> ".hasData"
>>> ".hasSpace"
>>> ".accept"
>>> ".attach"
>>> ".<pid>" (pid is 64bit value, max len IIRC is 19 symbols)
>>> So extra MAX_IPC_SUFFIX (25 symbols) is enough
>>>
>>>> Looking at createTransport for example, it also has:
>>>>
>>>> char prefix[MAX_IPC_PREFIX];
>>>>
>>>> and it produces an error if
>>>>
>>>> strlen(address) >= MAX_IPC_PREFIX
>>>>
>>>> but otherwise copies it across:
>>>>
>>>> strcpy(transport->name, address);
>>>>
>>>> and then later does:
>>>>
>>>>   sprintf(prefix, "%s.mutex", transport->name);
>>>>
>>>> so we may have overflowed again by adding ".mutex"! The same goes 
>>>> for the subsequent sprintf's.
>>>
>>> Thank you for the catch!
>>> I looked the file for other similar issues, but somehow overlokked 
>>> this case.
>>> Will fix it.
>>> Also will change confusing "prefix" name to "base_name".
>>>
>>> --alex
>>>
>>>>
>>>> So I think there is more work to do to ensure this code is immune 
>>>> from buffer overflows.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> --alex
> 
> 
>