strace: Qstrace controls whether tracing is on
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 26 Jan 2017 17:14:42 +0000 (12:14 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 9 Feb 2017 17:31:08 +0000 (12:31 -0500)
The ctl commands straceme and straceall set up tracing.  straceme turns off
inheritance; straceall turns it on.  The actual tracing only happens when
you open #proc/PID/strace (a.k.a. Qstrace).  There can be only one open
chan of this file.

Previously, there was a ctl message to turn off tracing, and tracing would
be turned on as soon as you sent the straceme ctl message.  There are a few
problems with this:

1) If the tracer exits or crashes, the tracee might continue to be traced.
If we start blocking the tracee when the queue is full, it'll just stop
forever.  This way, when the FD of the tracer is closed, tracing will stop.

2) If we want to filter syscalls, we'll want the strace struct to exist
first, and then later set the filter, and the later turn on tracing.

3) Having multiple readers is a mess.  Who knows who is getting the right
output?  That just seemed prone to bugs.

I had to move strace_on and strace_inherit back to the strace struct.  This
means that all processes sharing a struct strace (via inherit) are
controlled as a group.  The open FD for the struct strace is what controls
the tracing - you can't control it for individual processes.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/drivers/dev/proc.c
kern/include/env.h
kern/include/syscall.h
kern/src/syscall.c

index 2a91030..e43f5c4 100644 (file)
@@ -92,7 +92,6 @@ enum {
        CMvmkill,
        CMstraceme,
        CMstraceall,
-       CMstraceoff,
 };
 
 enum {
@@ -159,7 +158,6 @@ struct cmdtab proccmd[] = {
        {CMvmkill, "vmkill", 0},
        {CMstraceme, "straceme", 0},
        {CMstraceall, "straceall", 0},
-       {CMstraceoff, "straceoff", 0},
 };
 
 /*
@@ -622,6 +620,13 @@ static struct chan *procopen(struct chan *c, int omode)
                case Qstrace:
                        if (!p->strace)
                                error(ENOENT, "Process does not have tracing enabled");
+                       spin_lock(&p->strace->lock);
+                       if (p->strace->tracing) {
+                               spin_unlock(&p->strace->lock);
+                               error(EBUSY, "Process is already being traced");
+                       }
+                       p->strace->tracing = TRUE;
+                       spin_unlock(&p->strace->lock);
                        /* the ref we are upping is the one we put in __proc_free, which is
                         * the one we got from CMstrace{on,me}.  We have a ref on p, so we
                         * know we won't free until we decref the proc. */
@@ -837,6 +842,8 @@ static void procclose(struct chan *c)
        if (QID(c->qid) == Qstrace && c->aux != 0) {
                struct strace *s = c->aux;
 
+               assert(c->flag & COPEN);        /* only way aux should have been set */
+               s->tracing = FALSE;
                kref_put(&s->users);
                c->aux = NULL;
        }
@@ -1299,7 +1306,7 @@ void procctlclosefiles(struct proc *p, int all, int fd)
 static void strace_shutdown(struct kref *a)
 {
        struct strace *strace = container_of(a, struct strace, procs);
-       static const char base_msg[] = "Traced ~%lu syscs, Dropped %lu";
+       static const char base_msg[] = "Traced ~%lu syscs, Dropped %lu";
        size_t msg_len = NUMSIZE64 * 2 + sizeof(base_msg);
        char *msg = kmalloc(msg_len, 0);
 
@@ -1343,6 +1350,7 @@ static void procctlreq(struct proc *p, char *va, int n)
                /* common allocation.  if we inherited, we might have one already */
                if (!p->strace) {
                        strace = kzmalloc(sizeof(*p->strace), MEM_WAIT);
+                       spinlock_init(&strace->lock);
                        strace->q = qopen(65536, Qdropoverflow|Qcoalesce, NULL, NULL);
                        /* both of these refs are put when the proc is freed.  procs is for
                         * every process that has this p->strace.  users is procs + every
@@ -1399,16 +1407,10 @@ static void procctlreq(struct proc *p, char *va, int n)
        case CMvminit:
                break;
        case CMstraceme:
-               p->strace_on = TRUE;
-               p->strace_inherit = FALSE;
+               p->strace->inherit = FALSE;
                break;
        case CMstraceall:
-               p->strace_on = TRUE;
-               p->strace_inherit = TRUE;
-               break;
-       case CMstraceoff:
-               p->strace_on = FALSE;
-               p->strace_inherit = FALSE;
+               p->strace->inherit = TRUE;
                break;
        }
        poperror();
index 3c0fa6e..255a5b9 100644 (file)
@@ -118,8 +118,6 @@ struct proc {
        struct vmm vmm;
 
        struct strace                           *strace;
-       bool                                            strace_on;
-       bool                                            strace_inherit;
 };
 
 /* Til we remove all Env references */
index a4793ef..3735ec2 100644 (file)
@@ -50,6 +50,7 @@ struct strace {
        struct kref procs; /* when procs goes to zero, q is hung up. */
        struct kref users; /* when users goes to zero, q and struct are freed. */
        struct queue *q;
+       spinlock_t lock;
 };
 
 extern bool systrace_loud;
index 5802642..2cb47b8 100644 (file)
@@ -117,6 +117,15 @@ static void systrace_output(struct systrace_record *trace,
                printk("%s", trace->pretty_buf);
 }
 
+static bool should_strace(struct proc *p, struct syscall *sysc)
+{
+       if (systrace_loud)
+               return TRUE;
+       if (p->strace && p->strace->tracing)
+               return TRUE;
+       return FALSE;
+}
+
 /* Starts a trace for p running sysc, attaching it to kthread.  Pairs with
  * systrace_finish_trace(). */
 static void systrace_start_trace(struct kthread *kthread, struct syscall *sysc)
@@ -127,7 +136,7 @@ static void systrace_start_trace(struct kthread *kthread, struct syscall *sysc)
        size_t data_len = 0;
 
        kthread->strace = 0;
-       if (!p->strace_on && !systrace_loud)
+       if (!should_strace(p, sysc))
                return;
        trace = kmalloc(SYSTR_BUF_SZ, MEM_ATOMIC);
        if (p->strace) {
@@ -552,13 +561,11 @@ static size_t sys_getvcoreid(struct proc *p)
 /* Helper for proc_create and fork */
 static void inherit_strace(struct proc *parent, struct proc *child)
 {
-       if (parent->strace && parent->strace_inherit) {
+       if (parent->strace && parent->strace->inherit) {
                /* Refcnt on both, put in the child's ->strace. */
                kref_get(&parent->strace->users, 1);
                kref_get(&parent->strace->procs, 1);
                child->strace = parent->strace;
-               child->strace_on = TRUE;
-               child->strace_inherit = TRUE;
        }
 }