On Fri, May 09, 2014 at 02:56:14PM +0530, Srivardhan wrote: > > > > -----Original Message----- > > From: wayland-devel [mailto:wayland-devel- > > boun...@lists.freedesktop.org] On Behalf Of Hardening > > Sent: Friday, May 09, 2014 1:08 PM > > To: wayland-devel@lists.freedesktop.org > > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the > > pointer. > > > > Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : > > > Checking for NULL before dereferencing the wl_event_source pointer so > > > as to avoid crash. > > > > > > Signed-off-by: Srivardhan Hebbar <sri.heb...@samsung.com> > > > --- > > > src/event-loop.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/event-loop.c b/src/event-loop.c index > > > 9790cde..b62d16e 100644 > > > --- a/src/event-loop.c > > > +++ b/src/event-loop.c > > > @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source > > *source) > > > WL_EXPORT int > > > wl_event_source_remove(struct wl_event_source *source) > > > { > > > - struct wl_event_loop *loop = source->loop; > > > + struct wl_event_loop *loop; > > > + > > > + if (source == NULL) > > > + return 0; > > > + > > > + loop = source->loop; > > > > > > /* We need to explicitly remove the fd, since closing the fd > > > * isn't enough in case we've dup'ed the fd. */ > > > > > > > Hello Srivardhan, > > > > do you have a case where this check is hit ? I may be wrong but having no > > loop associated with a source event is not supposed to happen. So my guess > > is that a caller of wl_event_source_remove has forgotten to nullify the > event > > source after the call. > > Hi, > > I was going through the code of Weston. In the main function in compositor.c > wl_event_loop_add_signal() is called to allocate the memory for > signals[](Line no: 4196. struct wl_event_source *signals[4]). The function > returns NULL if memory allocation failed. After that there is no NULL check > for 'signals'. In the end while clearing up, this function is called. So if > memory allocation failed then a NULL is passed to this function. Hence > adding code to check for the NULL.
You caught a problem here, but the issue is that we don't check for NULL where we allocate the source. Passing a NULL pointer to wl_event_source_remove() is a programmer error and we don't want to silently fail. Kristian > Thank-you, > Hebbar > > > > Regards. > > > > -- > > David FORT > > website: http://www.hardening-consulting.com/ > > _______________________________________________ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel