[PATCH] wayland-server: Fix null pointer dereferencing
ASHIM SINGH SHAH
ashim.shah at samsung.com
Thu Jul 30 23:25:01 PDT 2015
if wl_resource_create() fails to create the resource (i.e. client->display_resource) and returns NULL.
For such case, the patch will post the message "no memory" for that client in "wl_resource_post_error()" and after that
it will return at below check.
I think patch is correct in above sense.
Ashim Singh Shah
------- Original Message -------
Sender : Marek Chalupa<mchqwerty at gmail.com>
Date : Jul 23, 2015 11:33 (GMT+05:30)
Title : Re: [PATCH] wayland-server: Fix null pointer dereferencing
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
>> 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:
> if (client->destroying)
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)
>> 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 <ashim.shah at samsung.com>
>>> 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;
>>> va_list ap;
>>> @@ -215,6 +215,11 @@ wl_resource_post_error(struct wl_resource
>>> vsnprintf(buffer, sizeof buffer, msg, ap);
>>> + if (resource == NULL)
>>> + return;
>>> + else
>>> + client = resource->client;
>>> client->error = 1;
More information about the wayland-devel