On 07/16/2015 09:37 AM, Marek Chalupa wrote:
On 07/16/2015 09:27 AM, Marek Chalupa wrote:
Nice catch (some static analysis tool? :).
I'm not sure if this is the right way to fix it, though. If somebody
calls wl_resource_post_error with NULL resource, it is his fault and
program should crash to reveal this error. So the right fix in this case
would be to delete wl_client_post_no_memory() on line 783 in
bind_display.
Can wl_resource_post_error be called on some other place with NULL?
The only thing I can make up is when destroying client - resources are
being destroyed and the first is destroyed display_resource, thus if
any destroy handler after that calls wl_client_post_no_memory, we get
wl_resource_post_error(NULL, ...).
So the fix is correct in the sense that there are paths with resource =
NULL. On the other hand, we'd like to discover cases where programmer
passes NULL. Hmm, maybe something like client->destroying + NULL checks
could solve this. Or we'll just go with this patch - since the NULL
paths are (I hope :) only on client destruction, this should be ok.
Or we could add check to wl_client_post_no_memory(). That would solve
the problem when client is being destroyed and wl_resource_post_error
would be untouched and passing NULL would crash the program as "desired".
Hmm.. Still keep the though with client->destroying. If we did:
wl_client_post_no_memory(client)
{
if (client->destroying)
return;
wl_resource_post_error(...);
}
Found and read this one again, so one more comment in my monologue :)
Flag client->destroying is useless since client->display_resource being
NULL has the same semantics, so we could use that.
Then all the programming errors would go through, reveling error in
program, yet calling this on client destruction would be safe.
(We still need to fix bind_display with this approach)
Cheers,
Marek
On 07/16/2015 08:27 AM, Ashim wrote:
Initialising 'wl_client *client = NULL' and checking 'resource' for
NULL and returning if found.
This patch will avoid dereferencing of 'resource' if NULL
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=91356
Signed-off-by: Ashim <[email protected]>
---
src/wayland-server.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 0f04f66..2a0d19c 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -207,7 +207,7 @@ WL_EXPORT void
wl_resource_post_error(struct wl_resource *resource,
uint32_t code, const char *msg, ...)
{
- struct wl_client *client = resource->client;
+ struct wl_client *client = NULL;
char buffer[128];
va_list ap;
@@ -215,6 +215,11 @@ wl_resource_post_error(struct wl_resource
*resource,
vsnprintf(buffer, sizeof buffer, msg, ap);
va_end(ap);
+ if (resource == NULL)
+ return;
+ else
+ client = resource->client;
+
client->error = 1;
/*
Cheers,
Marek
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
