[PATCH] potential array overrun

[Steve Grubb]

On Tuesday, June 12, 2012 10:53:10 AM Zsbán Ambrus wrote:
> On Mon, Jun 11, 2012 at 2:43 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> > Below is a patch that fixes a theoretical array overrun. I say theoretical
> > because I don't think there is a signal number high enough to trigger
> > this. But any ways... the issue is EV_NSIG starts off being 65. Then the
> > array is declared as   signals [EV_NSIG - 1]; Which means 0-63 would be
> > valid index values. In the function ev_feed_signal_event() is a check for
> > signum > EV_NSIG...meaning that if signum is 65, it won't return. This
> > gets decremented to 64 and then used as an index for a memory write. This
> > is 1 over the top since 63 is the largest valid index. I doubt this
> > causes any problems, but a patch below should fix the issue.
> > 
> > diff -urp libev-4.11.orig/ev.c libev-4.11/ev.c
> > --- libev-4.11.orig/ev.c        2012-02-04 14:09:52.000000000 -0500
> > +++ libev-4.11/ev.c     2012-06-11 07:54:14.719520216 -0400
> > @@ -1966,7 +1966,7 @@ ev_feed_signal_event (EV_P_ int signum)
> >  {
> >   WL w;
> > 
> > -  if (expect_false (signum <= 0 || signum > EV_NSIG))
> > +  if (expect_false (signum <= 0 || signum >= EV_NSIG))
> >     return;
> > 
> >   --signum;
> 
> I think your reasoning is incorrect.  While this may seem strange, 64
> (equal to EV_NSIG - 1) is typically a valid signal number on Linux
> (it's SIGRTMAX, the lowest priority POSIX sigqueue signal).  Thus,
> libev should support watchers on this signal number.

Indeed. Thanks for pointing that out.

/usr/include/asm-generic/signal.h
#define _NSIG           64
#define SIGRTMAX        _NSIG

which means...
ev.c
#elif defined(_NSIG)
# define EV_NSIG (_NSIG)

Which means only an assert needs touching up.

diff -urp libev-4.11.orig/ev.c libev-4.11/ev.c
--- libev-4.11.orig/ev.c	2012-02-04 14:09:52.000000000 -0500
+++ libev-4.11/ev.c	2012-06-12 10:09:17.287235305 -0400
@@ -3426,7 +3426,7 @@ ev_signal_start (EV_P_ ev_signal *w)
   if (expect_false (ev_is_active (w)))
     return;
 
-  assert (("libev: ev_signal_start called with illegal signal number", w->signum > 0 && w->signum < EV_NSIG));
+  assert (("libev: ev_signal_start called with illegal signal number", w->signum > 0 && w->signum <= EV_NSIG));
 
 #if EV_MULTIPLICITY
   assert (("libev: a signal must not be attached to two different loops",