Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/sys Convert EV_SET from macro to static __inline function



details:   https://anonhg.NetBSD.org/src/rev/3627cd754266
branches:  trunk
changeset: 824286:3627cd754266
user:      kamil <kamil%NetBSD.org@localhost>
date:      Wed May 31 00:45:59 2017 +0000

description:
Convert EV_SET from macro to static __inline function

LLDB introduced support for kevent(2) and it contains the following function:

Status MainLoop::RunImpl::Poll() {
  in_events.resize(loop.m_read_fds.size());
  unsigned i = 0;
  for (auto &fd : loop.m_read_fds)
    EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
  num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
                      out_events, llvm::array_lengthof(out_events), nullptr);
  if (num_events < 0)
    return Status("kevent() failed with error %d\n", num_events);
  return Status();
}

It works on FreeBSD and MacOSX, however it broke on NetBSD.

Culrpit line:
   EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);

FreeBSD defined EV_SET() as a macro this way:
#define EV_SET(kevp_, a, b, c, d, e, f) do {    \
        struct kevent *kevp = (kevp_);          \
        (kevp)->ident = (a);                    \
        (kevp)->filter = (b);                   \
        (kevp)->flags = (c);                    \
        (kevp)->fflags = (d);                   \
        (kevp)->data = (e);                     \
        (kevp)->udata = (f);                    \
} while(0)

NetBSD version was different:
#define EV_SET(kevp, a, b, c, d, e, f)                                  \
do {                                                                    \
        (kevp)->ident = (a);                                            \
        (kevp)->filter = (b);                                           \
        (kevp)->flags = (c);                                            \
        (kevp)->fflags = (d);                                           \
        (kevp)->data = (e);                                             \
        (kevp)->udata = (f);                                            \
} while (/* CONSTCOND */ 0)

This resulted in heap damage, as keyp was incremented every time value was
assigned to (keyp)->.

As suggested by Joerg, convert this macro on NetBSD to static __inline
function.

Credit to <coypu> for asan+ubsan research wiki entry that helped to narrow
down the bug.
Credit to <joerg> for peer-review

Sponsored by <The NetBSD Foundation>

diffstat:

 sys/sys/event.h |  25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diffs (46 lines):

diff -r 7c86c4cfd336 -r 3627cd754266 sys/sys/event.h
--- a/sys/sys/event.h   Wed May 31 00:22:06 2017 +0000
+++ b/sys/sys/event.h   Wed May 31 00:45:59 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: event.h,v 1.26 2016/01/31 04:40:01 christos Exp $      */
+/*     $NetBSD: event.h,v 1.27 2017/05/31 00:45:59 kamil Exp $ */
 
 /*-
  * Copyright (c) 1999,2000,2001 Jonathan Lemon <jlemon%FreeBSD.org@localhost>
@@ -45,17 +45,6 @@
 #define        EVFILT_TIMER            6U      /* arbitrary timer (in ms) */
 #define        EVFILT_SYSCOUNT         7U      /* number of filters */
 
-#define        EV_SET(kevp, a, b, c, d, e, f)                                  \
-do {                                                                   \
-       (kevp)->ident = (a);                                            \
-       (kevp)->filter = (b);                                           \
-       (kevp)->flags = (c);                                            \
-       (kevp)->fflags = (d);                                           \
-       (kevp)->data = (e);                                             \
-       (kevp)->udata = (f);                                            \
-} while (/* CONSTCOND */ 0)
-
-
 struct kevent {
        uintptr_t       ident;          /* identifier for this event */
        uint32_t        filter;         /* filter for event */
@@ -65,6 +54,18 @@
        intptr_t        udata;          /* opaque user data identifier */
 };
 
+static __inline void
+EV_SET(struct kevent *_kevp, uintptr_t _ident, uint32_t _filter,
+       uint32_t _flags, uint32_t _fflags, int64_t _data, intptr_t _udata)
+{
+       _kevp->ident = _ident;
+       _kevp->filter = _filter;
+       _kevp->flags = _flags;
+       _kevp->fflags = _fflags;
+       _kevp->data = _data;
+       _kevp->udata = _udata;
+}
+
 /* actions */
 #define        EV_ADD          0x0001U         /* add event to kq (implies ENABLE) */
 #define        EV_DELETE       0x0002U         /* delete event from kq */



Home | Main Index | Thread Index | Old Index