January 2017 - Linux-audit - Linux-Audit List Archives

by Bhagwat, Shriniketan Manjunath

Greetings, I was trying to reconfigure the Audit using SIGHUP and came across with below behavior of Audit. The audit version I am using is 2.3.6. Scenario 1: Starting auditd with active audispd plugin. 1. Activate the audispd plugin by setting active = yes in configuration file (example: /etc/audisp/plugins.d/syslog.conf) 2. Start the Auditd. Starting auditd will create the both Auditd and Audispd process. 3. Deactivate the audispd plugin by setting active = no in configuration file. 4. Reconfigure the Auditd by sending SIGHUP to Auditd (>> kill -SIGHUP <auditd_pid>). This reconfiguration stops the audispd process. 5. Activate the audispd plugin by setting active = yes in configuration file. 6. Reconfigure the Auditd by sending SIGHUP to Auditd. This should start the audispd process, however audispd process will not be started in this reconfiguration. Scenario2: Starting auditd with no active audispd plugin. 1. Start the Auditd. Starting auditd will create the only Auditd process. 2. Activate the audispd plugin by setting active = yes in configuration file. 3. Reconfigure the Auditd by sending SIGHUP to Auditd. This should start the audispd process, however in few cases audispd process will not be started in this reconfiguration. 4. Deactivate the audispd plugin by setting active = no in configuration file. 5. Reconfigure the Auditd by sending SIGHUP to Auditd. This reconfiguration stops the audispd process. 6. Activate the audispd plugin by setting active = yes in configuration file. 7. Reconfigure the Auditd by sending SIGHUP to Auditd. This should start the audispd process, however audispd process will not be started in this reconfiguration. As per the change log of audit version 2.3.2, below fix was made. Fix: In auditd, restart dispatcher on SIGHUP if it had previously exited I have analyzed Auditd code of version 2.3.6 and below is my observation. When the Auditd is started, it calls init_dispatcher() to start the Audispd. init_dispatcher() starts the Audispd and maintains its pid value in a global variable (auditd-dispatch.c). When the audispd is terminated, SIGCHLD handler of Auditd i.e child_handler() does the waitpid() to remove the child process from zombie state and calls dispatcher_reaped(), where this pid is set to zero. During reconfigure using SIGHUP, reconfigure_dispatcher() checks for this pid value. If pid is valid then, SIGHUP is sent to audispd otherwise init_dispatcher() is called to start the audispd. Auditd uses event loop ev_signal to track the child process (SIGCHLD). The event loop from libev also has child handler childcb() (in ev.c). This handler childcb() also does the waitpid(). When audispd terminates, libev's child handler is getting called first, waitpid() removes the audispd from zombie state. Then the control is passed to Auditd's child handler child_handler(). Since the audispd has been already removed from process stable, waitpid() call done in child_handler() will return ECHILD. Hence the dispatcher_reaped() is not getting called to make the internally maintained pid variable to zero. This results in subsequent SIGHUP set to Auditd end up in sending SIGHUP to non-existing audispd process. In case of step 3 of scenario 2: When the auditd is started without any active audispd plugin(step1), Auditd starts the audispd. However since there are no active plugins Audispd will get terminated. In this case shutdown_dispatcher() was called from dispatch_event() where pid value was set to zero. Hence the reconfiguration in step 3 started audispd process in my case. I have put traces in auditd code to validate the above behavior. This behavior is applicable for Audit version 2.6.4 as well. Is this behavior a known issue? Regards, Ketan

7 years, 11 months

2
6
0 / 0

audit logs to remote syslog?

by John Jasen

I'm currently using audisp with the syslog plugin to send audit logs off to a remote server for reduction and archiving, which for the most part, works reasonably well. I understand auditd has its own facility for sending to a remote auditd collector, but haven't played with it. I've also considered using rsyslog with an imfile directive for /var/log/audit/audit.log. I'm sure there are options I've not considered -- what are other folks doing?

7 years, 11 months

2
1
0 / 0

