Vcoreid sanity checks in event code
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 17 Dec 2011 00:43:26 +0000 (16:43 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sat, 17 Dec 2011 00:43:26 +0000 (16:43 -0800)
Very large vcoreid's could cause the kernel to find VCPD addresses
outside of procdata.  MAX_NUM_CPUS is what we use for MAX_NUM_VCORES
btw.  No one should want/need more vcores than that.  k/s/pmap.c has
checks built in to make sure procdata stays within its size too.

kern/src/event.c
kern/src/process.c

index 5ec00c0..9003041 100644 (file)
 #include <assert.h>
 #include <pmap.h>
 
+/* Userspace could give us a vcoreid that causes us to compute a vcpd that is
+ * outside procdata.  If we hit UWLIM, then we've gone farther than we should.
+ * We check the vcoreid, instead of the resulting address, to avoid issues like
+ * address wrap-around. */
+static bool vcoreid_is_safe(uint32_t vcoreid)
+{
+       /* MAX_NUM_VCORES == MAX_NUM_CPUS (check procinfo/procdata) */
+       return vcoreid < MAX_NUM_CPUS;
+}
+
 /* Note these three helpers return the user address of the mbox, not the KVA.
  * Load current to access this, and it will work for any process. */
 static struct event_mbox *get_vcpd_mbox_priv(uint32_t vcoreid)
@@ -341,7 +351,7 @@ void send_event(struct proc *p, struct event_queue *ev_q, struct event_msg *msg,
        }
        if (!is_user_rwaddr(ev_q, sizeof(struct event_queue))) {
                /* Ought to kill them, just warn for now */
-               warn("[kernel] Illegal addr for ev_q");
+               printk("[kernel] Illegal addr for ev_q\n");
                return;
        }
        /* ev_q is a user pointer, so we need to make sure we're in the right
@@ -361,6 +371,11 @@ void send_event(struct proc *p, struct event_queue *ev_q, struct event_msg *msg,
                vcoreid = (ev_q->ev_vcore + 1) % p->procinfo->num_vcores;
                ev_q->ev_vcore = vcoreid;
        }
+       if (!vcoreid_is_safe(vcoreid)) {
+               /* Ought to kill them, just warn for now */
+               printk("[kernel] Vcoreid %d unsafe! (too big?)\n", vcoreid);
+               goto out;
+       }
        /* If we're a SPAM_PUBLIC, they just want us to spam the message.  Note we
         * don't care about the mbox, since it'll go to VCPD public mboxes, and
         * we'll prefer to send it to whatever vcoreid we determined at this point
@@ -387,7 +402,7 @@ void send_event(struct proc *p, struct event_queue *ev_q, struct event_msg *msg,
        /* Even if we're using an mbox in procdata (VCPD), we want a user pointer */
        if (!is_user_rwaddr(ev_mbox, sizeof(struct event_mbox))) {
                /* Ought to kill them, just warn for now */
-               printk("[kernel] Illegal addr for ev_mbox");
+               printk("[kernel] Illegal addr for ev_mbox\n");
                goto out;
        }
        /* We used to support no msgs, but quit being lazy and send a 'msg'.  If the
index 752cfb6..98a0eea 100644 (file)
@@ -1161,6 +1161,8 @@ static void __proc_give_a_pcore(struct proc *p, uint32_t pcore)
  * WARNING: You must hold the proc_lock before calling this! */
 void __proc_give_cores(struct proc *SAFE p, uint32_t *pcorelist, size_t num)
 {
+       /* should never happen: */
+       assert(num + p->procinfo->num_vcores <= MAX_NUM_CPUS);
        switch (p->state) {
                case (PROC_RUNNABLE_S):
                case (PROC_RUNNING_S):