Well, although it's huge effort to fix and catch all the malloc calls, I
think it's still better to start somewhere. Note that for the list of
sources that you have in your webrev, they aren't linked against libjvm
and libjava, and thus you need to use local utility method(s) for
throwing the OOM error. Just like the PKCS11 native code that I
mentioned earlier.
Valerie
On 02/12/13 09:11, John Zavgren wrote:
Thanks, Dmitry.
If I were to rewrite the native code to throw exceptions, in the areas I
indicated, and no one caught them, would that create any problems. I have the
code in my sandbox right now for throwing them. If I can retain that in a
graceful way, that seems like a prudent thing to do, if there are no negative
consequences.
John
----- Original Message -----
From: dmitry.samers...@oracle.com
To: john.zavg...@oracle.com
Cc: security-dev@openjdk.java.net
Sent: Tuesday, February 12, 2013 11:04:28 AM GMT -05:00 US/Canada Eastern
Subject: Re: RFR: JDK-8007607
John,
Changing everything for throw OOM is the right goal but it's a huge work
to do - it's meaningless to throw OOM if all callers doesn't check for
exception.
I'm in doubt it could be done all at once and probably this problem
should go to the huge TODO pile.
So I'm speaking about *one particular case*, where returned value could
lead to misinterpretation of security context and lead to security
vulnerability or subtle bug.
We have to throw OOM there and change all callers as well for this case.
-Dmitry
On 2013-02-12 19:51, John Zavgren wrote:
On 02/08/2013 01:34 PM, Dmitry Samersoff wrote:
John,
Ideas?
It's a JNI so just throw OOM.
-Dmitry
On 2013-02-08 21:38, John Zavgren wrote:
Although I agree that the name: "GSS_C_NO_CHANNEL_BINDINGS" is
misleading,
I can't identify anything else that seems more appropriate.
The header file:
/jdk8-tl/jdk/src/share/native/sun/security/jgss/wrapper/gssapi.h defines
GSS_C_NO_CHANNEL_BINDINGS as follows:
#define GSS_C_NO_CHANNEL_BINDINGS ((gss_channel_bindings_t) 0)
The symbol matches the prototype of the function:
*/*
* Utility routine which creates a gss_channel_bindings_t structure
* using the specified org.ietf.jgss.ChannelBinding object.
*/
gss_channel_bindings_t getGSSCB(JNIEnv *env, jobject jcb) {
gss_channel_bindings_t cb;
jobject jinetAddr;
jbyteArray value;
if (jcb == NULL) {
return GSS_C_NO_CHANNEL_BINDINGS;
}
cb = malloc(sizeof(struct gss_channel_bindings_struct));
if(cb == NULL)
return GSS_C_NO_CHANNEL_BINDINGS;*
There doesn't appear to be anything in our set of options that is more
suggestive of a memory allocation failure and the symbol:
GSS_C_NO_CHANNEL_BINDINGS seems to be logically correct.
Ideas?
On 02/06/2013 04:57 AM, Dmitry Samersoff wrote:
John,
Not sure GSS_C_NO_CHANNEL_BINDINGS; is correct return value for this
case.
I'm second to Valerie - it's better to throw OOM
-Dmitry
On 2013-02-06 03:44, John Zavgren wrote:
Greetings:
I modified the native code to eliminate potential memory loss and
crashes by checking the return values of malloc() and realloc() calls.
The webrev image of these changes is visible at:
http://cr.openjdk.java.net/~jzavgren/8007607/webrev.01/
Thanks!
John Zavgren
--
John Zavgren
john.zavg...@oracle.com
603-821-0904
US-Burlington-MA
When I change the procedures in the following files:
src/share/native/sun/security/jgss/wrapper/GSSLibStub.c
src/share/native/sun/security/jgss/wrapper/NativeUtil.c
src/share/native/sun/security/smartcardio/pcsc.c
src/solaris/native/com/sun/security/auth/module/Solaris.c
src/solaris/native/com/sun/security/auth/module/Unix.c
to fix inappropriate usages of malloc, realloc, etc. (e.g., not checking
the return value and referring to a NULL pointer and crashing) should I
modify every instance so that an OOM (Out Of Memory) exception is thrown
(before returning) whenever memory allocation fails?
The exceptions would be thrown by a line of code that looks like:
throwOutOfMemoryError(JNIEnv *env, const char *msg);
where throwOutOfMemoryError(...) wraps something like this:
jclass cls = (*env)->FindClass(env, name);
if (cls != 0) /* Otherwise an exception has already been
thrown */
(*env)->ThrowNew(env, cls, msg);
If this is the right idea, what messages should I pass when an OOM
exception is thrown?
Thanks!
John
