RFR [8043720] (smartcardio) Native memory should be handled more accurately

[Valerie (Yu-Ching) Peng]

Changes look fine.
Thanks,
Valerie

On 05/28/14 14:00, Ivan Gerasimov wrote:
> One additional change:
> In pcsc_multi2jstring() it is better to make sure the whole parsed 
> buffer is null-terminates.
>
> This way we can guarantee that in the following line we will not 
> access memory outside the buffer:
>                js = (*env)->NewStringUTF(env, tab[cnt]);
>
> Would you please help review the updated webrev?
> http://cr.openjdk.java.net/~igerasim/8043720/1/webrev/
>
> Sincerely yours,
> Ivan
>
>
> On 27.05.2014 23:30, Ivan Gerasimov wrote:
>> Hello!
>>
>> Here is a proposal to make some native memory manipulations in 
>> src/share/native/sun/security/smartcardio/pcsc.c more accurate.
>>
>> 1)
>> Add another argument to pcsc_multi2jstring() which will hold the size 
>> of the character array to be parsed.
>> In the function we make sure we do not access memory outside the array.
>>
>> 2)
>> In pcsc_multi2jstring():
>> There is a constant PCSCLITE_MAX_READERS_CONTEXTS == 16, which limits 
>> number of cardreaders in the system.
>> So we don't need to allocate the buffer with malloc.
>>
>> 3)
>> In Java_sun_security_smartcardio_PCSC_SCardListReaders():
>> The 'size' variable is initialized in the pcsc-lite API call to 
>> SCardListReaders.
>> Even though it should not be zero, if  SCardListReaders returned 
>> SUCCESS, it's still a call of foreign function.
>> Thus we better handle the case when size == 0.
>>
>> 4)
>> Currently Java_sun_security_smartcardio_PCSC_SCardGetStatusChange is 
>> always called with non-empty jReaderNames argument.
>> However, it seems easy to make it more robust with a couple of checks 
>> for the empty array of names.
>>
>> 5)
>> In Java_sun_security_smartcardio_PCSC_SCardGetStatusChange, readers' 
>> names are allocated with strdup() and later freed with free().
>> If the program gets to cleanup upon an error, some of the names may 
>> turn out be be never initialized, so free() can fail.
>>
>> Would you please help review the fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8043720
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8043720/0/webrev/
>>
>> Sincerely yours,
>> Ivan
>>
>>
>>
>