Module Name: src
Committed By: rmind
Date: Tue May 26 00:39:14 UTC 2009
Modified Files:
src/sys/kern: sys_mqueue.c
Log Message:
- Slightly rework the way permissions are checked. Neither mq_receive() not
mq_send() should fail due to permissions. Noted by Stathis Kamperis!
- Check for empty message queue name (POSIX does not allow this for regular
files, and it's weird), check for DTYPE_MQUEUE, fix permission check in
mq_unlink(), clean up.
To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/kern/sys_mqueue.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/sys_mqueue.c
diff -u src/sys/kern/sys_mqueue.c:1.17 src/sys/kern/sys_mqueue.c:1.18
--- src/sys/kern/sys_mqueue.c:1.17 Sat May 16 23:58:09 2009
+++ src/sys/kern/sys_mqueue.c Tue May 26 00:39:14 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: sys_mqueue.c,v 1.17 2009/05/16 23:58:09 rmind Exp $ */
+/* $NetBSD: sys_mqueue.c,v 1.18 2009/05/26 00:39:14 rmind Exp $ */
/*
* Copyright (c) 2007, 2008 Mindaugas Rasiukevicius <rmind at NetBSD org>
@@ -42,7 +42,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.17 2009/05/16 23:58:09 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.18 2009/05/26 00:39:14 rmind Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -87,8 +87,6 @@
static int mq_stat_fop(file_t *, struct stat *);
static int mq_close_fop(file_t *);
-#define FNOVAL -1
-
static const struct fileops mqops = {
.fo_read = fbadop_read,
.fo_write = fbadop_write,
@@ -167,57 +165,28 @@
}
/*
- * Check access against message queue.
- */
-static inline int
-mqueue_access(struct lwp *l, struct mqueue *mq, int access)
-{
- mode_t acc_mode = 0;
-
- KASSERT(mutex_owned(&mq->mq_mtx));
- KASSERT(access != FNOVAL);
-
- /* Note the difference between VREAD/VWRITE and FREAD/FWRITE */
- if (access & FREAD)
- acc_mode |= VREAD;
- if (access & FWRITE)
- acc_mode |= VWRITE;
-
- return vaccess(VNON, mq->mq_mode, mq->mq_euid, mq->mq_egid,
- acc_mode, l->l_cred);
-}
-
-/*
- * Get the mqueue from the descriptor.
- * => locks the message queue, if found
- * => increments the reference on file entry
+ * mqueue_get: get the mqueue from the descriptor.
+ * => locks the message queue, if found.
+ * => holds a reference on the file descriptor.
*/
static int
-mqueue_get(struct lwp *l, mqd_t mqd, int access, file_t **fpr)
+mqueue_get(mqd_t mqd, file_t **fpr)
{
- file_t *fp;
struct mqueue *mq;
+ file_t *fp;
- /* Get the file and descriptor */
fp = fd_getfile((int)mqd);
- if (fp == NULL)
+ if (__predict_false(fp == NULL)) {
return EBADF;
-
- /* Increment the reference of file entry, and lock the mqueue */
- mq = fp->f_data;
- *fpr = fp;
- mutex_enter(&mq->mq_mtx);
- if (access == FNOVAL) {
- KASSERT(mutex_owned(&mq->mq_mtx));
- return 0;
}
-
- /* Check the access mode and permission */
- if ((fp->f_flag & access) != access || mqueue_access(l, mq, access)) {
- mutex_exit(&mq->mq_mtx);
+ if (__predict_false(fp->f_type != DTYPE_MQUEUE)) {
fd_putfile((int)mqd);
- return EPERM;
+ return EBADF;
}
+ mq = fp->f_data;
+ mutex_enter(&mq->mq_mtx);
+
+ *fpr = fp;
return 0;
}
@@ -364,6 +333,12 @@
return EMFILE;
}
+ /* Empty name is invalid */
+ if (name[0] == '\0') {
+ kmem_free(name, MQ_NAMELEN);
+ return EINVAL;
+ }
+
/* Check for mqueue attributes */
if (SCARG(uap, attr)) {
error = copyin(SCARG(uap, attr), &attr,
@@ -400,7 +375,9 @@
strlcpy(mq_new->mq_name, name, MQ_NAMELEN);
memcpy(&mq_new->mq_attrib, &attr, sizeof(struct mq_attr));
- mq_new->mq_attrib.mq_flags = oflag;
+
+ CTASSERT((O_MASK & (MQ_UNLINK | MQ_RECEIVE)) == 0);
+ mq_new->mq_attrib.mq_flags = (O_MASK & oflag);
/* Store mode and effective UID with GID */
mq_new->mq_mode = ((SCARG(uap, mode) &
@@ -425,6 +402,8 @@
mutex_enter(&mqlist_mtx);
mq = mqueue_lookup(name);
if (mq) {
+ mode_t acc_mode;
+
KASSERT(mutex_owned(&mq->mq_mtx));
/* Check if mqueue is not marked as unlinking */
@@ -437,8 +416,20 @@
error = EEXIST;
goto exit;
}
- /* Check the permission */
- if (mqueue_access(l, mq, fp->f_flag)) {
+
+ /*
+ * Check the permissions. Note the difference between
+ * VREAD/VWRITE and FREAD/FWRITE.
+ */
+ acc_mode = 0;
+ if (fp->f_flag & FREAD) {
+ acc_mode |= VREAD;
+ }
+ if (fp->f_flag & FWRITE) {
+ acc_mode |= VWRITE;
+ }
+ if (vaccess(VNON, mq->mq_mode, mq->mq_euid, mq->mq_egid,
+ acc_mode, l->l_cred)) {
error = EACCES;
goto exit;
}
@@ -509,7 +500,7 @@
int error;
/* Get the message queue */
- error = mqueue_get(l, mqdes, FREAD, &fp);
+ error = mqueue_get(mqdes, &fp);
if (error)
return error;
mq = fp->f_data;
@@ -670,7 +661,7 @@
msg->msg_prio = msg_prio;
/* Get the mqueue */
- error = mqueue_get(l, mqdes, FWRITE, &fp);
+ error = mqueue_get(mqdes, &fp);
if (error) {
mqueue_freemsg(msg, size);
return error;
@@ -814,7 +805,7 @@
return error;
}
- error = mqueue_get(l, SCARG(uap, mqdes), FNOVAL, &fp);
+ error = mqueue_get(SCARG(uap, mqdes), &fp);
if (error)
return error;
mq = fp->f_data;
@@ -853,7 +844,7 @@
int error;
/* Get the message queue */
- error = mqueue_get(l, SCARG(uap, mqdes), FNOVAL, &fp);
+ error = mqueue_get(SCARG(uap, mqdes), &fp);
if (error)
return error;
mq = fp->f_data;
@@ -884,7 +875,7 @@
nonblock = (attr.mq_flags & O_NONBLOCK);
/* Get the message queue */
- error = mqueue_get(l, SCARG(uap, mqdes), FNOVAL, &fp);
+ error = mqueue_get(SCARG(uap, mqdes), &fp);
if (error)
return error;
mq = fp->f_data;
@@ -942,7 +933,8 @@
}
/* Check the permissions */
- if (mqueue_access(l, mq, FWRITE)) {
+ if (kauth_cred_geteuid(l->l_cred) != mq->mq_euid &&
+ kauth_authorize_generic(l->l_cred, KAUTH_GENERIC_ISSUSER, NULL)) {
mutex_exit(&mq->mq_mtx);
error = EACCES;
goto error;