[GIT PULL] Audit fixes for v4.10 (#1)

by Paul Moore

Hi Linus, Two small fixes relating to audit's use of fsnotify. The first patch plugs a leak and the second fixes some lock shenanigans. The patches are small and I banged on this for an afternoon with our testsuite and didn't see anything odd; please consider this for the v4.10-rcX series. Thanks, -Paul --- The following changes since commit 533c7b69c764ad5febb3e716899f43a75564fcab: audit: use proper refcount locking on audit_sock (2016-12-14 13:06:04 -0500) are available in the git repository at: git://git.infradead.org/users/pcmoore/audit stable-4.10 for you to fetch changes up to be29d20f3f5db1f0b4e49a4f6eeedf840e2bf9b1: audit: Fix sleep in atomic (2017-01-03 15:56:38 -0500) ---------------------------------------------------------------- Jan Kara (2): fsnotify: Remove fsnotify_duplicate_mark() audit: Fix sleep in atomic fs/notify/mark.c | 12 ------------ include/linux/fsnotify_backend.h | 2 -- kernel/audit_tree.c | 18 ++++++++++++++---- 3 files changed, 14 insertions(+), 18 deletions(-) -- paul moore security @ redhat

7 years, 11 months

1
0
0 / 0

Re: [PATCH 05/22] audit: Fix sleep in atomic

by Paul Moore

On Thu, Dec 22, 2016 at 4:15 AM, Jan Kara <jack(a)suse.cz> wrote: > Audit tree code was happily adding new notification marks while holding > spinlocks. Since fsnotify_add_mark() acquires group->mark_mutex this can > lead to sleeping while holding a spinlock, deadlocks due to lock > inversion, and probably other fun. Fix the problem by acquiring > group->mark_mutex earlier. > > CC: Paul Moore <paul(a)paul-moore.com> > Signed-off-by: Jan Kara <jack(a)suse.cz> > --- > kernel/audit_tree.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) [SIDE NOTE: this patch explains your comments and my earlier concern about the locked/unlocked variants of fsnotify_add_mark() in untag_chunk()] Ouch. Thanks for catching this ... what is your goal with these patches, are you targeting this as a fix during the v4.10-rcX cycle? If not, any objections if I pull this patch into the audit tree and send this to Linus during the v4.10-rcX cycle (assuming it passes testing, yadda yadda)? > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > index f3130eb0a4bd..156b6a93f4fc 100644 > --- a/kernel/audit_tree.c > +++ b/kernel/audit_tree.c > @@ -231,6 +231,7 @@ static void untag_chunk(struct node *p) > if (size) > new = alloc_chunk(size); > > + mutex_lock(&entry->group->mark_mutex); > spin_lock(&entry->lock); > if (chunk->dead || !entry->inode) { > spin_unlock(&entry->lock); > @@ -258,7 +259,8 @@ static void untag_chunk(struct node *p) > if (!new) > goto Fallback; > > - if (fsnotify_add_mark(&new->mark, entry->group, entry->inode, NULL, 1)) { > + if (fsnotify_add_mark_locked(&new->mark, entry->group, entry->inode, > + NULL, 1)) { > fsnotify_put_mark(&new->mark); > goto Fallback; > } > @@ -309,6 +311,7 @@ static void untag_chunk(struct node *p) > spin_unlock(&hash_lock); > spin_unlock(&entry->lock); > out: > + mutex_unlock(&entry->group->mark_mutex); > fsnotify_put_mark(entry); > spin_lock(&hash_lock); > } > @@ -385,17 +388,21 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree) > > chunk_entry = &chunk->mark; > > + mutex_lock(&old_entry->group->mark_mutex); > spin_lock(&old_entry->lock); > if (!old_entry->inode) { > /* old_entry is being shot, lets just lie */ > spin_unlock(&old_entry->lock); > + mutex_unlock(&old_entry->group->mark_mutex); > fsnotify_put_mark(old_entry); > free_chunk(chunk); > return -ENOENT; > } > > - if (fsnotify_add_mark(chunk_entry, old_entry->group, old_entry->inode, NULL, 1)) { > + if (fsnotify_add_mark_locked(chunk_entry, old_entry->group, > + old_entry->inode, NULL, 1)) { > spin_unlock(&old_entry->lock); > + mutex_unlock(&old_entry->group->mark_mutex); > fsnotify_put_mark(chunk_entry); > fsnotify_put_mark(old_entry); > return -ENOSPC; > @@ -411,6 +418,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree) > chunk->dead = 1; > spin_unlock(&chunk_entry->lock); > spin_unlock(&old_entry->lock); > + mutex_unlock(&old_entry->group->mark_mutex); > > fsnotify_destroy_mark(chunk_entry, audit_tree_group); > > @@ -443,6 +451,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree) > spin_unlock(&hash_lock); > spin_unlock(&chunk_entry->lock); > spin_unlock(&old_entry->lock); > + mutex_unlock(&old_entry->group->mark_mutex); > fsnotify_destroy_mark(old_entry, audit_tree_group); > fsnotify_put_mark(chunk_entry); /* drop initial reference */ > fsnotify_put_mark(old_entry); /* pair to fsnotify_find mark_entry */ > -- > 2.10.2 > -- paul moore www.paul-moore.com

7 years, 11 months

2
7
0 / 0

Re: [PATCH 06/22] audit: Abstract hash key handling

by Paul Moore

On Thu, Dec 22, 2016 at 4:15 AM, Jan Kara <jack(a)suse.cz> wrote: > Audit tree currently uses inode pointer as a key into the hash table. > Getting that from notification mark will be somewhat more difficult with > coming fsnotify changes and there's no reason we really have to use the > inode pointer. So abstract getting of hash key from the audit chunk and > inode so that we can switch to a different key easily later. > > CC: Paul Moore <paul(a)paul-moore.com> > Signed-off-by: Jan Kara <jack(a)suse.cz> > --- > kernel/audit_tree.c | 39 ++++++++++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 11 deletions(-) I have no objections with this patch in particular, but in patch 8, are you certain that inode_to_key() and chunk_to_key() will continue to return the same key value? > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > index 156b6a93f4fc..f0859828de09 100644 > --- a/kernel/audit_tree.c > +++ b/kernel/audit_tree.c > @@ -163,33 +163,48 @@ enum {HASH_SIZE = 128}; > static struct list_head chunk_hash_heads[HASH_SIZE]; > static __cacheline_aligned_in_smp DEFINE_SPINLOCK(hash_lock); > > -static inline struct list_head *chunk_hash(const struct inode *inode) > +/* Function to return search key in our hash from inode. */ > +static unsigned long inode_to_key(const struct inode *inode) > { > - unsigned long n = (unsigned long)inode / L1_CACHE_BYTES; > + return (unsigned long)inode; > +} > + > +/* > + * Function to return search key in our hash from chunk. Key 0 is special and > + * should never be present in the hash. > + */ > +static unsigned long chunk_to_key(struct audit_chunk *chunk) > +{ > + return (unsigned long)chunk->mark.inode; > +} > + > +static inline struct list_head *chunk_hash(unsigned long key) > +{ > + unsigned long n = key / L1_CACHE_BYTES; > return chunk_hash_heads + n % HASH_SIZE; > } > > /* hash_lock & entry->lock is held by caller */ > static void insert_hash(struct audit_chunk *chunk) > { > - struct fsnotify_mark *entry = &chunk->mark; > + unsigned long key = chunk_to_key(chunk); > struct list_head *list; > > - if (!entry->inode) > + if (!key) > return; > - list = chunk_hash(entry->inode); > + list = chunk_hash(key); > list_add_rcu(&chunk->hash, list); > } > > /* called under rcu_read_lock */ > struct audit_chunk *audit_tree_lookup(const struct inode *inode) > { > - struct list_head *list = chunk_hash(inode); > + unsigned long key = inode_to_key(inode); > + struct list_head *list = chunk_hash(key); > struct audit_chunk *p; > > list_for_each_entry_rcu(p, list, hash) { > - /* mark.inode may have gone NULL, but who cares? */ > - if (p->mark.inode == inode) { > + if (chunk_to_key(p) == key) { > atomic_long_inc(&p->refs); > return p; > } > @@ -585,7 +600,8 @@ int audit_remove_tree_rule(struct audit_krule *rule) > > static int compare_root(struct vfsmount *mnt, void *arg) > { > - return d_backing_inode(mnt->mnt_root) == arg; > + return inode_to_key(d_backing_inode(mnt->mnt_root)) == > + (unsigned long)arg; > } > > void audit_trim_trees(void) > @@ -620,9 +636,10 @@ void audit_trim_trees(void) > list_for_each_entry(node, &tree->chunks, list) { > struct audit_chunk *chunk = find_chunk(node); > /* this could be NULL if the watch is dying else where... */ > - struct inode *inode = chunk->mark.inode; > node->index |= 1U<<31; > - if (iterate_mounts(compare_root, inode, root_mnt)) > + if (iterate_mounts(compare_root, > + (void *)chunk_to_key(chunk), > + root_mnt)) > node->index &= ~(1U<<31); > } > spin_unlock(&hash_lock); > -- > 2.10.2 > -- paul moore www.paul-moore.com

7 years, 11 months

2
4
0 / 0

[PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions

by Tyler Hicks

This patch set creates the basis for auditing information specific to a given seccomp return action and then starts auditing SECCOMP_RET_ERRNO return actions. The audit messages for SECCOMP_RET_ERRNO return actions include the errno value that will be returned to userspace. Tyler

7 years, 11 months

7
25
0 / 0

[PATCH 0/2] Support auditing while still allowing a syscall

by Tyler Hicks

Allow application authors to opt into auditing a syscall before allowing it. This differs slightly from SECCOMP_RET_ALLOW in that an audit message is generated for the syscall. It can be useful when initially setting up a seccomp sandbox for your application if you set the default action to audit instead of, for example, kill when the application makes a restricted system call. The application author can easily compile a list of syscalls that need to be allowed instead of iteratively building the application, testing the application, updating the filter, and repeating until all appropriate system calls have been allowed. This patch set depends on the following patch set: https://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canon... (At least, I hope it shows up at that link soon because marc.info doesn't yet know of the message-id.) The corresponding libseccomp changes can be found here: https://github.com/seccomp/libseccomp/pull/64 Thanks! Tyler

7 years, 11 months

2
3
0 / 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Linux-audit January 2017