Re: AUDIT_AVC_PATH problems

Selinux folks had been complaining about the lack of AVC_PATH records when audit is disabled. I must admit my stupidity - I assumed that avc_audit() really couldn't use audit_log_d_path() because of deadlocks (== could be called with dcache_lock or vfsmount_lock held). Shouldn't have made that assumption - it never gets called that way. It _is_ called under spinlocks, but not those. Since audit_log_d_path() uses ab->gfp_mask for allocations, kmalloc() in there is not a problem. IOW, the simple fix should be sufficient: let's rip AUDIT_AVC_PATH out and simply generate pathname as part of main record. It's trivial to do. Patch below is against the mainline, rhel5 variant is also trivial. Comments? diff --git a/include/linux/audit.h b/include/linux/audit.h index fccc6e5..f3d6e5f 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -399,7 +399,6 @@ extern int audit_bprm(struct linux_binprm *bprm); extern int audit_socketcall(int nargs, unsigned long *args); extern int audit_sockaddr(int len, void *addr); extern int __audit_fd_pair(int fd1, int fd2); -extern int audit_avc_path(struct dentry *dentry, struct vfsmount *mnt); extern int audit_set_macxattr(const char *name); 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); @@ -479,7 +478,6 @@ extern int audit_signals; #define audit_socketcall(n,a) ({ 0; }) #define audit_fd_pair(n,a) ({ 0; }) #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; }) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index e36481e..10e0e6e 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -176,12 +176,6 @@ struct audit_aux_data_fd_pair { int fd[2]; }; -struct audit_aux_data_path { - struct audit_aux_data d; - struct dentry *dentry; - struct vfsmount *mnt; -}; - struct audit_aux_data_pids { struct audit_aux_data d; pid_t target_pid[AUDIT_AUX_PIDS]; @@ -657,12 +651,6 @@ static inline void audit_free_aux(struct audit_context *context) struct audit_aux_data *aux; while ((aux = context->aux)) { - if (aux->type == AUDIT_AVC_PATH) { - struct audit_aux_data_path *axi = (void *)aux; - dput(axi->dentry); - mntput(axi->mnt); - } - context->aux = aux->next; kfree(aux); } @@ -998,11 +986,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts audit_log_hex(ab, axs->a, axs->len); break; } - case AUDIT_AVC_PATH: { - struct audit_aux_data_path *axi = (void *)aux; - audit_log_d_path(ab, "path=", axi->dentry, axi->mnt); - break; } - case AUDIT_FD_PAIR: { struct audit_aux_data_fd_pair *axs = (void *)aux; audit_log_format(ab, "fd0=%d fd1=%d", axs->fd[0], axs->fd[1]); @@ -1952,36 +1935,6 @@ void __audit_ptrace(struct task_struct *t) } /** - * audit_avc_path - record the granting or denial of permissions - * @dentry: dentry to record - * @mnt: mnt to record - * - * Returns 0 for success or NULL context or < 0 on error. - * - * Called from security/selinux/avc.c::avc_audit() - */ -int audit_avc_path(struct dentry *dentry, struct vfsmount *mnt) -{ - struct audit_aux_data_path *ax; - struct audit_context *context = current->audit_context; - - if (likely(!context)) - return 0; - - ax = kmalloc(sizeof(*ax), GFP_ATOMIC); - if (!ax) - return -ENOMEM; - - ax->dentry = dget(dentry); - ax->mnt = mntget(mnt); - - ax->d.type = AUDIT_AVC_PATH; - ax->d.next = context->aux; - context->aux = (void *)ax; - return 0; -} - -/** * audit_signal_info - record signal info for shutting down audit subsystem * @sig: signal value * @t: task being signaled diff --git a/security/selinux/avc.c b/security/selinux/avc.c index e4396a8..2f4c2bb 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -570,10 +570,12 @@ void avc_audit(u32 ssid, u32 tsid, case AVC_AUDIT_DATA_FS: if (a->u.fs.dentry) { struct dentry *dentry = a->u.fs.dentry; - if (a->u.fs.mnt) - audit_avc_path(dentry, a->u.fs.mnt); - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, dentry->d_name.name); + if (a->u.fs.mnt) { + audit_log_d_path(ab, "path=", dentry, a->u.fs.mnt); + } else { + audit_log_format(ab, " name="); + audit_log_untrustedstring(ab, dentry->d_name.name); + } inode = dentry->d_inode; } else if (a->u.fs.inode) { struct dentry *dentry; @@ -624,9 +626,8 @@ void avc_audit(u32 ssid, u32 tsid, case AF_UNIX: u = unix_sk(sk); if (u->dentry) { - audit_avc_path(u->dentry, u->mnt); - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, u->dentry->d_name.name); + audit_log_d_path(ab, "path=", + u->dentry, u->mnt); break; } if (!u->addr)