On Tuesday 16 May 2006 21:40, George C. Wilson wrote: Do we really need the header or just a forward declaration for mq structs? Do we really need 4 message types? IPC was able to get by on 2 of them. Are we recording more than IPC? Is this additional space really needed? what if u_mqstat==NULL and the fd was bad upon syscall entry? All the data structures are whatever the stack contained, and therefore completely bogus. I think there are a couple ways that the structs could be uninitialized. Do we really need to record the size? Is there anything here that's not sec relevant? The same comment applies to the next 3 message types. What if audit is not enabled? Need to check for it and bail out. memory leak from above allocation. audit_enabled? memleak audit_enabled? memleak memleak audit_enabled? memleak audit_enabled? -Steve

Hey George, Just some quick things. I didn't want to reproduce any of the things Steve pointed out already... These are just general comments... -tim On Tue, 2006-05-16 at 20:40 -0500, George C. Wilson wrote: <snip> Take the assignment out of the dang conditional :) Besides, you're over 80 characters here aren't ya? Also, you should probably just follow the same convention in the function.. ret = audit_mg_timedsend(..); if (!ret) goto out; [..] Eh, get rid of these... [..] At a cursory glance, this looks a little fishy to me... <snip>

This patch adds audit support to POSIX message queues. It applies cleanly to the lspp.b15 branch of Al Viro's git tree. There are new auxiliary data structures, and collection and emission routines in kernel/auditsc.c. New hooks in ipc/mqueue.c collect arguments from the syscalls. I tested the patch by building the examples from the POSIX MQ library tarball. Build them -lrt, not against the old MQ library in the tarball. Here's the URL: http://www.geocities.com/wronski12/posix_ipc/libmqueue-4.41.tar.gz Do auditctl -a exit,always -S for mq_open, mq_timedsend, mq_timedreceive, mq_notify, mq_getsetattr. mq_unlink has no new hooks. Please see the corresponding userspace patch to get correct output from auditd for the new record types. Notes: structs? No. Forward declaration added, #include removed. I don't see an easy easy way around this. The parameters are different for each of the calls. It is possible to emit empty fields or conditionalize the output if we want to go that route. No. Removed. Got rid of omqstat, moved collection inside the null check, got rid of audret. Yes, was doing much unnecessary work in the disabled case. Added checks for audit enabled. These do not appear to be leaks. They always get freed in do_syscall_trace_leave(), both when the syscalls return -EFAULT and when audit is disabled. I verified this with printk's. With the check for audit enabled, they no longer get allocated needlessly. Unless a syscall can bypass do_syscall_trace_leave(), these look like they don't leak. Relative to LSPP, no. But if someone is trying to DoS the machine, it may be helpful. These are very good questions. I'm happy to pare it down if consensus is this is too much data. Sage advice taken. Looked fishy to me, too, which I had pointed out in my comments. Addressed by 4th item above. The dummy function prototypes were grossly wrong. Corrected these. This code now builds with audit turned off in the .config. I had to slightly modify the base lspp.b12 code as it did not build with audit compiled out. Looks like this is OK in lspp.b15, though. include/linux/audit.h | 17 +++ ipc/mqueue.c | 22 ++++ kernel/auditsc.c | 264 +++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 302 insertions(+), 1 deletion(-) Signed-off-by: George Wilson <ltcgcw(a)us.ibm.com&gt; -- diff --git a/include/linux/audit.h b/include/linux/audit.h index d8101d3..aa93091 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -85,6 +85,10 @@ #define AUDIT_CWD 1307 /* Current working directory */ #define AUDIT_EXECVE 1309 /* execve arguments */ #define AUDIT_IPC_SET_PERM 1311 /* IPC new permissions record type */ +#define AUDIT_MQ_OPEN 1312 /* POSIX MQ open record type */ +#define AUDIT_MQ_SENDRECV 1313 /* POSIX MQ send/receive record type */ +#define AUDIT_MQ_NOTIFY 1314 /* POSIX MQ notify record type */ +#define AUDIT_MQ_GETSETATTR 1315 /* POSIX MQ get/set attribute record type */ #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ @@ -288,6 +292,8 @@ struct audit_context; struct inode; struct netlink_skb_parms; struct linux_binprm; +struct mq_attr; +struct mqstat; #define AUDITSC_INVALID 0 #define AUDITSC_SUCCESS 1 @@ -350,6 +356,12 @@ static inline int audit_ipc_set_perm(uns return __audit_ipc_set_perm(qbytes, uid, gid, mode); return 0; } + +extern int audit_mq_open(int oflag, mode_t mode, struct mq_attr __user *u_attr); +extern int audit_mq_timedsend(mqd_t mqdes, size_t msg_len, unsigned int msg_prio, const struct timespec __user *u_abs_timeout); +extern int audit_mq_timedreceive(mqd_t mqdes, size_t msg_len, unsigned int __user *u_msg_prio, const struct timespec __user *u_abs_timeout); +extern int audit_mq_notify(mqd_t mqdes, const struct sigevent __user *u_notification); +extern int audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat); #else #define audit_alloc(t) ({ 0; }) #define audit_free(t) do { ; } while (0) @@ -370,6 +382,11 @@ static inline int audit_ipc_set_perm(uns #define audit_sockaddr(len, addr) ({ 0; }) #define audit_avc_path(dentry, mnt) ({ 0; }) #define audit_set_macxattr(n) do { ; } while (0) +#define audit_mq_open(o,m,a) ({ 0; }) +#define audit_mq_timedsend(d,l,p,t) ({ 0; }) +#define audit_mq_timedreceive(d,l,p,t) ({ 0; }) +#define audit_mq_notify(d,n) ({ 0; }) +#define audit_mq_getsetattr(d,s) ({ 0; }) #endif #ifdef CONFIG_AUDIT diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 41ecbd4..1511714 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -8,6 +8,8 @@ * Lockless receive & send, fd based notify: * Manfred Spraul (manfred(a)colorfullife.com) * + * Audit: George Wilson (ltcgcw(a)us.ibm.com) + * * This file is released under the GPL. */ @@ -24,6 +26,7 @@ #include <linux/skbuff.h> #include <linux/netlink.h> #include <linux/syscalls.h> +#include <linux/audit.h> #include <linux/signal.h> #include <linux/mutex.h> @@ -657,6 +660,10 @@ asmlinkage long sys_mq_open(const char _ char *name; int fd, error; + error = audit_mq_open(oflag, mode, u_attr); + if (error != 0) + return error; + if (IS_ERR(name = getname(u_name))) return PTR_ERR(name); @@ -814,6 +821,10 @@ asmlinkage long sys_mq_timedsend(mqd_t m long timeout; int ret; + ret = audit_mq_timedsend(mqdes, msg_len, msg_prio, u_abs_timeout); + if (ret != 0) + return ret; + if (unlikely(msg_prio >= (unsigned long) MQ_PRIO_MAX)) return -EINVAL; @@ -896,6 +907,10 @@ asmlinkage ssize_t sys_mq_timedreceive(m struct mqueue_inode_info *info; struct ext_wait_queue wait; + ret = audit_mq_timedreceive(mqdes, msg_len, u_msg_prio, u_abs_timeout); + if (ret != 0) + return ret; + timeout = prepare_timeout(u_abs_timeout); ret = -EBADF; @@ -975,6 +990,10 @@ asmlinkage long sys_mq_notify(mqd_t mqde struct mqueue_inode_info *info; struct sk_buff *nc; + ret = audit_mq_notify(mqdes, u_notification); + if (ret != 0) + return ret; + nc = NULL; sock = NULL; if (u_notification != NULL) { @@ -1115,6 +1134,9 @@ asmlinkage long sys_mq_getsetattr(mqd_t omqstat = info->attr; omqstat.mq_flags = filp->f_flags & O_NONBLOCK; if (u_mqstat) { + ret = audit_mq_getsetattr(mqdes, &mqstat); + if (ret != 0) + goto out; if (mqstat.mq_flags & O_NONBLOCK) filp->f_flags |= O_NONBLOCK; else diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 0d1d9af..a7ab249 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -3,7 +3,7 @@ * * Copyright 2003-2004 Red Hat Inc., Durham, North Carolina. * Copyright 2005 Hewlett-Packard Development Company, L.P. - * Copyright (C) 2005 IBM Corporation + * Copyright (C) 2005, 2006 IBM Corporation * All Rights Reserved. * * This program is free software; you can redistribute it and/or modify @@ -29,6 +29,9 @@ * this file -- see entry.S) is based on a GPL'd patch written by * okir(a)suse.de and Copyright 2003 SuSE Linux AG. * + * POSIX message queue support added by George Wilson <ltcgcw(a)us.ibm.com&gt;, + * 2006. + * * The support of additional filter rules compares (>, <, >=, <=) was * added by Dustin Kirkland <dustin.kirkland(a)us.ibm.com&gt;, 2005. * @@ -49,6 +52,7 @@ #include <linux/module.h> #include <linux/mount.h> #include <linux/socket.h> +#include <linux/mqueue.h> #include <linux/audit.h> #include <linux/personality.h> #include <linux/time.h> @@ -102,6 +106,33 @@ struct audit_aux_data { #define AUDIT_AUX_IPCPERM 0 +struct audit_aux_data_mq_open { + struct audit_aux_data d; + int oflag; + mode_t mode; + struct mq_attr attr; +}; + +struct audit_aux_data_mq_sendrecv { + struct audit_aux_data d; + mqd_t mqdes; + size_t msg_len; + unsigned int msg_prio; + struct timespec abs_timeout; +}; + +struct audit_aux_data_mq_notify { + struct audit_aux_data d; + mqd_t mqdes; + struct sigevent notification; +}; + +struct audit_aux_data_mq_getsetattr { + struct audit_aux_data d; + mqd_t mqdes; + struct mq_attr mqstat; +}; + struct audit_aux_data_ipcctl { struct audit_aux_data d; struct ipc_perm p; @@ -708,6 +739,43 @@ static void audit_log_exit(struct audit_ continue; /* audit_panic has been called */ switch (aux->type) { + case AUDIT_MQ_OPEN: { + struct audit_aux_data_mq_open *axi = (void *)aux; + audit_log_format(ab, + "oflag=0x%x mode=%#o mq_flags=0x%lx mq_maxmsg=%ld " + "mq_msgsize=%ld mq_curmsgs=%ld", + axi->oflag, axi->mode, axi->attr.mq_flags, + axi->attr.mq_maxmsg, axi->attr.mq_msgsize, + axi->attr.mq_curmsgs); + break; } + + case AUDIT_MQ_SENDRECV: { + struct audit_aux_data_mq_sendrecv *axi = (void *)aux; + audit_log_format(ab, + "mqdes=%d msg_len=%ld msg_prio=%u " + "abs_timeout_sec=%ld abs_timeout_nsec=%ld", + axi->mqdes, axi->msg_len, axi->msg_prio, + axi->abs_timeout.tv_sec, axi->abs_timeout.tv_nsec); + break; } + + case AUDIT_MQ_NOTIFY: { + struct audit_aux_data_mq_notify *axi = (void *)aux; + audit_log_format(ab, + "mqdes=%d sigev_signo=%d", + axi->mqdes, + axi->notification.sigev_signo); + break; } + + case AUDIT_MQ_GETSETATTR: { + struct audit_aux_data_mq_getsetattr *axi = (void *)aux; + audit_log_format(ab, + "mqdes=%d mq_flags=0x%lx mq_maxmsg=%ld mq_msgsize=%ld " + "mq_curmsgs=%ld ", + axi->mqdes, + axi->mqstat.mq_flags, axi->mqstat.mq_maxmsg, + axi->mqstat.mq_msgsize, axi->mqstat.mq_curmsgs); + break; } + case AUDIT_IPC: { struct audit_aux_data_ipcctl *axi = (void *)aux; audit_log_format(ab, @@ -1230,6 +1298,200 @@ uid_t audit_get_loginuid(struct audit_co } /** + * audit_mq_open - record audit data for a POSIX MQ open + * @oflag: open flag + * @mode: mode bits + * @u_attr: queue attributes + * + * Returns 0 for success or NULL context or < 0 on error. + */ +int audit_mq_open(int oflag, mode_t mode, struct mq_attr __user *u_attr) +{ + struct audit_aux_data_mq_open *ax; + struct audit_context *context = current->audit_context; + + if (!audit_enabled) + return 0; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + if (u_attr != NULL) { + if (copy_from_user(&ax->attr, u_attr, sizeof(ax->attr))) + return -EFAULT; + } else + memset(&ax->attr, 0, sizeof(ax->attr)); + + ax->oflag = oflag; + ax->mode = mode; + + ax->d.type = AUDIT_MQ_OPEN; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** + * audit_mq_timedsend - record audit data for a POSIX MQ timed send + * @mqdes: MQ descriptor + * @msg_len: Message length + * @msg_prio: Message priority + * @abs_timeout: Message timeout in absolute time + * + * Returns 0 for success or NULL context or < 0 on error. + */ +int audit_mq_timedsend(mqd_t mqdes, size_t msg_len, unsigned int msg_prio, + const struct timespec __user *u_abs_timeout) +{ + struct audit_aux_data_mq_sendrecv *ax; + struct audit_context *context = current->audit_context; + + if (!audit_enabled) + return 0; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + if (u_abs_timeout != NULL) { + if (copy_from_user(&ax->abs_timeout, u_abs_timeout, sizeof(ax->abs_timeout))) + return -EFAULT; + } else + memset(&ax->abs_timeout, 0, sizeof(ax->abs_timeout)); + + ax->mqdes = mqdes; + ax->msg_len = msg_len; + ax->msg_prio = msg_prio; + + ax->d.type = AUDIT_MQ_SENDRECV; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** + * audit_mq_timedreceive - record audit data for a POSIX MQ timed receive + * @mqdes: MQ descriptor + * @msg_len: Message length + * @msg_prio: Message priority + * @abs_timeout: Message timeout in absolute time + * + * Returns 0 for success or NULL context or < 0 on error. + */ +int audit_mq_timedreceive(mqd_t mqdes, size_t msg_len, + unsigned int __user *u_msg_prio, + const struct timespec __user *u_abs_timeout) +{ + struct audit_aux_data_mq_sendrecv *ax; + struct audit_context *context = current->audit_context; + + if (!audit_enabled) + return 0; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + if (u_msg_prio != NULL) { + if (get_user(ax->msg_prio, u_msg_prio)) + return -EFAULT; + } else + ax->msg_prio = 0; + + if (u_abs_timeout != NULL) { + if (copy_from_user(&ax->abs_timeout, u_abs_timeout, sizeof(ax->abs_timeout))) + return -EFAULT; + } else + memset(&ax->abs_timeout, 0, sizeof(ax->abs_timeout)); + + ax->mqdes = mqdes; + ax->msg_len = msg_len; + + ax->d.type = AUDIT_MQ_SENDRECV; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** + * audit_mq_notify - record audit data for a POSIX MQ notify + * @mqdes: MQ descriptor + * @u_notification: Notification event + * + * Returns 0 for success or NULL context or < 0 on error. + */ + +int audit_mq_notify(mqd_t mqdes, const struct sigevent __user *u_notification) +{ + struct audit_aux_data_mq_notify *ax; + struct audit_context *context = current->audit_context; + + if (!audit_enabled) + return 0; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + if (u_notification != NULL) { + if (copy_from_user(&ax->notification, u_notification, sizeof(ax->notification))) + return -EFAULT; + } else + memset(&ax->notification, 0, sizeof(ax->notification)); + + ax->mqdes = mqdes; + + ax->d.type = AUDIT_MQ_NOTIFY; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** + * audit_mq_getsetattr - record audit data for a POSIX MQ get/set attribute + * @mqdes: MQ descriptor + * @mqstat: MQ flags + * + * Returns 0 for success or NULL context or < 0 on error. + */ +int audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat) +{ + struct audit_aux_data_mq_getsetattr *ax; + struct audit_context *context = current->audit_context; + + if (!audit_enabled) + return 0; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + ax->mqdes = mqdes; + ax->mqstat = *mqstat; + + ax->d.type = AUDIT_MQ_GETSETATTR; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** * audit_ipc_obj - record audit data for ipc object * @ipcp: ipc permissions * -- George Wilson <ltcgcw(a)us.ibm.com&gt; IBM Linux Technology Center

On Wed, May 24, 2006 at 04:09:55PM -0500, George C. Wilson wrote: Should be checking !context->in_syscall instead of !audit_enabled, please see https://www.redhat.com/archives/linux-audit/2006-May/msg00083.html Same applies to all the new audit_mq_* routines.