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
