RFR(xs): 8076475: Misuses of strncpy/strncat

[Dmitry Samersoff]

Thomas,

** agent/src/os/bsd/libproc_impl.c
** agent/src/os/linux/libproc_impl.c

newlib->name has the size of (PATH_MAX + NAME_MAX + 1)

see ./src/os/bsd/libproc_impl.h:92

So no truncation is actually possible here.

To make this code nice-looking it's better to add:

if (strlen(libname) >  sizeof(newlib->name)) {
//  Bail out with debug message
 ...
}
strcpy(newlib->name, libname);

rather than use snprintf here.

** arguments.cpp:3466

It might be better to use snprintf here.

** arguments.cpp:3629

It's better to just replace strncat/strncpy to strcat/strcpy
in the original code - as soon as we allocating memory with respect to
variable lenght, we don't need to check this length again.

** vmError.cpp:222

It's better to keep original code.


Other changes looks good for me!
-Dmitry


On 2015-04-08 11:09, Thomas Stüfe wrote:
> Hi,
> 
> please review these small fixes around use of strncpy/strncat.
> 
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8076475
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8076475/webrev.00/webrev/
> 
> Changes in detail are:
> 
> agent/src/os/bsd/libproc_impl.c
> agent/src/os/linux/libproc_impl.c:
>                 - missing \0 on truncation. Replaced with snprintf, add
> truncation handling
> 
> src/os/bsd/dtrace/libjvm_db.c
> src/os/solaris/dtrace/libjvm_db.c
>                 @@ -580,17 +580,18 @@
>                 - overwrite on truncation
>                 @@ -1093,13 +1094,13 @@
>                 - overwrite on truncation
> 
> src/share/vm/compiler/compileBroker.hpp
>                 - missing \0 on truncation.
> 
> src/share/tools/hsdis/hsdis.c
>                 - missing \0
> 
> src/os/bsd/vm/decoder_machO.cpp
>                 - missing \0 on truncation.
> 
> src/share/vm/compiler/compilerOracle.cpp
>                 - Replaced with jio_snprintf, less awkward and does not
> restrict each part to 255 chars each.
> 
> src/share/vm/compiler/disassembler.cpp
>                 - missing \0 on truncation.
> 
> src/share/vm/runtime/arguments.cpp
>                 @@ -2703,11 +2703,11 @@
>                 - replaced with strdup, easier to read
>                 @@ -3294,12 +3294,11 @@
>                 - the same
>                 @@ -3627,18 +3626,14 @@
>                 - replace strncpy/strncat sequence with jio_snprintf -
> easier to read.
>                 - replace malloc/strncpy with os::strdup
> 
> src/share/vm/utilities/ostream.cpp
>                 - avoid \0 padding
> 
> src/share/vm/utilities/vmError.cp
>                 @@ -219,7 +219,7 @@
>                 - avoid \0 padding
>                 @@ -463,14 +463,7 @@
>                 - simplified
> 
> Kind regards, Thomas
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.