[PATCH weston 04/11] compositor: Avoid NULL pointer dereference

[Armin Krezović]

On 20.06.2016 15:38, Pekka Paalanen wrote:
> On Sun, 19 Jun 2016 11:10:03 +0200
> Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> 
>> On 18/06/2016 19:15, Armin Krezović wrote:
>>> When there are no outputs present, an output pointer
>>> can be NULL. Dereferencing such pointer will result
>>> in a crash.
>>>
>>> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
>>> ---
>>>  src/compositor.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/compositor.c b/src/compositor.c
>>> index 37d94ec..d246046 100644
>>> --- a/src/compositor.c
>>> +++ b/src/compositor.c
>>> @@ -191,6 +191,9 @@ weston_output_mode_switch_to_native(struct weston_output *output)
>>>  	int ret;
>>>  	int mode_changed = 0, scale_changed = 0;
>>>
>>> +	if (!output)  
>>
>> Maybe do we want an assert here, and make sure the callers are properly 
>> checking for NULL beforehand?
> 
> Yeah, I kind of wonder how might one end up calling this function with
> NULL.
> 
> Armin, did you actually see it happen?

Uhm, I did see weston crash when calling weston_output_mode_switch_to_native

I just can't remember if that was with or without patch 5. I'll investigate more
and fix the possible callers not to call the mentioned function when an output is
NULL.

Thanks both for the review.

Armin.

> 
> Patch 5 fixes this already for one of the two call sites, and
> restore_output_mode() in fullscreen-shell.c would crash first if output
> was NULL.
> 
> I'd propose to drop this patch and fix the call sites for now as half
> of it is already done. The other alternative would be to move the
> output->original_mode check both callers have into
> weston_output_mode_switch_to_native() along with the NULL output check.
> That would make restore_output_mode() wrappers unnecessary. It is also
> questionable whether original_mode can even be NULL, or if it can,
> maybe it shouldn't. I think the latter might be a nice clean-up, but
> not essential at the moment.
> 
> 
> Thanks,
> pq
> 
>>> +		return -1;
>>> +
>>>  	if (!output->switch_mode)
>>>  		return -1;
>>>
>>>  
>>
>>
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 855 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160622/d6b6730d/attachment.sig>