Hi Alex,
On 21/03/2018 3:25 AM, Alex Menkov wrote:
Hi David,
On 03/19/2018 18:10, David Holmes wrote:
Hi Alex,
On 20/03/2018 10:28 AM, Alex Menkov wrote:
Hi guys,
please re-review the fix.
I still have an unanswered question about where the max of 49 is
enforced. I see it for the "address" but not names in general. ??
for shmem the "channel name" is the address (it's checked in
createTransport/openTransport).
Names for mutexes/events are generated by appending some strings to the
adddress and length of the added parts are supposed to be less than
MAX_IPC_SUFFIX (25 symbols):
".mutex" (+ up to 3 symbols)
".hasData" (+ up to 3 symbols)
".hasSpace" (+ up to 3 symbols)
".ctos"
".stoc"
".accept" (+ up to 3 symbols)
".attach" (+ up to 3 symbols)
".<pid>" (pid is a DWORD)
Okay so ... the code in shmemBase.c is very unclear as to which "names"
can come in from an external source and which are only ever derived from
other "names". If the "address" (which seems a very bad description in
this case!) is the only external source for a name, and it is limited to
a length of 49 then that is okay.
Reg.test is added the the issue.
I don't quite follow the test. I see you try to set the name with a
value that is too long, and if that doesn't cause an overflow and we
don't crash that is good. But I'd expect you to read back the name and
check it matches the truncated name with 49 characters.
The test specifies the maximum length supported (49 symbols)
(if longer name is specified, "address strings longer than 50 characters
are invalid" error reported).
I missed the substring that simply causes the name to be the maximum
supported length. That would trigger the overflow and so suffices as a
regression test for this fix.
Is there another test that already passes a too-long name and verifies
the error gets thrown?
As far as I see there is no way to read back the name used to create the
transport.
Ok.
Thanks,
David
-----
--alex
Thanks,
David
webrev:
http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.04/
--alex
On 03/13/2018 16:14, 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.
--alex
