Skip to content

Commit

Permalink
Refactor AppScope SIGUSR2 handling
Browse files Browse the repository at this point in the history
- Separate the `SIGUSR2` related API into the `sig.c` file
- Add logic to differentiate the source of the `SIGUSR2` signal
- Ensure that the `SIGUSR2` AppScope handler (`threadNow`) always handles
  the `SIGUSR2` signal
- If the `SIGUSR2` signal originates from outside the AppScope library and
  is handled by `threadNow`, invoke the application's SIGUSR2 handler
  instead

Fixes #1529
  • Loading branch information
michalbiesek committed Jun 23, 2023
1 parent 9602a10 commit a689b96
Show file tree
Hide file tree
Showing 13 changed files with 294 additions and 106 deletions.
1 change: 1 addition & 0 deletions os/linux/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ libtest: $(LIBRARY_C_FILES) $(LIBRARY_TEST_C_FILES) $(YAML_AR) $(JSON_AR) $(TEST
$(CC) $(TEST_CFLAGS) -o test/$(OS)/vdsotest vdsotest.o scopestdlib.o dbg.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
$(CC) $(TEST_CFLAGS) -o test/$(OS)/coredumptest coredumptest.o coredump.o scopestdlib.o dbg.o utils.o fn.o plattime.o os.o scopeelf.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
$(CC) $(TEST_CFLAGS) -o test/$(OS)/ipctest ipctest.o ipc.o ipc_resp.o cfgutils.o cfg.o mtc.o log.o evtformat.o ctl.o transport.o backoff.o mtcformat.o strset.o com.o scopestdlib.o dbg.o circbuf.o linklist.o fn.o utils.o os.o test.o report.o search.o httpagg.o state.o httpstate.o metriccapture.o plattime.o scopeelf.o $(TEST_AR) $(TEST_LD_FLAGS) -Wl,--wrap=jsonConfigurationObject -Wl,--wrap=doAndReplaceConfig
$(CC) $(TEST_CFLAGS) -o test/$(OS)/sigtest sigtest.o sig.o fn.o utils.o plattime.o os.o scopestdlib.o dbg.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
$(CC) $(TEST_CFLAGS) -o test/$(OS)/snapshottest snapshottest.o snapshot.o coredump.o scopestdlib.o dbg.o utils.o fn.o plattime.o os.o scopeelf.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
$(CC) $(TEST_CFLAGS) -o test/$(OS)/ostest ostest.o scopestdlib.o dbg.o fn.o utils.o plattime.o os.o scopeelf.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
$(CC) $(TEST_CFLAGS) -o test/$(OS)/ocitest ocitest.o oci.o scopestdlib.o dbg.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
Expand Down
2 changes: 1 addition & 1 deletion os/linux/aarch64.mk
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ARCH_OBJ=$(ARCH)
LD_FLAGS=$(MUSL_AR) $(UNWIND_AR) $(COREDUMPER_AR) $(PCRE2_AR) $(LS_HPACK_AR) $(YAML_AR) $(JSON_AR) -ldl -lpthread -lrt -lresolv -lz -Lcontrib/build/funchook -lfunchook -Lcontrib/build/funchook/capstone_src-prefix/src/capstone_src-build -lcapstone -z noexecstack
INCLUDES=-I./contrib/libyaml/include -I./contrib/cJSON -I./os/$(OS) -I./contrib/pcre2/src -I./contrib/build/pcre2 -I./contrib/funchook/capstone_src/include/ -I./contrib/jni -I./contrib/jni/linux/ -I./contrib/openssl/include -I./contrib/build/openssl/include -I./contrib/build/libunwind/include -I./contrib/libunwind/include/ -I./contrib/coredumper/src

$(LIBSCOPE): src/wrap.c src/state.c src/httpstate.c src/metriccapture.c src/report.c src/httpagg.c src/plattime.c src/fn.c os/$(OS)/os.c src/cfgutils.c src/cfg.c src/transport.c src/backoff.c src/log.c src/mtc.c src/circbuf.c src/linklist.c src/evtformat.c src/ctl.c src/mtcformat.c src/com.c src/scopestdlib.c src/dbg.c src/search.c src/oci.c src/wrap_go.c src/sysexec.c src/gocontext_arm.S src/scopeelf.c src/utils.c src/strset.c src/javabci.c src/javaagent.c src/ipc.c src/ipc_resp.c src/snapshot.c src/coredump.c
$(LIBSCOPE): src/wrap.c src/state.c src/httpstate.c src/metriccapture.c src/report.c src/httpagg.c src/plattime.c src/fn.c os/$(OS)/os.c src/cfgutils.c src/cfg.c src/transport.c src/backoff.c src/log.c src/mtc.c src/circbuf.c src/linklist.c src/evtformat.c src/ctl.c src/mtcformat.c src/com.c src/scopestdlib.c src/dbg.c src/search.c src/oci.c src/wrap_go.c src/sysexec.c src/gocontext_arm.S src/scopeelf.c src/utils.c src/strset.c src/javabci.c src/javaagent.c src/ipc.c src/ipc_resp.c src/sig.c src/snapshot.c src/coredump.c
@$(MAKE) -C contrib funchook pcre2 openssl ls-hpack musl libyaml libunwind cJSON coredumper
@echo "$${CI:+::group::}Building $@"
$(CC) $(LIBRARY_CFLAGS) $(ARCH_CFLAGS) \
Expand Down
50 changes: 0 additions & 50 deletions os/linux/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

// want to put this list in an obvious place
//static char thread_delay_list[] = "chrome:nacl_helper";
static timer_t g_timerid = 0;

static int
sendNL(int sd, ino_t node)
Expand Down Expand Up @@ -599,55 +598,6 @@ osGetArgv(pid_t pid, char *buf, size_t blen)
return argc;
}

