New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renderer-backend-egl.c: check interface pointer before usage #30

Merged
merged 1 commit into from Oct 23, 2018

Conversation

2 participants
@macpijan
Copy link
Contributor

macpijan commented Oct 15, 2018

The interface is used without checking for a valid address. In some
cases it may lead to NULL dereference and segmentation fault.
This was the case with the wpebackend-fdo which was returning nullptr
when requesting for the _wpe_renderer_backend_egl_offscreen_target_interface.

Igalia/meta-webkit#31 (comment)

Signed-off-by: Maciej Pijanowski maciej.pijanowski@3mdeb.com

renderer-backend-egl.c: check interface pointer before usage
The interface is used without checking for a valid address. In some
cases it may lead to NULL dereference and segmentation fault.
This was the case with the wpebackend-fdo which was returning nullptr
when requesting for the _wpe_renderer_backend_egl_offscreen_target_interface.

Igalia/meta-webkit#31 (comment)

Signed-off-by: Maciej Pijanowski <maciej.pijanowski@3mdeb.com>

@aperezdc aperezdc requested a review from zdobersek Oct 17, 2018

@aperezdc
Copy link
Contributor

aperezdc left a comment

I think the changes are fine, but I wonder if there's some reason why the function was using return 0; instead of return NULL;. Let's wait for @zdobersek's comment on that before merging this PR.

@@ -39,6 +39,9 @@ wpe_renderer_backend_egl_create(int host_fd)
return 0;

backend->interface = wpe_load_object("_wpe_renderer_backend_egl_interface");
if (!backend->interface)
return 0;

This comment has been minimized.

@aperezdc

aperezdc Oct 17, 2018

Contributor

I am somewhat surprised that return NULL; was not being used in this function before, which IMHO would have been more correct 🙃

Probably it's a good moment to start using NULL for early returns in this function.

This comment has been minimized.

@macpijan

macpijan Oct 17, 2018

Contributor

@aperezdc Yes I think returning NULL would make sense. I'm not that much familiar with the project and was just following the existing style.

This comment has been minimized.

@aperezdc

aperezdc Oct 18, 2018

Contributor

I think we can leave this with return 0; in this PR (to keep things consistent) and then if we want to change the code to use NULL make a follow-up PR with that change.

If @zdobersek does not comment in the next couple of days, I say let's merge this PR as-is 😉

@aperezdc aperezdc merged commit 093645c into WebPlatformForEmbedded:master Oct 23, 2018

@aperezdc

This comment has been minimized.

Copy link
Contributor

aperezdc commented Oct 23, 2018

Merged, thanks @macpijan for the contribution!

@aperezdc aperezdc added this to the Version 1.0.1 milestone Nov 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment