[LEDE-DEV] [PATCH procd] service: Start services normally when seccomp is disabled
When service init file declares seccomp support (procd_set_param seccomp),
but procd is compiled without seccomp support, the service should be
started normally, because seccomp-trace and utrace are not available.
Older procd versions decided about whether to start a service in
seccomp sandbox or not based on existence of seccomp whitelist in the
filesystem. This was recently removed (c8faedc "Do not disable seccomp
when configuration is not found", 2017-09-12) because it could be easy
for attackers to disable seccomp support. This changes is a follow-up
to the mentioned commit. With it, procd decides about whether to use
seccomp sandbox based only on compile-time configuration.
Signed-off-by: Michal Sojka
---
CMakeLists.txt | 1 +
service/instance.c | 9 ++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7d05e97..4b3eebd 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -88,6 +88,7 @@ ADD_CUSTOM_COMMAND(
ADD_CUSTOM_TARGET(capabilities-names-h DEPENDS capabilities-names.h)
IF(SECCOMP_SUPPORT)
+ADD_DEFINITIONS(-DSECCOMP_SUPPORT)
ADD_LIBRARY(preload-seccomp SHARED jail/preload.c jail/seccomp.c)
TARGET_LINK_LIBRARIES(preload-seccomp dl ubox blobmsg_json)
INSTALL(TARGETS preload-seccomp
diff --git a/service/instance.c b/service/instance.c
index 7703686..29d6ea6 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -141,8 +141,6 @@ static const struct rlimit_name rlimit_names[] = {
{ NULL, 0 }
};
-static char trace[] = "/sbin/utrace";
-
static void closefd(int fd)
{
if (fd > STDERR_FILENO)
@@ -315,10 +313,15 @@ instance_run(struct service_instance *in, int _stdout,
int _stderr)
argv = alloca(sizeof(char *) * (argc + in->jail.argc));
argc = 0;
+#ifdef SECCOMP_SUPPORT
if (in->trace)
- argv[argc++] = trace;
+ argv[argc++] = "/sbin/utrace";
else if (seccomp)
argv[argc++] = "/sbin/seccomp-trace";
+#else
+ if (in->trace || seccomp)
+ ULOG_WARN("Seccomp support for %s::%s not available\n",
in->srv->name, in->name);
+#endif
if (in->has_jail)
argc = jail_run(in, argv);
--
2.15.0.rc2
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] procd: Always tell cmake whether to include seccomp support or not
Without this change, when a user disables seccomp support in .config, procd does not get recompiled unless the package is cleaned manually. It is because when -D option is missing from cmake command line, cmake uses cached value from the previous run where seccomp was enabled. Signed-off-by: Michal Sojka --- package/system/procd/Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/package/system/procd/Makefile b/package/system/procd/Makefile index f424cea57d..3d413266e4 100644 --- a/package/system/procd/Makefile +++ b/package/system/procd/Makefile @@ -107,9 +107,8 @@ ifdef CONFIG_PACKAGE_procd-ujail CMAKE_OPTIONS += -DJAIL_SUPPORT=1 endif -ifdef CONFIG_PACKAGE_procd-seccomp - CMAKE_OPTIONS += -DSECCOMP_SUPPORT=1 -DUTRACE_SUPPORT=1 -endif +SECCOMP=$(if $(CONFIG_PACKAGE_procd-seccomp),1,0) +CMAKE_OPTIONS += -DSECCOMP_SUPPORT=$(SECCOMP) -DUTRACE_SUPPORT=$(SECCOMP) define Package/procd/install $(INSTALL_DIR) $(1)/sbin $(1)/etc $(1)/lib/functions -- 2.15.0.rc2 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] umdns fails to launch (seccom related)?
Hi Bryan, On Fri, Nov 03 2017, Bryan Mayland wrote: > I use a custom LEDE build as the basis for an open source project and > noticed recently that the umdns daemon has stopped loading. This also > occurs with the standard development snapshot with the umdns package > installed. > root@LEDE:~# /etc/init.d/umdns restart > Command failed: Request timed out > > umdns never launches so the service_started() handler waits for the 10 > seconds for the ubus response then fails. If I comment out the > `procd_set_param seccomp /etc/seccomp/umdns.json` line it works just > fine. I've also tried replacing the umdns executable with a simple > shell script and it never appears to execute. This occurs if the > daemon is configured to be jailed or not. > > Because my bcm2708 target does not have CONFIG_KERNEL_SECCOMP enabled, > this should not make a difference right? I've also tried adding a > /sbin/seccomp-trace which was just a shell script as well to launch > umdns and it had no effect. > > I believe it is a problem in procd but I'm not sure where else to look > as this seems like it could affect anything with a seccomp procd init, > and umdns is the only one that does this so far. you're right, I'm aware of the problem. I already have a patch with the fix. I need to do a few cleanups before posting it - I'll look at that tonight. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH procd v2 10/17] seccomp: Log seccomp violations with utrace
On Mon, Sep 25 2017, Michal Sojka wrote: > Older kernel version shipped by LEDE/OpenWrt contained patch > target/linux/generic/patches-3.18/999-seccomp_log.patch that logged > seccomp violations. For some reason, newer kernels do not have this > patch. Without this kind of logging, it is very hard to setup seccomp > whitelist properly, so this commit modifies utrace to serve as a > logger for seccomp violations. > > With this patch, when utrace is executed via seccomp-trace symlink, it > does not trace normal syscalls but only seccomp violations and logs > them to syslog. If've just discovered https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/seccomp/seccomp_bpf.c?h=v4.13#n1227. If this patch is going to be accepted, it would be better to rewrite its code to get and modify syscall number according to the link above, because it supports more architectures than my patch. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 18/18] utrace: Start the tracee only after uloop initialization
Without this, early ptrace stops can be missed because they can happen
before the call to uloop_init().
Signed-off-by: Michal Sojka
---
trace/trace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/trace/trace.c b/trace/trace.c
index d022079..76b6b7f 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -342,6 +342,9 @@ int main(int argc, char **argv, char **envp)
return -1;
}
+ /* Initialize uloop to catch all ptrace stops from now on. */
+ uloop_init();
+
int ptrace_options = PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK |
PTRACE_O_TRACECLONE;
switch (mode) {
case UTRACE:
@@ -362,7 +365,6 @@ int main(int argc, char **argv, char **envp)
return -1;
}
- uloop_init();
tracer.proc.pid = child;
tracer.proc.cb = tracer_cb;
uloop_process_add(&tracer.proc);
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] Another utrace bug
Hi, my automated tests revealed another bug (race condition) in utrace. I'm sending a patch that fixes it as a reply to this message. Let me know if it would be better to send the whole fixed patch series again as v2. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd v2 10/17] seccomp: Log seccomp violations with utrace
Older kernel version shipped by LEDE/OpenWrt contained patch
target/linux/generic/patches-3.18/999-seccomp_log.patch that logged
seccomp violations. For some reason, newer kernels do not have this
patch. Without this kind of logging, it is very hard to setup seccomp
whitelist properly, so this commit modifies utrace to serve as a
logger for seccomp violations.
With this patch, when utrace is executed via seccomp-trace symlink, it
does not trace normal syscalls but only seccomp violations and logs
them to syslog. For example:
seccomp-trace: uci[3955] tried to call non-whitelisted syscall: ftruncate64
(see /etc/seccomp/myservice.json)
Compared to the kernel-based logging, this approach gives users more
information - which json whitelist needs to be extended. This is
especially useful for services, which fork many diverse children such
as shell scripts.
Signed-off-by: Michal Sojka
---
jail/seccomp-bpf.h | 1 +
jail/seccomp.c | 4 +-
trace/trace.c | 143 +++--
3 files changed, 121 insertions(+), 27 deletions(-)
diff --git a/jail/seccomp-bpf.h b/jail/seccomp-bpf.h
index 82c0669..fc3ffe7 100644
--- a/jail/seccomp-bpf.h
+++ b/jail/seccomp-bpf.h
@@ -41,6 +41,7 @@
#define SECCOMP_RET_TRAP 0x0003U /* disallow and force a SIGSYS */
#define SECCOMP_RET_ERRNO 0x0005U /* returns an errno */
#define SECCOMP_RET_LOG0x0007U
+#define SECCOMP_RET_TRACE 0x7ff0U /* pass to a tracer or disallow */
#define SECCOMP_RET_ALLOW 0x7fffU /* allow */
#define SECCOMP_RET_ERROR(x) (SECCOMP_RET_ERRNO | ((x) & 0xU))
#define SECCOMP_RET_LOGGER(x) (SECCOMP_RET_LOG | ((x) & 0xU))
diff --git a/jail/seccomp.c b/jail/seccomp.c
index dcd19ec..1a2bb27 100644
--- a/jail/seccomp.c
+++ b/jail/seccomp.c
@@ -118,8 +118,8 @@ int install_syscall_filter(const char *argv, const char
*file)
}
if (default_policy)
- /* return -1 and set errno */
- set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_LOGGER(default_policy));
+ /* notify tracer; without tracer return -1 and set errno to
ENOSYS */
+ set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_TRACE);
else
/* kill the process */
set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_KILL);
diff --git a/trace/trace.c b/trace/trace.c
index 9d3cd0a..5189cfb 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -12,8 +12,10 @@
*/
#define _GNU_SOURCE
+#include
#include
#include
+#include
#include
#include
#include
@@ -49,18 +51,33 @@
# endif
#define reg_syscall_nr (EF_REG2 / 4)
#elif defined(__arm__)
+#include /* for PTRACE_SET_SYSCALL */
#define reg_syscall_nr _offsetof(struct user, regs.uregs[7])
+# if defined(__ARM_EABI__)
+# define reg_retval_nr _offsetof(struct user, regs.uregs[0])
+# endif
#else
#error tracing is not supported on this architecture
#endif
+enum mode {
+ UTRACE,
+ SECCOMP_TRACE,
+} mode = UTRACE;
+
+#define PROC_NAME(mode) (mode == UTRACE ? "utrace" : "seccomp-trace")
+
#define INFO(fmt, ...) do { \
- fprintf(stderr, "utrace: "fmt, ## __VA_ARGS__); \
+ fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
} while (0)
#define ERROR(fmt, ...) do { \
- syslog(LOG_ERR, "utrace: "fmt, ## __VA_ARGS__); \
- fprintf(stderr, "utrace: "fmt, ## __VA_ARGS__); \
+ syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
+ fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
+} while (0)
+
+#define LOGERR(fmt, ...) do { \
+ syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
} while (0)
struct tracee {
@@ -70,9 +87,12 @@ struct tracee {
static struct tracee tracer;
static int *syscall_count;
+static int violation_count;
static struct blob_buf b;
static int syscall_max;
static int debug;
+char *json = NULL;
+int ptrace_restart;
static int max_syscall = ARRAY_SIZE(syscall_names);
@@ -102,11 +122,13 @@ static void print_syscalls(int policy, const char *json)
void *c;
int i;
- set_syscall("rt_sigaction", 1);
- set_syscall("sigreturn", 1);
- set_syscall("rt_sigreturn", 1);
- set_syscall("exit_group", 1);
- set_syscall("exit", 1);
+ if (mode == UTRACE) {
+ set_syscall("rt_sigaction", 1);
+ set_syscall("sigreturn", 1);
+ set_syscall("rt_sigreturn", 1);
+ set_syscall("exit_group", 1);
+ set_syscall("exit", 1);
+ }
struct syscall sorted[ARRAY_SIZE(syscall_names)];
@@ -151,6 +173,30 @@ static void print_syscalls(int policy, con
Re: [LEDE-DEV] [PATCH procd 10/17] seccomp: Log seccomp violations with utrace
On Tue, Sep 12 2017, Michal Sojka wrote:
> Older kernel version shipped by LEDE/OpenWrt contained patch
> target/linux/generic/patches-3.18/999-seccomp_log.patch that logged
> seccomp violations. For some reason, newer kernels do not have this
> patch. Without this kind of logging, it is very hard to setup seccomp
> whitelist properly, so this commit modifies utrace to serve as a
> logger for seccomp violations.
>
> With this patch, when utrace is executed via seccomp-trace symlink, it
> does not trace normal syscalls but only seccomp violations and logs
> them to syslog. For example:
>
> seccomp-trace: uci[3955] tried to call non-whitelisted syscall:
> ftruncate64 (see /etc/seccomp/myservice.json)
It turns out that this patch has its problems too. It works properly
only on x86. On ARM, it reports the violations, but fails to block the
non-whitelisted syscalls. I don't have other hardware at hand so I
cannot test it on other archs.
The change needed for ARM is shown bellow and I'll send v2 patch
with this change soon.
I'm testing this on ARM with 4.1+ kernel and on x86 with 4.4.86. There
were some changes in seccomp/ptrace in Linux 4.8 - I believe this patch
will work the same even with the newer Linux, but this has not been
tested (yet).
-Michal
diff --git a/trace/trace.c b/trace/trace.c
index 6fb9335..d022079 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -52,7 +52,11 @@
# endif
#define reg_syscall_nr (EF_REG2 / 4)
#elif defined(__arm__)
+#include /* for PTRACE_SET_SYSCALL */
#define reg_syscall_nr _offsetof(struct user, regs.uregs[7])
+# if defined(__ARM_EABI__)
+# define reg_retval_nr _offsetof(struct user, regs.uregs[0])
+# endif
#else
#error tracing is not supported on this architecture
#endif
@@ -216,7 +220,12 @@ static void tracer_cb(struct uloop_process *c, int ret)
/* Nothing special to do here */
} else if ((ret >> 8) == (SIGTRAP | (PTRACE_EVENT_SECCOMP <<
8))) {
int syscall = ptrace(PTRACE_PEEKUSER, c->pid,
reg_syscall_nr);
+#if defined(__arm__)
+ptrace(PTRACE_SET_SYSCALL, c->pid, 0, -1);
+ptrace(PTRACE_POKEUSER, c->pid, reg_retval_nr,
-ENOSYS);
+#else
ptrace(PTRACE_POKEUSER, c->pid, reg_syscall_nr, -1);
+#endif
report_seccomp_vialation(c->pid, syscall);
} else {
inject_signal = WSTOPSIG(ret);
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH procd 16/17] utrace: Support non-contiguous syscall numbers
On Tue, Sep 12 2017, Michal Sojka wrote:
> ARM architecture does not have its system call numbers contiguous. So
> far, utrace ignored the non-contiguous system calls, but it makes it
> difficult to setup seccomp whitelists. This patch adds support for
> these extra out-of-range syscalls.
This patch is buggy. I'll send fixed patch soon. The diff between this
and new patch is:
diff --git a/trace/trace.c b/trace/trace.c
index 2621430..6fb9335 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -193,7 +193,7 @@ static void tracer_cb(struct uloop_process *c, int ret)
int syscall = ptrace(PTRACE_PEEKUSER, c->pid,
reg_syscall_nr);
int i = syscall_index(syscall);
if (i >= 0) {
-syscall_count[syscall]++;
+syscall_count[i]++;
if (debug)
fprintf(stderr, "%s()\n",
syscall_name(syscall));
} else if (debug) {
-Michal
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd v2 16/17] utrace: Support non-contiguous syscall numbers
ARM architecture does not have its system call numbers contiguous. So
far, utrace ignored the non-contiguous system calls, but it makes it
difficult to setup seccomp whitelists. This patch adds support for
these extra out-of-range syscalls.
It extends the generated file syscall_names.h to include a few
functions. Now, for ARM this file looks like:
#include
static const char *__syscall_names[] = {
[280] = "waitid",
[148] = "fdatasync",
...
[252] = "epoll_wait",
[74] = "sethostname",
};
static inline const char *syscall_name(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return __syscall_names[i];
switch (i) {
case 0x0f0001: return "breakpoint";
case 0x0f0003: return "usr26";
case 0x0f0004: return "usr32";
case 0x0f0005: return "set_tls";
case 0x0f0002: return "cacheflush";
default: return (void*)0;
}
}
static inline int syscall_index(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
case 0x0f0001: return ARRAY_SIZE(__syscall_names) + 0;
case 0x0f0003: return ARRAY_SIZE(__syscall_names) + 1;
case 0x0f0004: return ARRAY_SIZE(__syscall_names) + 2;
case 0x0f0005: return ARRAY_SIZE(__syscall_names) + 3;
case 0x0f0002: return ARRAY_SIZE(__syscall_names) + 4;
default: return -1;
}
}
static inline int syscall_index_to_number(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
case ARRAY_SIZE(__syscall_names) + 0: return 0x0f0001;
case ARRAY_SIZE(__syscall_names) + 1: return 0x0f0003;
case ARRAY_SIZE(__syscall_names) + 2: return 0x0f0004;
case ARRAY_SIZE(__syscall_names) + 3: return 0x0f0005;
case ARRAY_SIZE(__syscall_names) + 4: return 0x0f0002;
default: return -1;
}
}
#define SYSCALL_COUNT (ARRAY_SIZE(__syscall_names) + 5)
For x86, which does not have extra syscalls, the file looks this way:
#include
static const char *__syscall_names[] = {
[247] = "waitid",
[75] = "fdatasync",
...
[232] = "epoll_wait",
[170] = "sethostname",
};
static inline const char *syscall_name(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return __syscall_names[i];
switch (i) {
default: return (void*)0;
}
}
static inline int syscall_index(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
default: return -1;
}
}
static inline int syscall_index_to_number(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
default: return -1;
}
}
#define SYSCALL_COUNT (ARRAY_SIZE(__syscall_names) + 0)
Signed-off-by: Michal Sojka
---
jail/seccomp.c| 10 +-
make_syscall_h.sh | 48 +++-
trace/trace.c | 44 +---
3 files changed, 73 insertions(+), 29 deletions(-)
diff --git a/jail/seccomp.c b/jail/seccomp.c
index 27bf3ce..eeb5781 100644
--- a/jail/seccomp.c
+++ b/jail/seccomp.c
@@ -22,15 +22,15 @@
#include "seccomp.h"
#include "../syscall-names.h"
-static int max_syscall = ARRAY_SIZE(syscall_names);
-
static int find_syscall(const char *name)
{
int i;
- for (i = 0; i < max_syscall; i++)
- if (syscall_names[i] && !strcmp(syscall_names[i], name))
- return i;
+ for (i = 0; i < SYSCALL_COUNT; i++) {
+ int sc = syscall_index_to_number(i);
+ if (syscall_name(sc) && !strcmp(syscall_name(sc), name))
+ return sc;
+ }
return -1;
}
diff --git a/make_syscall_h.sh b/make_syscall_h.sh
index 3363bc7..18d9131 100755
--- a/make_syscall_h.sh
+++ b/make_syscall_h.sh
@@ -12,7 +12,53 @@ CC=$1
[ -n "$TARGET_CC_NOCACHE" ] && CC=$TARGET_CC_NOCACHE
echo "#include "
-echo "static const char *syscall_names[] = {"
+echo "static const char *__syscall_names[] = {"
echo "#include " | ${CC} -E -dM - | grep '^#define __NR_' | \
LC_ALL=C sed -r -n -e 's/^\#define[ \t]+__NR_([a-z0-9_]+)[ \t]+([
()+0-9a-zNR_Linux]+)(.*)/ [\2] = "\1",/p'
echo "};"
+
+extra_syscalls="$(echo "#include " | ${CC} -E -dM - | sed -n -e
'/^#define __ARM_NR_/ s///p')"
+
+cat <= 0) {
+ syscall_count[i]++;
LOGERR("%s[%u] tried to call non-whitelisted syscall: %s (see
%s)\n",
- buf, pid, syscall_names[syscall], json);
+ buf, pid, syscall_name(syscall),
Re: [LEDE-DEV] [PATCH procd 01/17] utrace: Fix environment initialization
On Thu, Sep 21 2017, John Crispin wrote: > Hi Michal. > > sorry for the delay, i am halfway through reviewing your patches. thanks > for sending them. No problem, thanks for reviewing :) -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH libubox 2/2] uloop: Enable utracing of multi-threaded programs
On Thu, Sep 14 2017, Yousong Zhou wrote:
> On 12 September 2017 at 19:12, Michal Sojka wrote:
>> This is needed for Linux < 4.7 or < 4.4.13 to report ptrace events in
>> threads.
>>
>> Signed-off-by: Michal Sojka
>> ---
>> uloop.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/uloop.c b/uloop.c
>> index 3813e18..e6d77df 100644
>> --- a/uloop.c
>> +++ b/uloop.c
>> @@ -369,7 +369,7 @@ static void uloop_handle_processes(void)
>> do_sigchld = false;
>>
>> while (1) {
>> - pid = waitpid(-1, &ret, WNOHANG);
>> + pid = waitpid(-1, &ret, WNOHANG|__WALL);
>> if (pid < 0 && errno == EINTR)
>> continue;
>>
>
> NACK because this changes the current behaviour of only handling
> termination of child processes notified via SIGCHLD signal. It may
> break existing users of the library.
OK. It should not influence the rest of the patch series, because LEDE
seems to ship newer kernel versions than those needing this patch.
-Michal
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 16/17] utrace: Support non-contiguous syscall numbers
ARM architecture does not have its system call numbers contiguous. So
far, utrace ignored the non-contiguous system calls, but it makes it
difficult to setup seccomp whitelists. This patch adds support for
these extra out-of-range syscalls.
It extends the generated file syscall_names.h to include a few
functions. Now, for ARM this file looks like:
#include
static const char *__syscall_names[] = {
[280] = "waitid",
[148] = "fdatasync",
...
[252] = "epoll_wait",
[74] = "sethostname",
};
static inline const char *syscall_name(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return __syscall_names[i];
switch (i) {
case 0x0f0001: return "breakpoint";
case 0x0f0003: return "usr26";
case 0x0f0004: return "usr32";
case 0x0f0005: return "set_tls";
case 0x0f0002: return "cacheflush";
default: return (void*)0;
}
}
static inline int syscall_index(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
case 0x0f0001: return ARRAY_SIZE(__syscall_names) + 0;
case 0x0f0003: return ARRAY_SIZE(__syscall_names) + 1;
case 0x0f0004: return ARRAY_SIZE(__syscall_names) + 2;
case 0x0f0005: return ARRAY_SIZE(__syscall_names) + 3;
case 0x0f0002: return ARRAY_SIZE(__syscall_names) + 4;
default: return -1;
}
}
static inline int syscall_index_to_number(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
case ARRAY_SIZE(__syscall_names) + 0: return 0x0f0001;
case ARRAY_SIZE(__syscall_names) + 1: return 0x0f0003;
case ARRAY_SIZE(__syscall_names) + 2: return 0x0f0004;
case ARRAY_SIZE(__syscall_names) + 3: return 0x0f0005;
case ARRAY_SIZE(__syscall_names) + 4: return 0x0f0002;
default: return -1;
}
}
#define SYSCALL_COUNT (ARRAY_SIZE(__syscall_names) + 5)
For x86, which does not have extra syscalls, the file looks this way:
#include
static const char *__syscall_names[] = {
[247] = "waitid",
[75] = "fdatasync",
...
[232] = "epoll_wait",
[170] = "sethostname",
};
static inline const char *syscall_name(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return __syscall_names[i];
switch (i) {
default: return (void*)0;
}
}
static inline int syscall_index(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
default: return -1;
}
}
static inline int syscall_index_to_number(unsigned i) {
if (i < ARRAY_SIZE(__syscall_names))
return i;
switch (i) {
default: return -1;
}
}
#define SYSCALL_COUNT (ARRAY_SIZE(__syscall_names) + 0)
Signed-off-by: Michal Sojka
---
jail/seccomp.c| 10 +-
make_syscall_h.sh | 48 +++-
trace/trace.c | 42 --
3 files changed, 72 insertions(+), 28 deletions(-)
diff --git a/jail/seccomp.c b/jail/seccomp.c
index 27bf3ce..eeb5781 100644
--- a/jail/seccomp.c
+++ b/jail/seccomp.c
@@ -22,15 +22,15 @@
#include "seccomp.h"
#include "../syscall-names.h"
-static int max_syscall = ARRAY_SIZE(syscall_names);
-
static int find_syscall(const char *name)
{
int i;
- for (i = 0; i < max_syscall; i++)
- if (syscall_names[i] && !strcmp(syscall_names[i], name))
- return i;
+ for (i = 0; i < SYSCALL_COUNT; i++) {
+ int sc = syscall_index_to_number(i);
+ if (syscall_name(sc) && !strcmp(syscall_name(sc), name))
+ return sc;
+ }
return -1;
}
diff --git a/make_syscall_h.sh b/make_syscall_h.sh
index 3363bc7..18d9131 100755
--- a/make_syscall_h.sh
+++ b/make_syscall_h.sh
@@ -12,7 +12,53 @@ CC=$1
[ -n "$TARGET_CC_NOCACHE" ] && CC=$TARGET_CC_NOCACHE
echo "#include "
-echo "static const char *syscall_names[] = {"
+echo "static const char *__syscall_names[] = {"
echo "#include " | ${CC} -E -dM - | grep '^#define __NR_' | \
LC_ALL=C sed -r -n -e 's/^\#define[ \t]+__NR_([a-z0-9_]+)[ \t]+([
()+0-9a-zNR_Linux]+)(.*)/ [\2] = "\1",/p'
echo "};"
+
+extra_syscalls="$(echo "#include " | ${CC} -E -dM - | sed -n -e
'/^#define __ARM_NR_/ s///p')"
+
+cat <= 0) {
+ syscall_count[i]++;
LOGERR("%s[%u] tried to call non-whitelisted syscall: %s (see
%s)\n",
- buf, pid, syscall_names[syscall], json);
+ buf, pid, syscall_name(syscall),
[LEDE-DEV] [PATCH procd 17/17] utrace: Switch all logging to ulog
This unifies all logs messages produced by utrace and removes
duplicated functionality.
Signed-off-by: Michal Sojka
---
trace/trace.c | 49 +++--
1 file changed, 19 insertions(+), 30 deletions(-)
diff --git a/trace/trace.c b/trace/trace.c
index 8228edf..2621430 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#ifndef PTRACE_EVENT_STOP
/* PTRACE_EVENT_STOP is defined in linux/ptrace.h, but this header
@@ -33,6 +32,7 @@
#define PTRACE_EVENT_STOP 128
#endif
+#include
#include
#include
#include
@@ -62,21 +62,6 @@ enum mode {
SECCOMP_TRACE,
} mode = UTRACE;
-#define PROC_NAME(mode) (mode == UTRACE ? "utrace" : "seccomp-trace")
-
-#define INFO(fmt, ...) do { \
- fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
-} while (0)
-
-#define ERROR(fmt, ...) do { \
- syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
- fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
-} while (0)
-
-#define LOGERR(fmt, ...) do { \
- syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
-} while (0)
-
struct tracee {
struct uloop_process proc;
int in_syscall;
@@ -148,7 +133,7 @@ static void print_syscalls(int policy, const char *json)
sc, syscall_name(sc), sorted[i].count);
blobmsg_add_string(&b, NULL, syscall_name(sc));
} else {
- ERROR("no name found for syscall(%d)\n", sc);
+ ULOG_ERR("no name found for syscall(%d)\n", sc);
}
}
blobmsg_close_array(&b, c);
@@ -158,9 +143,9 @@ static void print_syscalls(int policy, const char *json)
if (fp) {
fprintf(fp, "%s", blobmsg_format_json_indent(b.head,
true, 0));
fclose(fp);
- INFO("saving syscall trace to %s\n", json);
+ ULOG_INFO("saving syscall trace to %s\n", json);
} else {
- ERROR("failed to open %s\n", json);
+ ULOG_ERR("failed to open %s\n", json);
}
} else {
printf("%s\n",
@@ -186,11 +171,11 @@ static void report_seccomp_vialation(pid_t pid, unsigned
syscall)
int i = syscall_index(syscall);
if (i >= 0) {
syscall_count[i]++;
- LOGERR("%s[%u] tried to call non-whitelisted syscall: %s (see
%s)\n",
- buf, pid, syscall_name(syscall), json);
+ ULOG_ERR("%s[%u] tried to call non-whitelisted syscall: %s (see
%s)\n",
+buf, pid, syscall_name(syscall), json);
} else {
- LOGERR("%s[%u] tried to call non-whitelisted syscall: %d (see
%s)\n",
- buf, pid, syscall, json);
+ ULOG_ERR("%s[%u] tried to call non-whitelisted syscall: %d (see
%s)\n",
+buf, pid, syscall, json);
}
}
@@ -332,7 +317,7 @@ int main(int argc, char **argv, char **envp)
memcpy(&_envp[newenv], envp, envc * sizeof(char *));
ret = execve(_argv[0], _argv, _envp);
- ERROR("failed to exec %s: %s\n", _argv[0], strerror(errno));
+ ULOG_ERR("failed to exec %s: %s\n", _argv[0], strerror(errno));
free(_argv);
free(_envp);
@@ -344,7 +329,7 @@ int main(int argc, char **argv, char **envp)
waitpid(child, &status, WUNTRACED);
if (!WIFSTOPPED(status)) {
- ERROR("failed to start %s\n", *argv);
+ ULOG_ERR("failed to start %s\n", *argv);
return -1;
}
@@ -359,10 +344,14 @@ int main(int argc, char **argv, char **envp)
ptrace_restart = PTRACE_CONT;
break;
}
- if (ptrace(PTRACE_SEIZE, child, 0, ptrace_options) == -1)
- err(1, "PTRACE_SEIZE");
- if (ptrace(ptrace_restart, child, 0, SIGCONT) == -1)
- err(1, "ptrace restart");
+ if (ptrace(PTRACE_SEIZE, child, 0, ptrace_options) == -1) {
+ ULOG_ERR("PTRACE_SEIZE: %s\n", strerror(errno));
+ return -1;
+ }
+ if (ptrace(ptrace_restart, child, 0, SIGCONT) == -1) {
+ ULOG_ERR("ptrace_restart: %s\n", strerror(errno));
+ return -1;
+ }
uloop_init();
tracer.proc.pid = child;
@@ -377,7 +366,7 @@ int main(int argc, char **argv, char **envp)
case UTRACE:
if (!json)
if (aspr
[LEDE-DEV] [PATCH procd 11/17] Start seccomp-enabled services via seccomp-trace
Signed-off-by: Michal Sojka
---
service/instance.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/service/instance.c b/service/instance.c
index 1760a09..7703686 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -303,16 +303,13 @@ instance_run(struct service_instance *in, int _stdout,
int _stderr)
if (seccomp)
setenv("SECCOMP_FILE", in->seccomp, 1);
- if ((seccomp || setlbf) && asprintf(&ld_preload, "LD_PRELOAD=%s%s%s",
- seccomp ? "/lib/libpreload-seccomp.so" : "",
- seccomp && setlbf ? ":" : "",
- setlbf ? "/lib/libsetlbf.so" : "") > 0)
+ if (setlbf && asprintf(&ld_preload, "LD_PRELOAD=/lib/libsetlbf.so") > 0)
putenv(ld_preload);
blobmsg_list_for_each(&in->limits, var)
instance_limits(blobmsg_name(var->data),
blobmsg_data(var->data));
- if (in->trace)
+ if (in->trace || seccomp)
argc += 1;
argv = alloca(sizeof(char *) * (argc + in->jail.argc));
@@ -320,6 +317,8 @@ instance_run(struct service_instance *in, int _stdout, int
_stderr)
if (in->trace)
argv[argc++] = trace;
+ else if (seccomp)
+ argv[argc++] = "/sbin/seccomp-trace";
if (in->has_jail)
argc = jail_run(in, argv);
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH source] procd: Install seccomp-trace symlink
Signed-off-by: Michal Sojka --- package/system/procd/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/package/system/procd/Makefile b/package/system/procd/Makefile index fd1bca3f4b..425bf09a0b 100644 --- a/package/system/procd/Makefile +++ b/package/system/procd/Makefile @@ -130,6 +130,7 @@ define Package/procd-seccomp/install $(INSTALL_DIR) $(1)/sbin $(1)/lib $(INSTALL_DATA) $(PKG_INSTALL_DIR)/usr/lib/libpreload-seccomp.so $(1)/lib $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/sbin/utrace $(1)/sbin/ + ln -s utrace $(1)/sbin/seccomp-trace $(INSTALL_DATA) $(PKG_INSTALL_DIR)/usr/lib/libpreload-trace.so $(1)/lib endef -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH libubox 2/2] uloop: Enable utracing of multi-threaded programs
This is needed for Linux < 4.7 or < 4.4.13 to report ptrace events in
threads.
Signed-off-by: Michal Sojka
---
uloop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/uloop.c b/uloop.c
index 3813e18..e6d77df 100644
--- a/uloop.c
+++ b/uloop.c
@@ -369,7 +369,7 @@ static void uloop_handle_processes(void)
do_sigchld = false;
while (1) {
- pid = waitpid(-1, &ret, WNOHANG);
+ pid = waitpid(-1, &ret, WNOHANG|__WALL);
if (pid < 0 && errno == EINTR)
continue;
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 13/17] seccomp: Improve error message
Print "SECCOMP_FILE not specified" instead of "failed to load (null)".
Signed-off-by: Michal Sojka
---
jail/preload.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/jail/preload.c b/jail/preload.c
index 5466f27..24358c6 100644
--- a/jail/preload.c
+++ b/jail/preload.c
@@ -27,6 +27,11 @@ static int __preload_main__(int argc, char **argv, char
**envp)
{
char *env_file = getenv("SECCOMP_FILE");
+ if (!env_file || !env_file[0]) {
+ ERROR("SECCOMP_FILE not specified\n");
+ return -1;
+ }
+
if (install_syscall_filter(*argv, env_file))
return -1;
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 10/17] seccomp: Log seccomp violations with utrace
Older kernel version shipped by LEDE/OpenWrt contained patch
target/linux/generic/patches-3.18/999-seccomp_log.patch that logged
seccomp violations. For some reason, newer kernels do not have this
patch. Without this kind of logging, it is very hard to setup seccomp
whitelist properly, so this commit modifies utrace to serve as a
logger for seccomp violations.
With this patch, when utrace is executed via seccomp-trace symlink, it
does not trace normal syscalls but only seccomp violations and logs
them to syslog. For example:
seccomp-trace: uci[3955] tried to call non-whitelisted syscall: ftruncate64
(see /etc/seccomp/myservice.json)
Compared to the kernel-based logging, this approach gives users more
information - which json whitelist needs to be extended. This is
especially useful for services, which fork many diverse children such
as shell scripts.
Signed-off-by: Michal Sojka
---
jail/seccomp-bpf.h | 1 +
jail/seccomp.c | 4 +-
trace/trace.c | 134 +++--
3 files changed, 112 insertions(+), 27 deletions(-)
diff --git a/jail/seccomp-bpf.h b/jail/seccomp-bpf.h
index 82c0669..fc3ffe7 100644
--- a/jail/seccomp-bpf.h
+++ b/jail/seccomp-bpf.h
@@ -41,6 +41,7 @@
#define SECCOMP_RET_TRAP 0x0003U /* disallow and force a SIGSYS */
#define SECCOMP_RET_ERRNO 0x0005U /* returns an errno */
#define SECCOMP_RET_LOG0x0007U
+#define SECCOMP_RET_TRACE 0x7ff0U /* pass to a tracer or disallow */
#define SECCOMP_RET_ALLOW 0x7fffU /* allow */
#define SECCOMP_RET_ERROR(x) (SECCOMP_RET_ERRNO | ((x) & 0xU))
#define SECCOMP_RET_LOGGER(x) (SECCOMP_RET_LOG | ((x) & 0xU))
diff --git a/jail/seccomp.c b/jail/seccomp.c
index dcd19ec..1a2bb27 100644
--- a/jail/seccomp.c
+++ b/jail/seccomp.c
@@ -118,8 +118,8 @@ int install_syscall_filter(const char *argv, const char
*file)
}
if (default_policy)
- /* return -1 and set errno */
- set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_LOGGER(default_policy));
+ /* notify tracer; without tracer return -1 and set errno to
ENOSYS */
+ set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_TRACE);
else
/* kill the process */
set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_KILL);
diff --git a/trace/trace.c b/trace/trace.c
index 9d3cd0a..6fbe608 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -12,8 +12,10 @@
*/
#define _GNU_SOURCE
+#include
#include
#include
+#include
#include
#include
#include
@@ -54,13 +56,24 @@
#error tracing is not supported on this architecture
#endif
+enum mode {
+ UTRACE,
+ SECCOMP_TRACE,
+} mode = UTRACE;
+
+#define PROC_NAME(mode) (mode == UTRACE ? "utrace" : "seccomp-trace")
+
#define INFO(fmt, ...) do { \
- fprintf(stderr, "utrace: "fmt, ## __VA_ARGS__); \
+ fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
} while (0)
#define ERROR(fmt, ...) do { \
- syslog(LOG_ERR, "utrace: "fmt, ## __VA_ARGS__); \
- fprintf(stderr, "utrace: "fmt, ## __VA_ARGS__); \
+ syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
+ fprintf(stderr, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
+} while (0)
+
+#define LOGERR(fmt, ...) do { \
+ syslog(LOG_ERR, "%s: "fmt, PROC_NAME(mode), ## __VA_ARGS__); \
} while (0)
struct tracee {
@@ -70,9 +83,12 @@ struct tracee {
static struct tracee tracer;
static int *syscall_count;
+static int violation_count;
static struct blob_buf b;
static int syscall_max;
static int debug;
+char *json = NULL;
+int ptrace_restart;
static int max_syscall = ARRAY_SIZE(syscall_names);
@@ -102,11 +118,13 @@ static void print_syscalls(int policy, const char *json)
void *c;
int i;
- set_syscall("rt_sigaction", 1);
- set_syscall("sigreturn", 1);
- set_syscall("rt_sigreturn", 1);
- set_syscall("exit_group", 1);
- set_syscall("exit", 1);
+ if (mode == UTRACE) {
+ set_syscall("rt_sigaction", 1);
+ set_syscall("sigreturn", 1);
+ set_syscall("rt_sigreturn", 1);
+ set_syscall("exit_group", 1);
+ set_syscall("exit", 1);
+ }
struct syscall sorted[ARRAY_SIZE(syscall_names)];
@@ -151,6 +169,30 @@ static void print_syscalls(int policy, const char *json)
}
+static void report_seccomp_vialation(pid_t pid, unsigned syscall)
+{
+ char buf[200];
+ snprintf(buf, sizeof(buf), "/proc/%d/cmdline", pid);
+ int f = open(buf, O_RDONLY);
+ int r = read(f, buf, sizeof(buf) - 1);
+ if (r >= 0)
[LEDE-DEV] [PATCH procd 03/17] Do not disable seccomp when configuration is not found
Previously, when seccomp configuration file for a service was not
found, the service was started without seccomp. I consider this
potential attack vector.
With this change, procd starts the service as if the configuration
existed but the service fails in libpreload-seccomp.so, because the
configuration cannot be loaded. This is announced in the syslog.
Signed-off-by: Michal Sojka
---
service/instance.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/service/instance.c b/service/instance.c
index bb766ea..dc7e3ca 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -873,15 +873,8 @@ instance_config_parse(struct service_instance *in)
if (tb[INSTANCE_ATTR_NO_NEW_PRIVS])
in->no_new_privs =
blobmsg_get_bool(tb[INSTANCE_ATTR_NO_NEW_PRIVS]);
- if (!in->trace && tb[INSTANCE_ATTR_SECCOMP]) {
- char *seccomp = blobmsg_get_string(tb[INSTANCE_ATTR_SECCOMP]);
- struct stat s;
-
- if (stat(seccomp, &s))
- ERROR("%s: not starting seccomp as %s is missing\n",
in->name, seccomp);
- else
- in->seccomp = seccomp;
- }
+ if (!in->trace && tb[INSTANCE_ATTR_SECCOMP])
+ in->seccomp = blobmsg_get_string(tb[INSTANCE_ATTR_SECCOMP]);
if (tb[INSTANCE_ATTR_PIDFILE]) {
char *pidfile = blobmsg_get_string(tb[INSTANCE_ATTR_PIDFILE]);
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 12/17] preload-seccomp: Use proper log level for error messages
Signed-off-by: Michal Sojka
---
jail/seccomp.c | 10 +-
jail/seccomp.h | 4
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/jail/seccomp.c b/jail/seccomp.c
index 1a2bb27..27bf3ce 100644
--- a/jail/seccomp.c
+++ b/jail/seccomp.c
@@ -67,13 +67,13 @@ int install_syscall_filter(const char *argv, const char
*file)
blob_buf_init(&b, 0);
if (!blobmsg_add_json_from_file(&b, file)) {
- INFO("%s: failed to load %s\n", argv, file);
+ ERROR("%s: failed to load %s\n", argv, file);
return -1;
}
blobmsg_parse(policy, __SECCOMP_MAX, tb, blob_data(b.head),
blob_len(b.head));
if (!tb[SECCOMP_WHITELIST]) {
- INFO("%s: %s is missing the syscall table\n", argv, file);
+ ERROR("%s: %s is missing the syscall table\n", argv, file);
return -1;
}
@@ -85,7 +85,7 @@ int install_syscall_filter(const char *argv, const char *file)
filter = calloc(sz, sizeof(struct sock_filter));
if (!filter) {
- INFO("failed to allocate filter memory\n");
+ ERROR("failed to allocate filter memory\n");
return -1;
}
@@ -125,7 +125,7 @@ int install_syscall_filter(const char *argv, const char
*file)
set_filter(&filter[idx], BPF_RET + BPF_K, 0, 0,
SECCOMP_RET_KILL);
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
- INFO("%s: prctl(PR_SET_NO_NEW_PRIVS) failed: %s\n", argv,
strerror(errno));
+ ERROR("%s: prctl(PR_SET_NO_NEW_PRIVS) failed: %s\n", argv,
strerror(errno));
return errno;
}
@@ -133,7 +133,7 @@ int install_syscall_filter(const char *argv, const char
*file)
prog.filter = filter;
if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
- INFO("%s: prctl(PR_SET_SECCOMP) failed: %s\n", argv,
strerror(errno));
+ ERROR("%s: prctl(PR_SET_SECCOMP) failed: %s\n", argv,
strerror(errno));
return errno;
}
return 0;
diff --git a/jail/seccomp.h b/jail/seccomp.h
index 6032812..24c1dd7 100644
--- a/jail/seccomp.h
+++ b/jail/seccomp.h
@@ -20,6 +20,10 @@
syslog(LOG_INFO,"preload-seccomp: "fmt, ## __VA_ARGS__); \
fprintf(stderr,"preload-seccomp: "fmt, ## __VA_ARGS__); \
} while (0)
+#define ERROR(fmt, ...) do { \
+ syslog(LOG_ERR,"preload-seccomp: "fmt, ## __VA_ARGS__); \
+ fprintf(stderr,"preload-seccomp: "fmt, ## __VA_ARGS__); \
+ } while (0)
int install_syscall_filter(const char *argv, const char *file);
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 06/17] utrace: Trace processes across forks
Without this change, utrace can trace only a single process. When the
process forks, syscalls of its children do not appear in utrace
output. This is a problem, because seccomp filters are inherited by
children and therefore filters generated by utrace may lack legitimate
syscalls.
This commit enables utrace to trace processes across forks. The
functionality can be demonstrated by the following examples:
utrace /bin/touch /tmp/xxx
produces:
{
"whitelist": [
"rt_sigaction",
"rt_sigreturn",
"exit",
"getuid",
"exit_group",
"utimensat"
],
"policy": 1
}
The command:
utrace /bin/sh -c 'touch /tmp/xxx'
without this commit produces:
{
"whitelist": [
"stat",
"rt_sigaction",
"rt_sigprocmask",
"rt_sigreturn",
"getpid",
"fork",
"exit",
"wait4",
"uname",
"getcwd",
"getuid",
"getppid",
"exit_group"
],
"policy": 1
}
but with this commit, the output is the following:
{
"whitelist": [
"read",
"open",
"close",
"stat",
"fstat",
"mmap",
"mprotect",
"rt_sigaction",
"rt_sigprocmask",
"rt_sigreturn",
"getpid",
"fork",
"execve",
"exit",
"wait4",
"uname",
"fcntl",
"getcwd",
"getuid",
"getppid",
"arch_prctl",
"gettid",
"set_tid_address",
"exit_group",
"utimensat"
],
"policy": 1
}
Note that in addition to utimensat syscall from touch, this output
contains more syscalls than what is in the union of single-process sh
and touch traces. The reason is that single-process traces do not
include syscalls from dynamic linker (due to preload trick), but the
trace of forked processes includes the dynamic linker syscalls. This
is unavoidable, because dynamic linker of the forked processes will be
subject to seccomp filters of the parent process.
Signed-off-by: Michal Sojka
---
trace/trace.c | 64 +--
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/trace/trace.c b/trace/trace.c
index 35bc548..f882c2e 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -57,11 +57,15 @@
fprintf(stderr, "utrace: "fmt, ## __VA_ARGS__); \
} while (0)
-static struct uloop_process tracer;
+struct tracee {
+ struct uloop_process proc;
+ int in_syscall;
+};
+
+static struct tracee tracer;
static int *syscall_count;
static struct blob_buf b;
static int syscall_max;
-static int in_syscall;
static int debug;
static int max_syscall = ARRAY_SIZE(syscall_names);
@@ -143,25 +147,45 @@ static void print_syscalls(int policy, const char *json)
static void tracer_cb(struct uloop_process *c, int ret)
{
- if (WIFSTOPPED(ret) && WSTOPSIG(ret) & 0x80) {
- if (!in_syscall) {
- int syscall = ptrace(PTRACE_PEEKUSER, c->pid,
reg_syscall_nr);
-
- if (syscall < syscall_max) {
- syscall_count[syscall]++;
- if (debug)
- fprintf(stderr, "%s()\n",
syscall_names[syscall]);
- } else if (debug) {
- fprintf(stderr, "syscal(%d)\n", syscall);
+ struct tracee *tracee = container_of(c, struct tracee, proc);
+
+ if (WIFSTOPPED(ret)) {
+ if (WSTOPSIG(ret) & 0x80) {
+ if (!tracee->in_syscall) {
+ int syscall = ptrace(PTRACE_PEEKUSER, c->pid,
reg_syscall_nr);
+
+ if (syscall < syscall_max) {
+
[LEDE-DEV] [PATCH procd 14/17] utrace: Report ptrace errors
--- trace/trace.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/trace/trace.c b/trace/trace.c index 6fbe608..eead9c5 100644 --- a/trace/trace.c +++ b/trace/trace.c @@ -25,6 +25,7 @@ #include #include #include +#include #ifndef PTRACE_EVENT_STOP /* PTRACE_EVENT_STOP is defined in linux/ptrace.h, but this header @@ -351,8 +352,10 @@ int main(int argc, char **argv, char **envp) ptrace_restart = PTRACE_CONT; break; } - ptrace(PTRACE_SEIZE, child, 0, ptrace_options); - ptrace(ptrace_restart, child, 0, SIGCONT); + if (ptrace(PTRACE_SEIZE, child, 0, ptrace_options) == -1) + err(1, "PTRACE_SEIZE"); + if (ptrace(ptrace_restart, child, 0, SIGCONT) == -1) + err(1, "ptrace restart"); uloop_init(); tracer.proc.pid = child; -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 09/17] utrace: Use PTHREAD_SEIZE instead of PTHREAD_TRACEME
This makes it easier to handle initial ptrace-stops (after
fork/clone/...), because we don't need to distinguish whether SIGSTOP
is from user or from ptrace. Also execve() does not deliver an extra
SIGTRAP, which we would have to handle.
Signed-off-by: Michal Sojka
---
trace/preload.c | 1 -
trace/trace.c | 19 +++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/trace/preload.c b/trace/preload.c
index 99dd906..ee97429 100644
--- a/trace/preload.c
+++ b/trace/preload.c
@@ -32,7 +32,6 @@ static main_t __main__;
static int __preload_main__(int argc, char **argv, char **envp)
{
unsetenv("LD_PRELOAD");
- ptrace(PTRACE_TRACEME);
kill(getpid(), SIGSTOP);
return (*__main__)(argc, argv, envp);
diff --git a/trace/trace.c b/trace/trace.c
index cad2d37..9d3cd0a 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -24,6 +24,12 @@
#include
#include
+#ifndef PTRACE_EVENT_STOP
+/* PTRACE_EVENT_STOP is defined in linux/ptrace.h, but this header
+ * collides with musl's sys/ptrace.h */
+#define PTRACE_EVENT_STOP 128
+#endif
+
#include
#include
#include
@@ -150,7 +156,10 @@ static void tracer_cb(struct uloop_process *c, int ret)
struct tracee *tracee = container_of(c, struct tracee, proc);
int inject_signal = 0;
- if (WIFSTOPPED(ret)) {
+ /* We explicitely check for events in upper 16 bits, because
+* musl (as opposed to glibc) does not report
+* PTRACE_EVENT_STOP as WIFSTOPPED */
+ if (WIFSTOPPED(ret) || (ret >> 16)) {
if (WSTOPSIG(ret) & 0x80) {
if (!tracee->in_syscall) {
int syscall = ptrace(PTRACE_PEEKUSER, c->pid,
reg_syscall_nr);
@@ -175,6 +184,8 @@ static void tracer_cb(struct uloop_process *c, int ret)
uloop_process_add(&child->proc);
if (debug)
fprintf(stderr, "Tracing new child %d\n",
child->proc.pid);
+ } else if ((ret >> 16) == PTRACE_EVENT_STOP) {
+ /* Nothing special to do here */
} else {
inject_signal = WSTOPSIG(ret);
if (debug)
@@ -254,18 +265,18 @@ int main(int argc, char **argv, char **envp)
syscall_max = ARRAY_SIZE(syscall_names);
syscall_count = calloc(syscall_max, sizeof(int));
- waitpid(child, &status, 0);
+ waitpid(child, &status, WUNTRACED);
if (!WIFSTOPPED(status)) {
ERROR("failed to start %s\n", *argv);
return -1;
}
- ptrace(PTRACE_SETOPTIONS, child, 0,
+ ptrace(PTRACE_SEIZE, child, 0,
PTRACE_O_TRACESYSGOOD |
PTRACE_O_TRACEFORK |
PTRACE_O_TRACEVFORK |
PTRACE_O_TRACECLONE);
- ptrace(PTRACE_SYSCALL, child, 0, 0);
+ ptrace(PTRACE_SYSCALL, child, 0, SIGCONT);
uloop_init();
tracer.proc.pid = child;
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 08/17] utrace: Deliver signals to traced processes
Without this change, traced processes do not receive any signal,
because all the signals are "eaten" by the tracer.
Signed-off-by: Michal Sojka
---
trace/trace.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/trace/trace.c b/trace/trace.c
index 4c25a4f..cad2d37 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -148,6 +148,7 @@ static void print_syscalls(int policy, const char *json)
static void tracer_cb(struct uloop_process *c, int ret)
{
struct tracee *tracee = container_of(c, struct tracee, proc);
+ int inject_signal = 0;
if (WIFSTOPPED(ret)) {
if (WSTOPSIG(ret) & 0x80) {
@@ -174,8 +175,13 @@ static void tracer_cb(struct uloop_process *c, int ret)
uloop_process_add(&child->proc);
if (debug)
fprintf(stderr, "Tracing new child %d\n",
child->proc.pid);
+ } else {
+ inject_signal = WSTOPSIG(ret);
+ if (debug)
+ fprintf(stderr, "Injecting signal %d into pid
%d\n",
+ inject_signal, tracee->proc.pid);
}
- } else if (WIFEXITED(ret)) {
+ } else if (WIFEXITED(ret) || (WIFSIGNALED(ret) && WTERMSIG(ret))) {
if (tracee == &tracer) {
uloop_end(); /* Main process exit */
} else {
@@ -186,7 +192,7 @@ static void tracer_cb(struct uloop_process *c, int ret)
return;
}
- ptrace(PTRACE_SYSCALL, c->pid, 0, 0);
+ ptrace(PTRACE_SYSCALL, c->pid, 0, inject_signal);
uloop_process_add(c);
}
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 07/17] utrace: Support tracing multi-threaded processes and vfork
Signed-off-by: Michal Sojka
---
trace/trace.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/trace/trace.c b/trace/trace.c
index f882c2e..4c25a4f 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -163,7 +163,9 @@ static void tracer_cb(struct uloop_process *c, int ret)
}
}
tracee->in_syscall = !tracee->in_syscall;
- } else if ((ret >> 8) == (SIGTRAP | (PTRACE_EVENT_FORK << 8))) {
+ } else if ((ret >> 8) == (SIGTRAP | (PTRACE_EVENT_FORK << 8)) ||
+ (ret >> 8) == (SIGTRAP | (PTRACE_EVENT_VFORK << 8))
||
+ (ret >> 8) == (SIGTRAP | (PTRACE_EVENT_CLONE << 8)))
{
struct tracee *child = calloc(1, sizeof(struct tracee));
ptrace(PTRACE_GETEVENTMSG, c->pid, 0, &child->proc.pid);
@@ -254,7 +256,9 @@ int main(int argc, char **argv, char **envp)
ptrace(PTRACE_SETOPTIONS, child, 0,
PTRACE_O_TRACESYSGOOD |
- PTRACE_O_TRACEFORK);
+ PTRACE_O_TRACEFORK |
+ PTRACE_O_TRACEVFORK |
+ PTRACE_O_TRACECLONE);
ptrace(PTRACE_SYSCALL, child, 0, 0);
uloop_init();
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 05/17] utrace: Sort syscalls by number of invocations
seccomp and service jailing announce email [1] mentioned that "utrace
tool will sort the syscalls by the number of invocations". The code
did not do that until this commit.
[1] https://lists.openwrt.org/pipermail/openwrt-devel/2015-March/032197.html
Signed-off-by: Michal Sojka
---
trace/trace.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/trace/trace.c b/trace/trace.c
index 65fe067..35bc548 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -77,6 +77,16 @@ static void set_syscall(const char *name, int val)
}
}
+struct syscall {
+ int syscall;
+ int count;
+};
+
+static int cmp_count(const void *a, const void *b)
+{
+ return ((struct syscall*)b)->count - ((struct syscall*)a)->count;
+}
+
static void print_syscalls(int policy, const char *json)
{
void *c;
@@ -88,19 +98,29 @@ static void print_syscalls(int policy, const char *json)
set_syscall("exit_group", 1);
set_syscall("exit", 1);
+ struct syscall sorted[ARRAY_SIZE(syscall_names)];
+
+ for (i = 0; i < ARRAY_SIZE(syscall_names); i++) {
+ sorted[i].syscall = i;
+ sorted[i].count = syscall_count[i];
+ }
+
+ qsort(sorted, ARRAY_SIZE(syscall_names), sizeof(sorted[0]), cmp_count);
+
blob_buf_init(&b, 0);
c = blobmsg_open_array(&b, "whitelist");
for (i = 0; i < ARRAY_SIZE(syscall_names); i++) {
- if (!syscall_count[i])
- continue;
- if (syscall_names[i]) {
+ int sc = sorted[i].syscall;
+ if (!sorted[i].count)
+ break;
+ if (syscall_names[sc]) {
if (debug)
printf("syscall %d (%s) was called %d times\n",
- i, syscall_names[i], syscall_count[i]);
- blobmsg_add_string(&b, NULL, syscall_names[i]);
+ sc, syscall_names[sc], sorted[i].count);
+ blobmsg_add_string(&b, NULL, syscall_names[sc]);
} else {
- ERROR("no name found for syscall(%d)\n", i);
+ ERROR("no name found for syscall(%d)\n", sc);
}
}
blobmsg_close_array(&b, c);
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] procd seccomp enhancements
Hi all, this patch series enhances seccomp sandboxing of procd services. It introduces two main features: 1. Support for multi-threaded and multi-process services (previously, utrace which creates seccomp whitelists was usable only for single-threaded processes) and 2. logging of seccomp violations via a user-space tracer rather than via kernel patch. In addition to that, there are many bug fixes and smaller enhancements such as support for tracing non-contiguous syscalls on ARM. libubox patches (2): uloop: Fix race condition in SIGCHLD handling uloop: Enable utracing of multi-threaded programs procd patches (17): utrace: Fix environment initialization utrace: Fix off-by-one errors Do not disable seccomp when configuration is not found Update trace attribute utrace: Sort syscalls by number of invocations utrace: Trace processes across forks utrace: Support tracing multi-threaded processes and vfork utrace: Deliver signals to traced processes utrace: Use PTHREAD_SEIZE instead of PTHREAD_TRACEME seccomp: Log seccomp violations with utrace Start seccomp-enabled services via seccomp-trace preload-seccomp: Use proper log level for error messages seccomp: Improve error message utrace: Report ptrace errors utrace: Forward SIGTERM to the traced process utrace: Support non-contiguous syscall numbers utrace: Switch all logging to ulog source patches (1): procd: Install seccomp-trace symlink procd diffstat: jail/preload.c | 5 + jail/seccomp-bpf.h | 1 + jail/seccomp.c | 24 ++--- jail/seccomp.h | 4 + make_syscall_h.sh | 48 - service/instance.c | 21 ++-- trace/preload.c| 1 - trace/trace.c | 279 + 8 files changed, 291 insertions(+), 92 deletions(-) -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 15/17] utrace: Forward SIGTERM to the traced process
When a service is started with "/etc/init.d/ trace" or when
it has seccomp enabled (i.e. runs under seccomp-trace), stopping the
service with "/etc/init.d/ stop" stops only the tracer. The
service itself continue executing. This patch ensures that the service
is terminated as well.
Signed-off-by: Michal Sojka
---
trace/trace.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/trace/trace.c b/trace/trace.c
index eead9c5..d86c215 100644
--- a/trace/trace.c
+++ b/trace/trace.c
@@ -254,6 +254,15 @@ static void tracer_cb(struct uloop_process *c, int ret)
uloop_process_add(c);
}
+static void sigterm_handler(int signum)
+{
+ /* When we receive SIGTERM, we forward it to the tracee. After
+* the tracee exits, trace_cb() will be called and make us
+* exit too. */
+ kill(tracer.proc.pid, SIGTERM);
+}
+
+
int main(int argc, char **argv, char **envp)
{
int status, ch, policy = EPERM;
@@ -361,6 +370,7 @@ int main(int argc, char **argv, char **envp)
tracer.proc.pid = child;
tracer.proc.cb = tracer_cb;
uloop_process_add(&tracer.proc);
+ signal(SIGTERM, sigterm_handler); /* Override uloop's SIGTERM handler */
uloop_run();
uloop_done();
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 02/17] utrace: Fix off-by-one errors
This fixes two errors: 1) memcpy() copies envc elements starting from index 1, so the number of elements in target array should be envc + 1. But only envc was allocated. 2) If original environment envp is empty, i.e. it contains only a NULL element, the while loop misses it. Signed-off-by: Michal Sojka --- trace/trace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trace/trace.c b/trace/trace.c index 04bf7a5..65fe067 100644 --- a/trace/trace.c +++ b/trace/trace.c @@ -177,7 +177,7 @@ int main(int argc, char **argv, char **envp) char **_argv = calloc(argc + 1, sizeof(char *)); char **_envp; char *preload = "LD_PRELOAD=/lib/libpreload-trace.so"; - int envc = 1; + int envc = 0; int ret; memcpy(_argv, argv, argc * sizeof(char *)); @@ -185,7 +185,7 @@ int main(int argc, char **argv, char **envp) while (envp[envc++]) ; - _envp = calloc(envc, sizeof(char *)); + _envp = calloc(envc + 1, sizeof(char *)); memcpy(&_envp[1], envp, envc * sizeof(char *)); *_envp = preload; -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH libubox 1/2] uloop: Fix race condition in SIGCHLD handling
When uloop_process_add() is called outside of uloop_run(), i.e. not
from a callback (which is the case of at least utrace and ujail),
child events can be missed. The reason is that when SIGCHILD handler
is installed in uloop_run(), after the uloop_process_add() is called,
then an initial signal could be missed.
Commit 4e3a47a ("uloop: use a waker for notifying sigchld and loop
cancel events", 2016-06-09) solved a similar problem and introduced
uloop_init() but forgot to move a call to uloop_setup_signals() there.
This is what this commit does.
Now, uloop_process_add() can be called any time after uloop_init()
without missing any event.
Signed-off-by: Michal Sojka
---
uloop.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/uloop.c b/uloop.c
index d2f41bb..3813e18 100644
--- a/uloop.c
+++ b/uloop.c
@@ -116,6 +116,8 @@ static int waker_init(void)
return 0;
}
+static void uloop_setup_signals(bool add);
+
int uloop_init(void)
{
if (uloop_init_pollfd() < 0)
@@ -126,6 +128,8 @@ int uloop_init(void)
return -1;
}
+ uloop_setup_signals(true);
+
return 0;
}
@@ -525,12 +529,7 @@ int uloop_run_timeout(int timeout)
int next_time = 0;
struct timeval tv;
- /*
-* Handlers are only updated for the first call to uloop_run() (and
restored
-* when this call is done).
-*/
- if (!uloop_run_depth++)
- uloop_setup_signals(true);
+ uloop_run_depth++;
uloop_status = 0;
uloop_cancelled = false;
@@ -553,14 +552,15 @@ int uloop_run_timeout(int timeout)
uloop_run_events(next_time);
}
- if (!--uloop_run_depth)
- uloop_setup_signals(false);
+ --uloop_run_depth;
return uloop_status;
}
void uloop_done(void)
{
+ uloop_setup_signals(false);
+
if (poll_fd >= 0) {
close(poll_fd);
poll_fd = -1;
--
2.14.1
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 01/17] utrace: Fix environment initialization
We want to copy the existing environment instead of the new one to itself. Other bugs in this code are fixed in the next commit. Signed-off-by: Michal Sojka --- trace/trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace/trace.c b/trace/trace.c index fdffaba..04bf7a5 100644 --- a/trace/trace.c +++ b/trace/trace.c @@ -186,7 +186,7 @@ int main(int argc, char **argv, char **envp) ; _envp = calloc(envc, sizeof(char *)); - memcpy(&_envp[1], _envp, envc * sizeof(char *)); + memcpy(&_envp[1], envp, envc * sizeof(char *)); *_envp = preload; ret = execve(_argv[0], _argv, _envp); -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH procd 04/17] Update trace attribute
When a service is started for the first time without trace attribute (e.g. during boot), then it was impossible to restart it in tracing mode (/etc/init.d/service trace). This is fixed here. Signed-off-by: Michal Sojka --- service/instance.c | 1 + 1 file changed, 1 insertion(+) diff --git a/service/instance.c b/service/instance.c index dc7e3ca..1760a09 100644 --- a/service/instance.c +++ b/service/instance.c @@ -941,6 +941,7 @@ instance_config_move(struct service_instance *in, struct service_instance *in_sr in->command = in_src->command; in->pidfile = in_src->pidfile; in->name = in_src->name; + in->trace = in_src->trace; in->node.avl.key = in_src->node.avl.key; free(in->config); -- 2.14.1 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] procd: Do not leak pipe file descriptors to children
Without this change, a process started by procd can have access to
stdout/err of processes started by procd before.
Signed-off-by: Michal Sojka
---
rcS.c | 1 +
service/instance.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/rcS.c b/rcS.c
index 4ecf0c1..b1202bf 100644
--- a/rcS.c
+++ b/rcS.c
@@ -82,6 +82,7 @@ static void q_initd_run(struct runqueue *q, struct
runqueue_task *t)
if (pid) {
close(pipefd[1]);
+ fcntl(pipefd[0], F_SETFD, FD_CLOEXEC);
s->fd.stream.string_data = true,
s->fd.stream.notify_read = pipe_cb,
runqueue_process_add(q, &s->proc, pid);
diff --git a/service/instance.c b/service/instance.c
index e5c4830..bb766ea 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -444,11 +444,13 @@ instance_start(struct service_instance *in)
if (opipe[0] > -1) {
ustream_fd_init(&in->_stdout, opipe[0]);
closefd(opipe[1]);
+ fcntl(opipe[0], F_SETFD, FD_CLOEXEC);
}
if (epipe[0] > -1) {
ustream_fd_init(&in->_stderr, epipe[0]);
closefd(epipe[1]);
+ fcntl(epipe[0], F_SETFD, FD_CLOEXEC);
}
service_event("instance.start", in->srv->name, in->name);
--
2.13.2
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH v2] image.mk: Generate cpiogz with root-owned files
Some files (e.g. /etc/dropbear) need to be owned by root. Add cpio option to ensure that. Other image types (at least targz and squashfs) already have this. Signed-off-by: Michal Sojka --- include/image.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/image.mk b/include/image.mk index 81519cd183..065d619dfb 100644 --- a/include/image.mk +++ b/include/image.mk @@ -277,7 +277,7 @@ endif ifdef CONFIG_TARGET_ROOTFS_CPIOGZ define Image/Build/cpiogz - ( cd $(TARGET_DIR); find . | cpio -o -H newc | gzip -9n >$(BIN_DIR)/$(IMG_PREFIX)-rootfs.cpio.gz ) + ( cd $(TARGET_DIR); find . | cpio -o -H newc -R root:root | gzip -9n >$(BIN_DIR)/$(IMG_PREFIX)-rootfs.cpio.gz ) endef endif -- 2.11.0 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] procd/rcS: Use /dev/null as stdin
This change ensures that /etc/init.d/* scripts are started with
/dev/null as stdin. This is useful in cases where an init.d script
reads (e.g. by mistake) from stdin, which a user can perceive as if
some characters typed into shell on serial console are "eaten" by
something else (i.e. by the init.d script running on background). This
is very annoying, because each character needs to be pressed several
times before it appears on the screen.
Signed-off-by: Michal Sojka
---
rcS.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/rcS.c b/rcS.c
index 70d7505..e1c308a 100644
--- a/rcS.c
+++ b/rcS.c
@@ -90,9 +90,15 @@ static void q_initd_run(struct runqueue *q, struct
runqueue_task *t)
return;
}
close(pipefd[0]);
+
+ int devnull = open("/dev/null", O_RDONLY);
+ dup2(devnull, STDIN_FILENO);
dup2(pipefd[1], STDOUT_FILENO);
dup2(pipefd[1], STDERR_FILENO);
+ if (devnull > STDERR_FILENO)
+ close(devnull);
+
execlp(s->file, s->file, s->param, NULL);
exit(1);
}
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] image.mk: Generate cpiogz with root-owned files
Some files (e.g. /etc/dropbear) need to be owned by root. Add cpio option to ensure that. Other image types (at least targz and squashfs) already have this. Signed-off-by: Michal Sojka --- include/image.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/image.mk b/include/image.mk index 81519cd183..53fe25e71e 100644 --- a/include/image.mk +++ b/include/image.mk @@ -277,7 +277,7 @@ endif ifdef CONFIG_TARGET_ROOTFS_CPIOGZ define Image/Build/cpiogz - ( cd $(TARGET_DIR); find . | cpio -o -H newc | gzip -9n >$(BIN_DIR)/$(IMG_PREFIX)-rootfs.cpio.gz ) + ( cd $(TARGET_DIR); find . | cpio -o -H newc --owner=root:root | gzip -9n >$(BIN_DIR)/$(IMG_PREFIX)-rootfs.cpio.gz ) endef endif -- 2.11.0 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH] build: Fix unpacking of overlaid packages
Hi Yousong, On Thu, Mar 23 2017, Yousong Zhou wrote: > On 18 March 2017 at 06:54, Michal Sojka wrote: >> On Fri, Mar 17 2017, Michal Sojka wrote: >>> Since commit 5af113eb7c0f6e4b2d09e536e1a2adc4e2d6071d, LEDE's build >>> system contains overlay functionality, which allows to locally amend >>> properties of packages. When one wants to overlay certain properties >>> such as PKG_VERSION, the unpack command (UNPACK_CMD) uses the original >>> value rather than the overlaid one and unpacking fails. >> >> Hmm, there are more places where is this overlay functionality broken. >> I'll send another patch that makes it work for me, but there are places, >> which cannot be fixed that easy. One of them is the -iremap CFLAG, which >> is appended to TARGET_CFLAGS with "+=" operator, which expands to the >> non-overlaid path. >> >> -Michal > > The change will incur too much implementation details and is very > likely hard to maintain later. Yes, I think the same. IMHO, the best thing to do would be to remove the package overlay code, because it either does not work at all or it works only in a few very limited (and undocumented) cases. > In your case, I would recommend either maintaining your own branch, or > maintain a package feed for your needs and prefer it at feed install > time. What do you mean by "prefer at feed install time"? Thanks -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] build: Fix stamps of overlaid packages
---
include/package.mk | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/package.mk b/include/package.mk
index 91871f06fb..14321642c0 100644
--- a/include/package.mk
+++ b/include/package.mk
@@ -52,16 +52,16 @@ find_library_dependencies = $(wildcard $(patsubst
%,$(STAGING_DIR)/pkginfo/%.ver
PKG_DIR_NAME:=$(lastword $(subst /,$(space),$(CURDIR)))
STAMP_NO_AUTOREBUILD=$(wildcard $(PKG_BUILD_DIR)/.no_autorebuild)
-PREV_STAMP_PREPARED:=$(if $(STAMP_NO_AUTOREBUILD),$(wildcard
$(PKG_BUILD_DIR)/.prepared*))
+PREV_STAMP_PREPARED=$(if $(STAMP_NO_AUTOREBUILD),$(wildcard
$(PKG_BUILD_DIR)/.prepared*))
ifneq ($(PREV_STAMP_PREPARED),)
- STAMP_PREPARED:=$(PREV_STAMP_PREPARED)
+ STAMP_PREPARED=$(PREV_STAMP_PREPARED)
CONFIG_AUTOREBUILD:=
else
STAMP_PREPARED=$(PKG_BUILD_DIR)/.prepared$(if $(QUILT)$(DUMP),,_$(shell
$(call find_md5,${CURDIR} $(PKG_FILE_DEPENDS),))_$(call
confvar,CONFIG_AUTOREMOVE $(PKG_PREPARED_DEPENDS)))
endif
STAMP_CONFIGURED=$(PKG_BUILD_DIR)/.configured$(if $(DUMP),,_$(call
confvar,$(PKG_CONFIG_DEPENDS)))
STAMP_CONFIGURED_WILDCARD=$(PKG_BUILD_DIR)/.configured_*
-STAMP_BUILT:=$(PKG_BUILD_DIR)/.built
+STAMP_BUILT=$(PKG_BUILD_DIR)/.built
STAMP_INSTALLED:=$(STAGING_DIR)/stamp/.$(PKG_DIR_NAME)$(if
$(BUILD_VARIANT),.$(BUILD_VARIANT),)_installed
STAGING_FILES_LIST:=$(PKG_DIR_NAME)$(if
$(BUILD_VARIANT),.$(BUILD_VARIANT),).list
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH] build: Fix unpacking of overlaid packages
On Fri, Mar 17 2017, Michal Sojka wrote: > Since commit 5af113eb7c0f6e4b2d09e536e1a2adc4e2d6071d, LEDE's build > system contains overlay functionality, which allows to locally amend > properties of packages. When one wants to overlay certain properties > such as PKG_VERSION, the unpack command (UNPACK_CMD) uses the original > value rather than the overlaid one and unpacking fails. Hmm, there are more places where is this overlay functionality broken. I'll send another patch that makes it work for me, but there are places, which cannot be fixed that easy. One of them is the -iremap CFLAG, which is appended to TARGET_CFLAGS with "+=" operator, which expands to the non-overlaid path. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] build: Fix unpacking of overlaid packages
Since commit 5af113eb7c0f6e4b2d09e536e1a2adc4e2d6071d, LEDE's build system contains overlay functionality, which allows to locally amend properties of packages. When one wants to overlay certain properties such as PKG_VERSION, the unpack command (UNPACK_CMD) uses the original value rather than the overlaid one and unpacking fails. Fix that problem by replacing simply expanded variable with recursively expanded variable. This ensures that the unpack command uses the overlaid values, because the variable expansion is deferred after the overlay is loaded. Signed-off-by: Michal Sojka Signed-off-by: Michal Vokáč --- include/unpack.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/unpack.mk b/include/unpack.mk index a139827490..6e4de7ec8e 100644 --- a/include/unpack.mk +++ b/include/unpack.mk @@ -21,15 +21,15 @@ ifeq ($(strip $(UNPACK_CMD)),) ifeq ($(filter gz tgz,$(EXT)),$(EXT)) EXT:=$(call ext,$(PKG_SOURCE:.$(EXT)=)) - DECOMPRESS_CMD:=gzip -dc $(DL_DIR)/$(PKG_SOURCE) | + DECOMPRESS_CMD=gzip -dc $(DL_DIR)/$(PKG_SOURCE) | endif ifeq ($(filter bzip2 bz2 bz tbz2 tbz,$(EXT)),$(EXT)) EXT:=$(call ext,$(PKG_SOURCE:.$(EXT)=)) - DECOMPRESS_CMD:=bzcat $(DL_DIR)/$(PKG_SOURCE) | + DECOMPRESS_CMD=bzcat $(DL_DIR)/$(PKG_SOURCE) | endif ifeq ($(filter xz txz,$(EXT)),$(EXT)) EXT:=$(call ext,$(PKG_SOURCE:.$(EXT)=)) - DECOMPRESS_CMD:=xzcat $(DL_DIR)/$(PKG_SOURCE) | + DECOMPRESS_CMD=xzcat $(DL_DIR)/$(PKG_SOURCE) | endif ifeq ($(filter tgz tbz tbz2 txz,$(EXT1)),$(EXT1)) EXT:=tar -- 2.11.0 ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH 2/3] procd: Don't use syslog before its initialization
Hi David, I understand what you're saying and you are right that there can be a lot of problems (e.g. hangs) due logging. Some comments below. On Sat, Mar 11 2017, David Lang wrote: > On Sat, 11 Mar 2017, Michal Sojka wrote: > >> On Sat, Mar 11 2017, John Crispin wrote: >>> On 11/03/17 01:48, Michal Sojka wrote: >>>> When procd starts a rcS script, it captures its stdout and stderr and >>>> logs them to syslog. It could happen (and I don't know exactly why), >>>> that a rcS scripts produces some output before openlog() is called in >>>> procd. The result is that all subsequent logs from procd are logged >>>> with ident and PID of some other service that happens to be started at >>>> that time. I also have no clue why this is happening (bug in musl?), >>>> but it is very confusing when one tries to debug boot problems. >>> >>> is there a way to reliably reproduce this issue ? >> >> I can reproduce it reliably with handful of custom packages. I'll try to >> prepare a more simpler setup where this can be seen. > > This sounds like a problem with procd, capturing stdout and stderr and > feeding > them to syslog could cause a serious problem if the error messages are from > the > syslog daemon. > > it sounds like they are trying to emulate systemd, and that's not always a > good > thing :-) > > sending a bunch of things to syslog before the system is up can also run into > problems if the syslog daemon is configured to send the logs to a remote > server, > and can't reach it yet (all sorts of possible reasons for that). > > all sorts of programs spit various garbaage out to stdout or stderr that is > meaningless/debug stuff that really should not be turned on for production > use, > but was missed, this can cause a lot of log messages (and stderr messages in > particular tend to be very badly formed) > > this sort of thing in the init system needs safeguards, and those need to be > thought about. > > possibly try to deliver the log to syslog, but throw them away if the attempt > blocks This would require not using syslog(3), but writing a non-blocking variant of it. That's certainly possible, but I don't know how portable it should such be for LEDE. > possibly try to detect that it's starting the syslog daemon and skip > delivering > logs > > poosibly have the startup script for the logging daemon set some lock to turn > on > this functionality (and leave it up to that script to figure out when to do > that) But the script does not know when the logging daemon is ready. Wouldn't it be better for the logging daemon to notify procd when it is ready and procd would start logging via it after that? > and it should be configurable so that people can turn on/off stdout and/or > stderr capturing either per-script and/or globally Now it's configurable for services, but not for scripts that start the services. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH v2] procd: Don't use syslog before its initialization
When procd starts a rcS script, it captures its stdout and stderr and
logs them via syslog(). The problem with that is that the rest of
procd code uses ulog rather than syslog() directly and ulog_open()
doesn't call openlog() immediately, but only after something is logged
with ulog(). This lazy calling of openlog() can result in the
following unwanted behavior:
1) When rcS's stdout/err is logged via syslog(), the log identifier is
not set yet (due to openlog() not called) and so the log message
lacks information about source.
2) procd can also log stdout/err from services. When a message from a
service needs to be logged, ulog_open() is called to change the log
identifier to match the service name/PID. After logging the service
messages, ulog_open() is called again the change the identifier
back to "procd". The lazy call to openlog() means that the messages
logged directly with syslog() will be logged with the
identification of the previously logged service and not of the rcS
script that produced the message.
Both problems are fixed by replacing direct call to syslog() with
ULOG_NOTE, which automatically calls openlog() if needed.
Signed-off-by: Michal Sojka
---
rcS.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rcS.c b/rcS.c
index 0208a75..4813146 100644
--- a/rcS.c
+++ b/rcS.c
@@ -54,7 +54,7 @@ static void pipe_cb(struct ustream *s, int bytes)
break;
*newline = 0;
len = newline + 1 - str;
- syslog(LOG_NOTICE, "%s", str);
+ ULOG_NOTE("%s", str);
#ifdef SHOW_BOOT_ON_CONSOLE
fprintf(stderr, "%s\n", str);
#endif
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH 2/3] procd: Don't use syslog before its initialization
On Sat, Mar 11 2017, Michal Sojka wrote: > On Sat, Mar 11 2017, John Crispin wrote: >> On 11/03/17 01:48, Michal Sojka wrote: >>> When procd starts a rcS script, it captures its stdout and stderr and >>> logs them to syslog. It could happen (and I don't know exactly why), >>> that a rcS scripts produces some output before openlog() is called in >>> procd. The result is that all subsequent logs from procd are logged >>> with ident and PID of some other service that happens to be started at >>> that time. I also have no clue why this is happening (bug in musl?), >>> but it is very confusing when one tries to debug boot problems. >> >> is there a way to reliably reproduce this issue ? > > I can reproduce it reliably with handful of custom packages. I'll try to > prepare a more simpler setup where this can be seen. I've figured out what exactly happened. The fix is correct, but the commit message can be improved. I'll send v2 patch. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH 2/3] procd: Don't use syslog before its initialization
On Sat, Mar 11 2017, John Crispin wrote: > On 11/03/17 01:48, Michal Sojka wrote: >> When procd starts a rcS script, it captures its stdout and stderr and >> logs them to syslog. It could happen (and I don't know exactly why), >> that a rcS scripts produces some output before openlog() is called in >> procd. The result is that all subsequent logs from procd are logged >> with ident and PID of some other service that happens to be started at >> that time. I also have no clue why this is happening (bug in musl?), >> but it is very confusing when one tries to debug boot problems. > > is there a way to reliably reproduce this issue ? I can reproduce it reliably with handful of custom packages. I'll try to prepare a more simpler setup where this can be seen. -Michal ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH 2/3] procd: Don't use syslog before its initialization
When procd starts a rcS script, it captures its stdout and stderr and
logs them to syslog. It could happen (and I don't know exactly why),
that a rcS scripts produces some output before openlog() is called in
procd. The result is that all subsequent logs from procd are logged
with ident and PID of some other service that happens to be started at
that time. I also have no clue why this is happening (bug in musl?),
but it is very confusing when one tries to debug boot problems.
Note that the problem does not appear, when booting with init_debug=5.
This problem seems to be fixed by replacing direct call to syslog()
with ULOG_NOTE, which automatically calls openlog() if needed.
Signed-off-by: Michal Sojka
---
rcS.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rcS.c b/rcS.c
index 0208a75..4813146 100644
--- a/rcS.c
+++ b/rcS.c
@@ -54,7 +54,7 @@ static void pipe_cb(struct ustream *s, int bytes)
break;
*newline = 0;
len = newline + 1 - str;
- syslog(LOG_NOTICE, "%s", str);
+ ULOG_NOTE("%s", str);
#ifdef SHOW_BOOT_ON_CONSOLE
fprintf(stderr, "%s\n", str);
#endif
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH 1/3] procd: Add missing \n in debug message
Signed-off-by: Michal Sojka
---
inittab.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/inittab.c b/inittab.c
index 011d7a6..21172f7 100644
--- a/inittab.c
+++ b/inittab.c
@@ -303,7 +303,7 @@ void procd_inittab(void)
if (regexec(&pat_inittab, line, 5, matches, 0))
continue;
- DEBUG(4, "Parsing inittab - %s", line);
+ DEBUG(4, "Parsing inittab - %s\n", line);
for (i = TAG_ID; i <= TAG_PROCESS; i++) {
line[matches[i].rm_eo] = '\0';
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH 3/3] procd: Log initscript output prefixed with script name
It helps with debugging of initscript problems.
Signed-off-by: Michal Sojka
---
rcS.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/rcS.c b/rcS.c
index 4813146..0dc0aa2 100644
--- a/rcS.c
+++ b/rcS.c
@@ -42,6 +42,7 @@ struct initd {
static void pipe_cb(struct ustream *s, int bytes)
{
+ struct initd *initd = container_of(s, struct initd, fd.stream);
char *newline, *str;
int len;
@@ -54,9 +55,9 @@ static void pipe_cb(struct ustream *s, int bytes)
break;
*newline = 0;
len = newline + 1 - str;
- ULOG_NOTE("%s", str);
+ ULOG_NOTE("%s: %s", initd->file, str);
#ifdef SHOW_BOOT_ON_CONSOLE
- fprintf(stderr, "%s\n", str);
+ fprintf(stderr, "%s: %s\n", initd->file, str);
#endif
ustream_consume(s, len);
} while (1);
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
[LEDE-DEV] [PATCH] scripts/feeds: Reuse TOPDIR if defined in environment
The feeds script sets value of TOPDIR in a way that is inconsistent
with how toplevel Makefile sets it. The inconsistency manifests when I
use a "build directory" with symlinks to LEDE source (see below).
When make is invoked in such a directory, make's TOPDIR variable is
set to that directory, whereas scripts/feeds sets TOPDIR to the top of
LEDE source, which results in creating feeds directory inside the LEDE
source instead of in the build directory.
This patch changes the script so that it reuses the TOPDIR value form
the environment if it exists. The result is that 'make
package/symlinks' correctly fetches feeds to the build directory
instead in the source.
I use the following commands to create the build directory:
ln -s $SRC/config config
ln -s $SRC/Config.in Config.in
ln -s $SRC/feeds.conf.default feeds.conf.default
ln -s $SRC/include include
ln -s $SRC/Makefile Makefile
mkdir package
ln -s $SRC/package/base-files package/base-files
ln -s $SRC/package/boot package/boot
ln -s $SRC/package/devel package/devel
ln -s $SRC/package/firmware package/firmware
ln -s $SRC/package/kernel package/kernel
ln -s $SRC/package/libs package/libs
ln -s $SRC/package/Makefile package/Makefile
ln -s $SRC/package/network package/network
ln -s $SRC/package/system package/system
ln -s $SRC/package/utils package/utils
ln -s $SRC/rules.mk rules.mk
ln -s $SRC/scripts scripts
ln -s $SRC/target target
ln -s $SRC/toolchain toolchain
ln -s $SRC/tools tools
This allows me to easily test changes in LEDE on multiple targets.
Signed-off-by: Michal Sojka
---
scripts/feeds | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/feeds b/scripts/feeds
index 3016eac92d..55c294ad0a 100755
--- a/scripts/feeds
+++ b/scripts/feeds
@@ -9,7 +9,8 @@ use strict;
use Cwd 'abs_path';
chdir "$FindBin::Bin/..";
-$ENV{TOPDIR}=getcwd();
+$ENV{TOPDIR} //= getcwd();
+chdir $ENV{TOPDIR};
$ENV{GIT_CONFIG_PARAMETERS}="'core.autocrlf=false'";
$ENV{GREP_OPTIONS}="";
--
2.11.0
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
