2023-06-30 05:27:03

by Andrei Vagin

Subject: [PATCH] perf/bench/seccomp-notify: don't use assert to check syscall errors

Since 616b14b47a86, perf is built with NDEBUG=1 so the macro
assert() is nop.

Signed-off-by: Andrei Vagin <[email protected]>
---
tools/perf/bench/sched-seccomp-notify.c | 35 +++++++++++++++----------
1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/perf/bench/sched-seccomp-notify.c b/tools/perf/bench/sched-seccomp-notify.c
index eac4ef60090f..73ab86269427 100644
--- a/tools/perf/bench/sched-seccomp-notify.c
+++ b/tools/perf/bench/sched-seccomp-notify.c
@@ -22,9 +22,7 @@
#include <sys/wait.h>
#include <string.h>
#include <errno.h>
-/* keep all assert() calls. */
-#undef NDEBUG
-#include <assert.h>
+#include <err.h>

#define LOOPS_DEFAULT 1000000UL
static uint64_t loops = LOOPS_DEFAULT;
@@ -74,15 +72,18 @@ static void user_notification_sync_loop(int listener)

for (nr = 0; nr < loops; nr++) {
memset(&req, 0, sizeof(req));
- assert(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req) == 0);
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req))
+ err(EXIT_FAILURE, "SECCOMP_IOCTL_NOTIF_RECV failed");

- assert(req.data.nr == __NR_gettid);
+ if (req.data.nr != __NR_gettid)
+ errx(EXIT_FAILURE, "unexpected syscall: %d", req.data.nr);

resp.id = req.id;
resp.error = 0;
resp.val = USER_NOTIF_MAGIC;
resp.flags = 0;
- assert(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp) == 0);
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp))
+ err(EXIT_FAILURE, "SECCOMP_IOCTL_NOTIF_SEND failed");
}
}

@@ -105,12 +106,15 @@ int bench_sched_seccomp_notify(int argc, const char **argv)
prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
listener = user_notif_syscall(__NR_gettid,
SECCOMP_FILTER_FLAG_NEW_LISTENER);
- assert(listener >= 0);
+ if (listener < 0)
+ err(EXIT_FAILURE, "can't create a notification descriptor");

pid = fork();
- assert(pid >= 0);
+ if (pid < 0)
+ err(EXIT_FAILURE, "fork");
if (pid == 0) {
- assert(prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0) == 0);
+ if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0))
+ err(EXIT_FAILURE, "can't set the parent death signal");
while (1) {
ret = syscall(__NR_gettid);
if (ret == USER_NOTIF_MAGIC)
@@ -121,15 +125,18 @@ int bench_sched_seccomp_notify(int argc, const char **argv)
}

if (sync_mode) {
- assert(ioctl(listener, SECCOMP_IOCTL_NOTIF_SET_FLAGS,
- SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP, 0) == 0);
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_SET_FLAGS,
+ SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP, 0))
+ err(EXIT_FAILURE,
+ "can't set SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP");
}
user_notification_sync_loop(listener);

kill(pid, SIGKILL);
- assert(waitpid(pid, &status, 0) == pid);
- assert(WIFSIGNALED(status));
- assert(WTERMSIG(status) == SIGKILL);
+ if (waitpid(pid, &status, 0) != pid)
+ err(EXIT_FAILURE, "waitpid(%d) failed", pid);
+ if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL)
+ errx(EXIT_FAILURE, "unexpected exit code: %d", status);

gettimeofday(&stop, NULL);
timersub(&stop, &start, &diff);
--
2.40.1

2023-07-03 20:17:52

by Kees Cook

Subject: Re: [PATCH] perf/bench/seccomp-notify: don't use assert to check syscall errors

On Thu, Jun 29, 2023 at 10:19:53PM -0700, Andrei Vagin wrote:
> Since 616b14b47a86, perf is built with NDEBUG=1 so the macro
> assert() is nop.
>
> Signed-off-by: Andrei Vagin <[email protected]>

Thanks! I've squashed this onto the patch that introduces the benchmark.

--
Kees Cook