> -----Original Message----- > From: Pekka Paalanen [mailto:ppaala...@gmail.com] > Sent: Friday, May 09, 2014 3:09 PM > To: Srivardhan > Cc: 'Hardening'; wayland-devel@lists.freedesktop.org > Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the > pointer. > > On Fri, 09 May 2014 14:56:14 +0530 > Srivardhan <sri.heb...@samsung.com> 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. > > Hi, > > I think we should be fixing the caller instead of wl_event_source_remove(). I > do not believe NULL is a valid argument for it, so the bug is in the caller. > > If any wl_event_loop_add_signal() in Weston fails, it would be a reason to > exit with an error. >
Oh... k... Will fix that and send the patch... In general isn't it fine to check for NULL before dereferencing? Thank-you, Hebbar > > Thanks, > pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel