Skip to content

Commit

Permalink
8315074: Possible null pointer access in native glass
Browse files Browse the repository at this point in the history
Reviewed-by: jvos, kcr
  • Loading branch information
jayathirthrao authored and kevinrushforth committed Sep 13, 2023
1 parent 7c8dd1e commit f7b21e5
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 35 deletions.
Expand Up @@ -262,8 +262,13 @@ JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_GtkApplication__1submitForLater
(void)obj;

RunnableContext* context = (RunnableContext*)malloc(sizeof(RunnableContext));
context->runnable = env->NewGlobalRef(runnable);
gdk_threads_add_idle_full(G_PRIORITY_HIGH_IDLE + 30, call_runnable, context, NULL);
if (context != NULL) {
context->runnable = env->NewGlobalRef(runnable);
gdk_threads_add_idle_full(G_PRIORITY_HIGH_IDLE + 30, call_runnable, context, NULL);
// we release this context in call_runnable
} else {
fprintf(stderr, "malloc failed in GtkApplication__1submitForLaterInvocation\n");
}
}

/*
Expand Down
14 changes: 10 additions & 4 deletions modules/javafx.graphics/src/main/native-glass/gtk/GlassTimer.cpp
Expand Up @@ -45,10 +45,16 @@ JNIEXPORT jlong JNICALL Java_com_sun_glass_ui_gtk_GtkTimer__1start
(void)obj;

RunnableContext* context = (RunnableContext*) malloc(sizeof(RunnableContext));
context->runnable = env->NewGlobalRef(runnable);
context->flag = 0;
gdk_threads_add_timeout_full(G_PRIORITY_HIGH_IDLE, period, call_runnable_in_timer, context, NULL);
return PTR_TO_JLONG(context);
if (context != NULL) {
context->runnable = env->NewGlobalRef(runnable);
context->flag = 0;
gdk_threads_add_timeout_full(G_PRIORITY_HIGH_IDLE, period, call_runnable_in_timer, context, NULL);
return PTR_TO_JLONG(context);
} else {
// we throw RuntimeException on Java side when we can't
// start the timer
return 0L;
}
}

/*
Expand Down
Expand Up @@ -96,6 +96,12 @@ struct jni_exception: public std::exception {
jstring jmessage;
};

#define SAFE_FREE(PTR) \
if ((PTR) != NULL) { \
free(PTR); \
(PTR) = NULL; \
}

#define EXCEPTION_OCCURED(env) (check_and_clear_exception(env))

#define CHECK_JNI_EXCEPTION(env) \
Expand Down
Expand Up @@ -59,6 +59,12 @@ bool WindowContextBase::im_filter_keypress(GdkEventKey* event) {
buffer = (char*)malloc(buf_len * sizeof (char));
}

if (buffer == NULL) {
// dont process the key event
fprintf(stderr, "malloc failed in im_filter_keypress\n");
return false;
}

KeySym keysym;
Status status;
XKeyPressedEvent xevent = convert_event(event);
Expand All @@ -74,7 +80,14 @@ bool WindowContextBase::im_filter_keypress(GdkEventKey* event) {
int len = Xutf8LookupString(xim.ic, &xevent, buffer, buf_len - 1, &keysym, &status);
if (status == XBufferOverflow) {
buf_len = len + 1;
buffer = (char*)realloc(buffer, buf_len * sizeof (char));
char *tmpBuffer = (char*)realloc(buffer, buf_len * sizeof (char));
if (tmpBuffer == NULL) {
SAFE_FREE(buffer);
// dont process the key event
fprintf(stderr, "realloc failed in im_filter_keypress\n");
return false;
}
buffer = tmpBuffer;
len = Xutf8LookupString(xim.ic, &xevent, buffer, buf_len - 1,
&keysym, &status);
}
Expand Down
32 changes: 22 additions & 10 deletions modules/javafx.graphics/src/main/native-glass/ios/GlassMacros.h
Expand Up @@ -73,22 +73,34 @@ do {
GlassThreadData *_GlassThreadData = (GlassThreadData*)pthread_getspecific(GlassThreadDataKey); \
if (_GlassThreadData == NULL) \
{ \
_GlassThreadData = malloc(sizeof(GlassThreadData)); \
memset(_GlassThreadData, 0x00, sizeof(GlassThreadData)); \
pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \
_GlassThreadData = calloc(1, sizeof(GlassThreadData)); \
if (_GlassThreadData != NULL) \
{ \
pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \
} \
else \
{ \
fprintf(stderr, "malloc failed in GLASS_POOL_ENTER\n"); \
} \
} \
assert(_GlassThreadData->counter >= 0); \
if (_GlassThreadData->counter++ == 0) \
if (_GlassThreadData != NULL) \
{ \
_GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \
assert(_GlassThreadData->counter >= 0); \
if (_GlassThreadData->counter++ == 0) \
{ \
_GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \
} \
}
#define GLASS_POOL_EXIT \
if (--_GlassThreadData->counter == 0) \
if (_GlassThreadData != NULL) \
{ \
[_GlassThreadData->pool drain]; \
_GlassThreadData->pool = nil; \
if (--_GlassThreadData->counter == 0) \
{ \
[_GlassThreadData->pool drain]; \
_GlassThreadData->pool = nil; \
} \
assert(_GlassThreadData->counter >= 0); \
} \
assert(_GlassThreadData->counter >= 0); \
}

// variations of GLASS_POOL_ENTER/GLASS_POOL_EXIT allowing them to be used accross different calls
Expand Down
Expand Up @@ -362,6 +362,7 @@ - (void)application:(NSApplication *)theApplication openFiles:(NSArray *)filenam
LOG("GlassApplication:application:openFiles");

GET_MAIN_JENV;
NSApplicationDelegateReply reply = NSApplicationDelegateReplySuccess;
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
{
NSUInteger count = [filenames count];
Expand All @@ -371,21 +372,26 @@ - (void)application:(NSApplication *)theApplication openFiles:(NSArray *)filenam
}
jobjectArray files = (*env)->NewObjectArray(env, (jsize)count, stringClass, NULL);
GLASS_CHECK_EXCEPTION(env);
for (NSUInteger i=0; i<count; i++)
{
NSString *file = [filenames objectAtIndex:i];
if (file != nil)
if (files != NULL) {
for (NSUInteger i=0; i<count; i++)
{
(*env)->SetObjectArrayElement(env, files, (jsize)i, (*env)->NewStringUTF(env, [file UTF8String]));
GLASS_CHECK_EXCEPTION(env);
NSString *file = [filenames objectAtIndex:i];
if (file != nil)
{
(*env)->SetObjectArrayElement(env, files, (jsize)i, (*env)->NewStringUTF(env, [file UTF8String]));
GLASS_CHECK_EXCEPTION(env);
}
}
(*env)->CallVoidMethod(env, self->jApplication, [GlassHelper ApplicationNotifyOpenFilesMethod], files);
} else {
fprintf(stderr, "NewObjectArray failed in GlassApplication_application\n");
reply = NSApplicationDelegateReplyFailure;
}
(*env)->CallVoidMethod(env, self->jApplication, [GlassHelper ApplicationNotifyOpenFilesMethod], files);
}
[pool drain];
GLASS_CHECK_EXCEPTION(env);

[theApplication replyToOpenOrPrint:NSApplicationDelegateReplySuccess];
[theApplication replyToOpenOrPrint:reply];
}

- (BOOL)application:(NSApplication *)theApplication openFile:(NSString *)filename
Expand Down
3 changes: 3 additions & 0 deletions modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m
Expand Up @@ -461,6 +461,9 @@ jcharArray GetJavaKeyChars(JNIEnv *env, NSEvent *event)
jchar jc[16];
[chars getCharacters:jc range:NSMakeRange(0, [chars length])];
jcharArray jChars = (*env)->NewCharArray(env, (jsize)[chars length]);
if (jChars == NULL) {
return NULL;
}
(*env)->SetCharArrayRegion(env, jChars, 0, (jsize)[chars length], jc);
GLASS_CHECK_EXCEPTION(env);
return jChars;
Expand Down
32 changes: 22 additions & 10 deletions modules/javafx.graphics/src/main/native-glass/mac/GlassMacros.h
Expand Up @@ -177,22 +177,34 @@ do {
GlassThreadData *_GlassThreadData = (GlassThreadData*)pthread_getspecific(GlassThreadDataKey); \
if (_GlassThreadData == NULL) \
{ \
_GlassThreadData = malloc(sizeof(GlassThreadData)); \
memset(_GlassThreadData, 0x00, sizeof(GlassThreadData)); \
pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \
_GlassThreadData = calloc(1, sizeof(GlassThreadData)); \
if (_GlassThreadData != NULL) \
{ \
pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \
} \
else \
{ \
fprintf(stderr, "malloc failed in GLASS_POOL_ENTER\n"); \
} \
} \
assert(_GlassThreadData->counter >= 0); \
if (_GlassThreadData->counter++ == 0) \
if (_GlassThreadData != NULL) \
{ \
_GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \
assert(_GlassThreadData->counter >= 0); \
if (_GlassThreadData->counter++ == 0) \
{ \
_GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \
} \
}
#define GLASS_POOL_EXIT \
if (--_GlassThreadData->counter == 0) \
if (_GlassThreadData != NULL) \
{ \
[_GlassThreadData->pool drain]; \
_GlassThreadData->pool = nil; \
if (--_GlassThreadData->counter == 0) \
{ \
[_GlassThreadData->pool drain]; \
_GlassThreadData->pool = nil; \
} \
assert(_GlassThreadData->counter >= 0); \
} \
assert(_GlassThreadData->counter >= 0); \
}

// variations of GLASS_POOL_ENTER/GLASS_POOL_EXIT allowing them to be used accross different calls
Expand Down

5 comments on commit f7b21e5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayathirthrao
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jfx21u

@openjdk
Copy link

@openjdk openjdk bot commented on f7b21e5 Sep 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayathirthrao the backport was successfully created on the branch jayathirthrao-backport-f7b21e54 in my personal fork of openjdk/jfx21u. To create a pull request with this backport targeting openjdk/jfx21u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit f7b21e54 from the openjdk/jfx repository.

The commit being backported was authored by Jayathirth D V on 13 Sep 2023 and was reviewed by Johan Vos and Kevin Rushforth.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jfx21u:

$ git fetch https://github.com/openjdk-bots/jfx21u.git jayathirthrao-backport-f7b21e54:jayathirthrao-backport-f7b21e54
$ git checkout jayathirthrao-backport-f7b21e54
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jfx21u.git jayathirthrao-backport-f7b21e54

@kevinrushforth
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/tag 22+9

@openjdk
Copy link

@openjdk openjdk bot commented on f7b21e5 Sep 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinrushforth The tag 22+9 was successfully created.

Please sign in to comment.