bool
osTimerStop(void)
{

if (g_timerid) {
timer_delete(g_timerid);
g_timerid = 0;
return TRUE;
}

return FALSE;
}

bool
osThreadInit(void(*handler)(int), unsigned interval)
{
struct sigaction sact;
struct sigevent sevent = {0};
struct itimerspec tspec;
sigemptyset(&sact.sa_mask);
sact.sa_handler = handler;
sact.sa_flags = SA_RESTART;

if (!g_fn.sigaction) return FALSE;

if (g_fn.sigaction(SIGUSR2, &sact, NULL) == -1) {
DBG("errno %d", errno);
return FALSE;
}

sevent.sigev_notify = SIGEV_SIGNAL;
sevent.sigev_signo = SIGUSR2;

if (timer_create(CLOCK_MONOTONIC, &sevent, &g_timerid) == -1) {
DBG("errno %d", errno);
return FALSE;
}

tspec.it_interval.tv_sec = 0;
tspec.it_interval.tv_nsec = 0;
tspec.it_value.tv_sec = interval;
tspec.it_value.tv_nsec = 0;
if (timer_settime(g_timerid, 0, &tspec, NULL) == -1) {
DBG("errno %d", errno);
return FALSE;
}
return TRUE;
}

// In linux, this is declared weak so it can be overridden by the strong
// definition in src/javaagent.c. The scope library will do this.
// This weak definition allows us to not have to define this symbol for
Expand Down
2 changes: 0 additions & 2 deletions os/linux/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,10 @@ extern int osInitTimer(platform_time_t *);
extern int osGetProcMemory(pid_t);
extern int osIsFilePresent(const char *);
extern int osGetCmdline(pid_t, char **);
extern bool osThreadInit(void(*handler)(int), unsigned);
extern int osUnixSockPeer(ino_t);
extern void osInitJavaAgent(void);
extern int osGetPageProt(unsigned long);
extern int osGetExePath(pid_t, char **);
extern bool osTimerStop(void);
extern bool osGetCgroup(pid_t, char *, size_t);
extern char *osGetFileMode(mode_t);
extern int osNeedsConnect(int);
Expand Down
2 changes: 1 addition & 1 deletion os/linux/x86_64.mk
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ INCLUDES=-I./contrib/libyaml/include -I./contrib/cJSON -I./os/$(OS) -I./contrib/
# objcopy -I binary -O elf64-x86-64 -B i386 ./lib/$(OS)/libscope.so ./lib/$(OS)/libscope.o && \
# rm -f ./lib/$(OS)/libscope.so

