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
