RFR: JDK-8007607

[John Zavgren]

Greetings:
I posted a new webrev image.
http://cr.openjdk.java.net/~jzavgren/8007607/webrev.04/ 
<http://cr.openjdk.java.net/%7Ejzavgren/8007607/webrev.04/>

The code now throws an OOM exception when *alloc() fails, and the 
callers of procedures that call procedures that throw it, check for it.

John

On 02/12/2013 11:03 AM, Dmitry Samersoff wrote:
> 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.zavgren at 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
>>
>


-- 
John Zavgren
john.zavgren at oracle.com
603-821-0904
US-Burlington-MA

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20130218/486e5ec0/attachment.htm>