[PATCH] Replacement of IPC char* context to sid for improved performance

On Thu, 2006-03-30 at 22:01 -0600, Dustin Kirkland wrote: This replaces the string contexts for IPC label collection with u32 sid's, and translates those sid's to context strings at audit_log_exit() time. Steve Grubb's patch for a similar replacement for inode objects was used as a framework for this patch. This patch applies to Al Viro's audit-current.git tree (lspp.b4 branch), after the following two prereq patches have been applied: - My rework of the ipc audit code (not yet merged to Al's tree, though no outstanding negative comments on list...(?)): http://www.redhat.com/archives/linux-audit/2006-March/msg00088.html - Steve Grubb's inode patch: http://www.redhat.com/archives/linux-audit/2006-March/msg00285.html A few notes... - The references to [security|selinux]_ipc_getsecurity have been eliminated, and therefore I removed these definitions in the SELinux code. I could conceivably move that to a separate patch (?) or it can remain in the kernel. As we have a cleaner solution to the overall problem and there are no remaining callers, I think these should be removed entirely. - Steve's patch is required as I call selinux_ctxid_to_string() which his patch added. - Otherwise, this is a pretty straight duplication of Steve's approach to inode context/sid auditing. I would expect the same criticisms to apply. - This code compiles cleanly, though I was not able to run my built kernel due to a seemingly unrelated error that surfaced during 'make install': sh /local/work/kernel/audit/arch/i386/boot/install.sh 2.6.16 arch/i386/boot/bzImage System.map "/boot" WARNING: /lib/modules/2.6.16/kernel/fs/reiserfs/reiserfs.ko needs unknown symbol generic_file_splice_read WARNING: /lib/modules/2.6.16/kernel/fs/reiserfs/reiserfs.ko needs unknown symbol generic_file_splice_write WARNING: /lib/modules/2.6.16/kernel/fs/ext3/ext3.ko needs unknown symbol generic_file_splice_read WARNING: /lib/modules/2.6.16/kernel/fs/ext3/ext3.ko needs unknown symbol generic_file_splice_write Steve, if you could (a) take a close look at this patch and ensure that I'm closely following your model, and (b) build this patch and run similar performance benchmarks, that would be great. I'm traveling tomorrow and will be out of pocket for the next few days. :-Dustin -------- diff --git a/include/linux/security.h b/include/linux/security.h index aaa0a5c..1bab48f 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -869,11 +869,6 @@ struct swap_info_struct; * @ipcp contains the kernel IPC permission structure * @flag contains the desired (requested) permission set * Return 0 if permission is granted. - * @ipc_getsecurity: - * Copy the security label associated with the ipc object into - * @buffer. @buffer may be NULL to request the size of the buffer - * required. @size indicates the size of @buffer in bytes. Return - * number of bytes used/required on success. * * Security hooks for individual messages held in System V IPC message queues * @msg_msg_alloc_security: @@ -1223,7 +1218,6 @@ struct security_operations { void (*task_to_inode)(struct task_struct *p, struct inode *inode); int (*ipc_permission) (struct kern_ipc_perm * ipcp, short flag); - int (*ipc_getsecurity)(struct kern_ipc_perm *ipcp, void *buffer, size_t size); int (*msg_msg_alloc_security) (struct msg_msg * msg); void (*msg_msg_free_security) (struct msg_msg * msg); @@ -1887,11 +1881,6 @@ static inline int security_ipc_permissio return security_ops->ipc_permission (ipcp, flag); } -static inline int security_ipc_getsecurity(struct kern_ipc_perm *ipcp, void *buffer, size_t size) -{ - return security_ops->ipc_getsecurity(ipcp, buffer, size); -} - static inline int security_msg_msg_alloc (struct msg_msg * msg) { return security_ops->msg_msg_alloc_security (msg); @@ -2532,11 +2521,6 @@ static inline int security_ipc_permissio return 0; } -static inline int security_ipc_getsecurity(struct kern_ipc_perm *ipcp, void *buffer, size_t size) -{ - return -EOPNOTSUPP; -} - static inline int security_msg_msg_alloc (struct msg_msg * msg) { return 0; diff --git a/include/linux/selinux.h b/include/linux/selinux.h index 84a6c74..d5a9055 100644 --- a/include/linux/selinux.h +++ b/include/linux/selinux.h @@ -98,6 +98,15 @@ int selinux_ctxid_to_string(u32 ctxid, c */ void selinux_get_inode_sid(const struct inode *inode, u32 *sid); +/** + * selinux_get_ipc_sid - get the ipc obj's security context ID + * @ipcp: ipc structure to get the sid from. + * @sid: pointer to security context ID to be filled in. + * + * Returns nothing + */ +void selinux_get_ipc_sid(const struct kern_ipc_perm *ipcp, u32 *sid); + #else static inline int selinux_audit_rule_init(u32 field, u32 op, @@ -141,6 +150,11 @@ static inline void selinux_get_inode_sid *sid = 0; } +static inline void selinux_get_ipc_sid(const struct kern_ipc_perm *ipcp, u32 *sid) +{ + *sid = 0; +} + #endif /* CONFIG_SECURITY_SELINUX */ #endif /* _LINUX_SELINUX_H */ diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 27631cd..e0f592a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -107,7 +107,7 @@ struct audit_aux_data_ipcctl { uid_t uid; gid_t gid; mode_t mode; - char *ctx; + u32 osid; }; struct audit_aux_data_socketcall { @@ -432,12 +432,6 @@ static inline void audit_free_aux(struct dput(axi->dentry); mntput(axi->mnt); } - if ( aux->type == AUDIT_IPC || - aux->type == AUDIT_IPC_NEW_PERM ) { - struct audit_aux_data_ipcctl *axi = (void *)aux; - kfree(axi->ctx); - } - context->aux = aux->next; kfree(aux); } @@ -633,11 +627,25 @@ static void audit_log_exit(struct audit_ switch (aux->type) { case AUDIT_IPC: { - struct audit_aux_data_ipcctl *axi = (void *)aux; - audit_log_format(ab, - " iuid=%u igid=%u mode=%x obj=%s", - axi->uid, axi->gid, axi->mode, axi->ctx); - break; } + case AUDIT_IPC: { + struct audit_aux_data_ipcctl *axi = (void *)aux; + audit_log_format(ab, + " qbytes=%lx iuid=%u igid=%u mode=%x", + axi->qbytes, axi->uid, axi->gid, axi->mode); + if (axi->osid != 0) { + char *ctx = NULL; + int len; + if (selinux_ctxid_to_string( + axi->osid, &ctx, &len)) { + audit_log_format(ab, " obj=%u", + axi->osid); + call_panic = 1; + } else { + audit_log_format(ab, " obj=%s", ctx); + } + kfree(ctx); + } + break; } case AUDIT_IPC_NEW_PERM: { struct audit_aux_data_ipcctl *axi = (void *)aux; @@ -1148,36 +1156,9 @@ uid_t audit_get_loginuid(struct audit_co return ctx ? ctx->loginuid : -1; } -static char *audit_ipc_context(struct kern_ipc_perm *ipcp) +void audit_ipc_context(struct kern_ipc_perm *ipcp, u32 *osid) { - struct audit_context *context = current->audit_context; - char *ctx = NULL; - int len = 0; - - if (likely(!context)) - return NULL; - - len = security_ipc_getsecurity(ipcp, NULL, 0); - if (len == -EOPNOTSUPP) - goto ret; - if (len < 0) - goto error_path; - - ctx = kmalloc(len, GFP_ATOMIC); - if (!ctx) - goto error_path; - - len = security_ipc_getsecurity(ipcp, ctx, len); - if (len < 0) - goto error_path; - - return ctx; - -error_path: - kfree(ctx); - audit_panic("error in audit_ipc_context"); -ret: - return NULL; + selinux_get_ipc_sid(ipcp, osid); } /** @@ -1190,6 +1171,7 @@ int audit_ipc_obj(struct kern_ipc_perm * { struct audit_aux_data_ipcctl *ax; struct audit_context *context = current->audit_context; + u32 osid; if (likely(!context)) return 0; @@ -1201,7 +1183,8 @@ int audit_ipc_obj(struct kern_ipc_perm * ax->uid = ipcp->uid; ax->gid = ipcp->gid; ax->mode = ipcp->mode; - ax->ctx = audit_ipc_context(ipcp); + audit_ipc_context(ipcp, &osid); + ax->osid = osid; ax->d.type = AUDIT_IPC; ax->d.next = context->aux; diff --git a/security/dummy.c b/security/dummy.c index fd99429..8cccccc 100644 --- a/security/dummy.c +++ b/security/dummy.c @@ -563,11 +563,6 @@ static int dummy_ipc_permission (struct return 0; } -static int dummy_ipc_getsecurity(struct kern_ipc_perm *ipcp, void *buffer, size_t size) -{ - return -EOPNOTSUPP; -} - static int dummy_msg_msg_alloc_security (struct msg_msg *msg) { return 0; @@ -976,7 +971,6 @@ void security_fixup_ops (struct security set_to_dummy_if_null(ops, task_reparent_to_init); set_to_dummy_if_null(ops, task_to_inode); set_to_dummy_if_null(ops, ipc_permission); - set_to_dummy_if_null(ops, ipc_getsecurity); set_to_dummy_if_null(ops, msg_msg_alloc_security); set_to_dummy_if_null(ops, msg_msg_free_security); set_to_dummy_if_null(ops, msg_queue_alloc_security); diff --git a/security/selinux/exports.c b/security/selinux/exports.c index 07ddce7..626ba36 100644 --- a/security/selinux/exports.c +++ b/security/selinux/exports.c @@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/selinux.h> #include <linux/fs.h> +#include <linux/ipc.h> #include "security.h" #include "objsec.h" @@ -50,3 +51,12 @@ void selinux_get_inode_sid(const struct *sid = 0; } +void selinux_get_ipc_sid(const struct kern_ipc_perm *ipcp, u32 *sid) +{ + if (selinux_enabled) { + struct ipc_security_struct *isec = ipcp->security; + *sid = isec->sid; + return; + } + *sid = 0; +} diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b61b955..3cf368a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4052,13 +4052,6 @@ static int selinux_ipc_permission(struct return ipc_has_perm(ipcp, av); } -static int selinux_ipc_getsecurity(struct kern_ipc_perm *ipcp, void *buffer, size_t size) -{ - struct ipc_security_struct *isec = ipcp->security; - - return selinux_getsecurity(isec->sid, buffer, size); -} - /* module stacking operations */ static int selinux_register_security (const char *name, struct security_operations *ops) { @@ -4321,7 +4314,6 @@ static struct security_operations selinu .task_to_inode = selinux_task_to_inode, .ipc_permission = selinux_ipc_permission, - .ipc_getsecurity = selinux_ipc_getsecurity, .msg_msg_alloc_security = selinux_msg_msg_alloc_security, .msg_msg_free_security = selinux_msg_msg_free_security,