RFR: 8239563 - Reduce public exports in dynamic libraries built from static JDK libraries

[Magnus Ihse Bursie]

On 2020-02-26 08:41, David Holmes wrote:
> Hi Bob,
>
> Adding build-dev.
Thanks for noticing that we were missing, David!

>
> On 26/02/2020 6:37 am, Bob Vandette wrote:
>>
>> Please review this RFE that alters the visibility of JNI entrypoints 
>> to hidden when a shared library
>> is created using static JDK libraries.
>>
>> RFE:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8239563
>>
>> WEBREV:
>>
>> http://cr.openjdk.java.net/~bobv/8239563/webrev.00/
>>
>> CSR:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8239791
>>
>> All JNI entrypoints that exist in JDK static libraries are declared 
>> as exported or visible.
>> If a dynamic library is built from these static libraries, we end up 
>> with many exported
>> functions that we don't want to provide access to,
>>
>> This RFE will change the definition of JNIEXPORT for libraries built 
>> when JNI_STATIC_BUILD
>> is defined.  When defined, functions declared with JNIEXPORT will not 
>> be exported when
>> linked into shared or dynamic libraries.  This will still allow 
>> linking of these functions into
>> dynamic libraries but will not export the JDK symbols outside of the 
>> shared library.
>>
>> A CSR has been filed 
>> (https://bugs.openjdk.java.net/browse/JDK-8239791) to add the 
>> JNI_STATIC_BUILD
>> define support in jni.h.
>
> I have reservations about turning this into something we have to 
> expose and support, rather than being something totally handled within 
> the OpenJDK build system.
I fully agree. The JNI headers are an exported interface. Our internal 
build mechanisms have nothing to do there.

> I'm tempted to suggest the header files be adjusted to have:
>
>     #ifndef JNIEXPORT
>     <JNIEXPORT basic definitions as they are now >
>     #endif
>
> and then we define the modified JNIEXPORT via the build system when 
> doing a static build.
>
> Is that feasible?
It's definitely doable, and a far better solution.

A patch something akin to this would be needed:

diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -709,7 +709,10 @@
    # JDK libraries.
    STATIC_LIBS_CFLAGS="-DSTATIC_BUILD=1"
    if test "x$TOOLCHAIN_TYPE" = xgcc || test "x$TOOLCHAIN_TYPE" = 
xclang; then
-    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections 
-fdata-sections"
+    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -ffunction-sections \
+        -fdata-sections 
-DJNIEXPORT=__attribute__((visibility(\"hidden\")))"
+  else
+    STATIC_LIBS_CFLAGS="$STATIC_LIBS_CFLAGS -DJNIEXPORT="
    fi
    if test "x$TOOLCHAIN_TYPE" = xgcc; then
      # Disable relax-relocation to enable compatibility with older linkers

(With the reservation that I just wrote this on the fly and have not 
tested it -- things like quoting might be off. Also, I'm not sure if the 
match of
compilers is correct -- it might be the case that all compilers except 
Microsoft defines __GNUC__, so maybe the addition of this -D flag might 
need
a separate if statement to cover all our compilers correctly.)

/Magnus
>
> Thanks,
> David
>
>> BACKGROUND:
>>
>> In JDK8 the JNI specification and JDK implementation was enhanced to 
>> support static JNI libraries
>> but we didn’t consider the issue of exportibility of JNI entrypoint 
>> symbols.
>>
>>    https://bugs.openjdk.java.net/browse/JDK-8005716
>>
>> If developers use these static JDK libraries in order to produce a 
>> custom shared library, all of the
>> JNIEXPORTS will be exposed by this library even if the developer did 
>> not choose to export these.
>> This is a security issue and a potential problem if this library is 
>> mixed with other libraries containing
>> these symbols.
>>
>> Bob.
>>
>>
>>