$(LIBSCOPE): src/wrap.c src/state.c src/httpstate.c src/metriccapture.c src/report.c src/httpagg.c src/plattime.c src/fn.c os/$(OS)/os.c src/cfgutils.c src/cfg.c src/transport.c src/backoff.c src/log.c src/mtc.c src/circbuf.c src/linklist.c src/evtformat.c src/ctl.c src/mtcformat.c src/com.c src/scopestdlib.c src/dbg.c src/search.c src/sysexec.c src/gocontext.S src/scopeelf.c src/oci.c src/wrap_go.c src/utils.c src/strset.c src/javabci.c src/javaagent.c src/ipc.c src/ipc_resp.c src/snapshot.c src/coredump.c
$(LIBSCOPE): src/wrap.c src/state.c src/httpstate.c src/metriccapture.c src/report.c src/httpagg.c src/plattime.c src/fn.c os/$(OS)/os.c src/cfgutils.c src/cfg.c src/transport.c src/backoff.c src/log.c src/mtc.c src/circbuf.c src/linklist.c src/evtformat.c src/ctl.c src/mtcformat.c src/com.c src/scopestdlib.c src/dbg.c src/search.c src/sysexec.c src/gocontext.S src/scopeelf.c src/oci.c src/wrap_go.c src/utils.c src/strset.c src/javabci.c src/javaagent.c src/ipc.c src/ipc_resp.c src/sig.c src/snapshot.c src/coredump.c
@$(MAKE) -C contrib funchook pcre2 openssl ls-hpack musl libyaml cJSON libunwind coredumper
@echo "$${CI:+::group::}Building $@"
$(CC) $(LIBRARY_CFLAGS) $(ARCH_CFLAGS) \
Expand Down
20 changes: 0 additions & 20 deletions os/macOS/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,6 @@ osGetCmdline(pid_t pid, char **cmd)
return (*cmd != NULL);
}

/*
* TBD:
* Note that this is incomplete.
* In Linux we create a timer that delivers a
* signal on expiry. The signal handler starts
* the periodic thread. Need the equivalent
* for OSX.
*/
bool
osThreadInit(void(*handler)(int), unsigned interval)
{
return TRUE;
}

/*
* In Linux we use Netlink socket capabilities
* in order to extract the peer inode for
Expand Down Expand Up @@ -153,12 +139,6 @@ osGetPageProt(uint64_t addr)
return -1;
}

bool
osTimerStop(void)
{
return TRUE;
}

char *
osGetFileMode(mode_t perm)
{
Expand Down
2 changes: 0 additions & 2 deletions os/macOS/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@ extern int osInitTSC(platform_time_t *);
extern int osGetProcMemory(pid_t);
extern int osIsFilePresent(const char *);
extern int osGetCmdline(pid_t, char **);
extern bool osThreadInit(void(*handler)(int), unsigned);
extern int osUnixSockPeer(ino_t);
extern void osInitJavaAgent(void);
extern int osGetPageProt(uint64_t);
extern bool osTimerStop(void);
extern bool osGetCgroup(pid_t, char *, size_t);
extern char *osGetFileMode(mode_t);
extern int osNeedsConnect(int);
Expand Down
167 changes: 167 additions & 0 deletions src/sig.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
#define _GNU_SOURCE

#include "sig.h"
#include "errno.h"
#include "scopestdlib.h"
#include "fn.h"
#include "dbg.h"

/*
* Signal timer ID
*/
static timer_t g_sigTimerId = 0;
/*
* Appscope signal handler state (active or not)
*/
static bool g_sigScopeSignalHandler = FALSE;
/*
* Application signal handler state (active or not)
*/
static bool g_sigAppSignalHandler = FALSE;
/*
* Application signal action structure
*/
static struct sigaction g_sigAppAction;

/*
* Distinguish the SIGUSR2 generated by the AppScope library
*/
#define SIGTIMER_SCOPE_ID (123456789)

