AUDIT_AVC_PATH problems
Subsystems (2)
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?
include/linux/audit.h +0 -2
@@ -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; })
kernel/auditsc.c +0 -47
71 lines not on this page · raw message
security/selinux/avc.c +8 -7
@@ -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)
Stephen Smalley
Acked-byQuoted 138 lines · Alexander Viro
Selinux folks had been complaining about the lack of
AVC_PATH
records when audit is disabled. I must admit my stupidity - I assumed thatavc_audit()really couldn't useaudit_log_d_path()because of deadlocks (== could be called withdcache_lockorvfsmount_lockheld). 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_PATHout 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?
include/linux/audit.h +0
-2@@ -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; })kernel/auditsc.c +0
-47@@ -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 *tsaudit_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 security/selinux/avc.c +6
-4@@ -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 :
security/selinux/avc.c void avc_audit(u32 ssid, u32 tsid, · line 633
Quoted 10 lines
security/selinux/avc.c +2
-3u =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)
Signature · 2 lines
Stephen Smalley National Security Agency