Skip to content

Commit

Permalink
Avoid theoretically possible null pointer dereference in callbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
@spk121
spk121 committed Dec 18, 2020
1 parent 8bda5c9 commit 858e062
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/gig_callback.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,8 @@ gig_callback_to_scm(GICallbackInfo *info, gpointer callback)
// we probably shouldn't cache this, because C callbacks can be
// invalidated
GigCallback *gcb = gig_callback_new_for_callback(info, callback);
if (gcb == NULL)
return SCM_BOOL_F;
return scm_c_make_gsubr("(anonymous)", 0, 0, 1, gcb->callback_ptr);
}

Expand Down

4 comments on commit 858e062

@LordYuuma
Copy link
Collaborator

@LordYuuma LordYuuma commented on 858e062 Dec 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Could we throw an SCM memerror here?

@spk121
Copy link
Owner Author

@spk121 spk121 commented on 858e062 Dec 18, 2020

Choose a reason for hiding this comment

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

gcc's -fanalyzer flagged this. It was concerned about the case when ffi_prep_cif returns NULL. I'm not sure if the way that ffi_prep_cif is called can result in a NULL return value.
We could throw an error. I don't think it is a specifically a memerror.

@LordYuuma
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed, ffi_prep_cif specifies some error conditions, don't seem too relevant to our use case, though. We could just as well assert, that gcb != NULL if that's the only reason why it would be NULL (but realistically speaking, memory can always be an issue, no?)

@spk121
Copy link
Owner Author

@spk121 spk121 commented on 858e062 Dec 18, 2020

Choose a reason for hiding this comment

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

Even though most of these ffi errors should be rare or impossible, I think I'll rework the error reporting for this file and have it throw errors for all the ffi-reported failure conditions.

Please sign in to comment.