/*
* Save application handler
*/
void
sigSaveAppAction(const struct sigaction *act) {
g_sigAppAction.sa_flags = act->sa_flags;
if (g_sigAppAction.sa_flags & SA_SIGINFO) {
g_sigAppAction.sa_sigaction = act->sa_sigaction;
} else {
g_sigAppAction.sa_handler = act->sa_handler;
}

g_sigAppAction.sa_handler = act->sa_handler;
g_sigAppAction.sa_mask = act->sa_mask;
g_sigAppSignalHandler = TRUE;
}

/*
* Call application handler
*/
void
sigCallAppAction(int sig, siginfo_t *info, void *secret) {
if (g_sigAppAction.sa_flags & SA_SIGINFO) {
g_sigAppAction.sa_sigaction(sig, info, secret);
} else {
g_sigAppAction.sa_handler(sig);
}
}

/*
* Check if AppScope Handler is active
*/
bool
sigIsAppcopeActionActive(void) {
return g_sigScopeSignalHandler;
}

/*
* Check if Application installed signal Handler
*/
bool
sigIsAppActionInstalled(void) {
return g_sigAppSignalHandler;
}

/*
* Check if siginfo comes from AppScope signal
*/
bool
sigIsSigFromAppscopeTimer(const siginfo_t *si) {
if (!si) {
return FALSE;
}

if (si->si_signo != SIGUSR2) {
return FALSE;
}

if (si->si_code != SI_TIMER) {
return FALSE;
}

if (si->si_int != SIGTIMER_SCOPE_ID) {
return FALSE;
}

return TRUE;
}

/*
* Register the signal handler for Appscope
* TODO: Consolidate this with other signals used by the Appscope
*/
bool
sigHandlerRegister(int sigNo, void(*handler)(int, siginfo_t *, void *)) {
struct sigaction sact;
sigemptyset(&sact.sa_mask);
sact.sa_handler = (void (*))handler;
sact.sa_flags = SA_SIGINFO | SA_RESTART;

if (!g_fn.sigaction) {
return FALSE;
}

struct sigaction oldact = { 0 };
if (g_fn.sigaction(sigNo, &sact, &oldact) == -1) {
DBG("errno %d", errno);
return FALSE;
}
g_sigScopeSignalHandler = TRUE;

return TRUE;
}

/*
* Starts the "signal timer"
* On timer expiration will generate specific signal
*/
bool
sigTimerStart(int sigNo, unsigned secInterval) {
struct sigevent sevent = {0};
struct itimerspec tspec;

sevent.sigev_notify = SIGEV_SIGNAL;
sevent.sigev_signo = sigNo;
sevent.sigev_value.sival_int = SIGTIMER_SCOPE_ID;

if (timer_create(CLOCK_MONOTONIC, &sevent, &g_sigTimerId) == -1) {
DBG("errno %d", errno);
return FALSE;
}

tspec.it_interval.tv_sec = 0;
tspec.it_interval.tv_nsec = 0;
tspec.it_value.tv_sec = secInterval;
tspec.it_value.tv_nsec = 0;
if (timer_settime(g_sigTimerId, 0, &tspec, NULL) == -1) {
DBG("errno %d", errno);
return FALSE;
}
return TRUE;
}

/*
* Stops the "signal timer"
*/
bool
sigTimerStop(void) {
if (g_sigTimerId) {
timer_delete(g_sigTimerId);
g_sigTimerId = 0;
return TRUE;
}

return FALSE;
}

16 changes: 16 additions & 0 deletions src/sig.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#ifndef __SIG_H__
#define __SIG_H__

#include <signal.h>
#include "scopetypes.h"

bool sigIsAppcopeActionActive(void);
bool sigIsAppActionInstalled(void);
void sigCallAppAction(int, siginfo_t *, void *);
bool sigIsSigFromAppscopeTimer(const siginfo_t *);
void sigSaveAppAction(const struct sigaction *);
bool sigHandlerRegister(int, void(*handler)(int, siginfo_t *, void *));
bool sigTimerStart(int, unsigned);
bool sigTimerStop(void);

#endif // __SIG_H__
Loading

0 comments on commit a689b96

Please sign in to comment.