Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/kern fix some security critical bugs:



details:   https://anonhg.NetBSD.org/src/rev/a5b1093db7da
branches:  trunk
changeset: 749822:a5b1093db7da
user:      drochner <drochner%NetBSD.org@localhost>
date:      Thu Dec 10 12:22:48 2009 +0000

description:
fix some security critical bugs:
-an invalid signal number passed to mq_notify(2) could crash the kernel
 on delivery -- add a boundary check
-mq_receive(2) from an empty queue crashed the kernel by NULL dereference
 in timeout calculation -- handle the NULL case
-likewise for mq_send(2) to a full queue
-a user could set mq_maxmsg (the maximal number of messages in a queue)
 to a huge value on mq_open(O_CREAT) and later use up all kernel
 memory by mq_send(2) -- add a sysctl'able limit which defaults
 to 16*mq_def_maxmsg

(mq_notify(2) should get some more checks, and SIGEV_* values other
than SIGEV_SIGNAL should be handled somehow, but this doesn't look
security critical)

diffstat:

 sys/kern/sys_mqueue.c |  41 +++++++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 12 deletions(-)

diffs (104 lines):

diff -r def750cffc47 -r a5b1093db7da sys/kern/sys_mqueue.c
--- a/sys/kern/sys_mqueue.c     Thu Dec 10 07:57:02 2009 +0000
+++ b/sys/kern/sys_mqueue.c     Thu Dec 10 12:22:48 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sys_mqueue.c,v 1.27 2009/12/09 21:32:59 dsl Exp $      */
+/*     $NetBSD: sys_mqueue.c,v 1.28 2009/12/10 12:22:48 drochner Exp $ */
 
 /*
  * Copyright (c) 2007-2009 Mindaugas Rasiukevicius <rmind at NetBSD org>
@@ -42,7 +42,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.27 2009/12/09 21:32:59 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.28 2009/12/10 12:22:48 drochner Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -82,6 +82,7 @@
 static u_int                   mq_prio_max = MQ_PRIO_MAX;
 static u_int                   mq_max_msgsize = 16 * MQ_DEF_MSGSIZE;
 static u_int                   mq_def_maxmsg = 32;
+static u_int                   mq_max_maxmsg = 16 * 32;
 
 static kmutex_t                        mqlist_mtx;
 static pool_cache_t            mqmsg_cache;
@@ -447,7 +448,9 @@
                                kmem_free(name, MQ_NAMELEN);
                                return error;
                        }
-                       if (attr.mq_maxmsg <= 0 || attr.mq_msgsize <= 0 ||
+                       if (attr.mq_maxmsg <= 0 ||
+                           attr.mq_maxmsg > mq_max_maxmsg ||
+                           attr.mq_msgsize <= 0 ||
                            attr.mq_msgsize > mq_max_msgsize) {
                                kmem_free(name, MQ_NAMELEN);
                                return EINVAL;
@@ -630,10 +633,12 @@
                        error = EAGAIN;
                        goto error;
                }
-               error = abstimeout2timo(ts, &t);
-               if (error) {
-                       goto error;
-               }
+               if (ts) {
+                       error = abstimeout2timo(ts, &t);
+                       if (error)
+                               goto error;
+               } else
+                       t = 0;
                /*
                 * Block until someone sends the message.
                 * While doing this, notification should not be sent.
@@ -815,10 +820,12 @@
                        error = EAGAIN;
                        goto error;
                }
-               error = abstimeout2timo(ts, &t);
-               if (error) {
-                       goto error;
-               }
+               if (ts) {
+                       error = abstimeout2timo(ts, &t);
+                       if (error)
+                               goto error;
+               } else
+                       t = 0;
                /* Block until queue becomes available */
                error = cv_timedwait_sig(&mq->mq_recv_cv, &mq->mq_mtx, t);
                if (error || (mqattr->mq_flags & MQ_UNLINK)) {
@@ -844,7 +851,8 @@
 
        /* Check for the notify */
        if (mqattr->mq_curmsgs == 0 && mq->mq_notify_proc &&
-           (mqattr->mq_flags & MQ_RECEIVE) == 0) {
+           (mqattr->mq_flags & MQ_RECEIVE) == 0 &&
+           mq->mq_sig_notify.sigev_notify == SIGEV_SIGNAL) {
                /* Initialize the signal */
                KSI_INIT(&ksi);
                ksi.ksi_signo = mq->mq_sig_notify.sigev_signo;
@@ -938,6 +946,9 @@
                    sizeof(struct sigevent));
                if (error)
                        return error;
+               if (sig.sigev_notify == SIGEV_SIGNAL &&
+                   (sig.sigev_signo <=0 || sig.sigev_signo >= NSIG))
+                       return EINVAL;
        }
 
        error = mqueue_get(SCARG(uap, mqdes), &fp);
@@ -1161,6 +1172,12 @@
                SYSCTL_DESCR("Default maximal message count"),
                NULL, 0, &mq_def_maxmsg, 0,
                CTL_CREATE, CTL_EOL);
+       sysctl_createv(clog, 0, &node, NULL,
+               CTLFLAG_PERMANENT | CTLFLAG_READWRITE,
+               CTLTYPE_INT, "mq_max_maxmsg",
+               SYSCTL_DESCR("Maximal allowed message count"),
+               NULL, 0, &mq_max_maxmsg, 0,
+               CTL_CREATE, CTL_EOL);
 }
 
 /*



Home | Main Index | Thread Index | Old Index