[PATCH] audit: track the owner of the command mutex ourselves
Richard Guy Briggs
rgb at redhat.com
Thu Feb 22 08:40:47 UTC 2018
On 2018-02-20 11:55, Paul Moore wrote:
> From: Paul Moore <paul at paul-moore.com>
>
> Evidently the __mutex_owner() function was never intended for use
> outside the core mutex code, so build a thing locking wrapper around
> the mutex code which allows us to track the mutex owner.
>
> One, arguably positive, side effect is that this allows us to hide
> the audit_cmd_mutex inside of kernel/audit.c behind the lock/unlock
> functions.
>
> Reported-by: Peter Zijlstra <peterz at infradead.org>
> Signed-off-by: Paul Moore <paul at paul-moore.com>
This is what I was trying to accomplish here through several iterations,
but your solution looks more graceful:
https://www.redhat.com/archives/linux-audit/2015-October/msg00073.html
/me has no dog...
Reviewed-by: Richard Guy Briggs <rgb at redhat.com>
> ---
> kernel/audit.c | 66 +++++++++++++++++++++++++++++++++++++++++++--------
> kernel/audit.h | 3 ++
> kernel/audit_tree.c | 8 +++---
> 3 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 5c2544984375..3c4f6f3d7041 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -181,9 +181,21 @@ static char *audit_feature_names[2] = {
> "loginuid_immutable",
> };
>
> -
> -/* Serialize requests from userspace. */
> -DEFINE_MUTEX(audit_cmd_mutex);
> +/**
> + * struct audit_ctl_mutex - serialize requests from userspace
> + * @lock: the mutex used for locking
> + * @owner: the task which owns the lock
> + *
> + * Description:
> + * This is the lock struct used to ensure we only process userspace requests
> + * in an orderly fashion. We can't simply use a mutex/lock here because we
> + * need to track lock ownership so we don't end up blocking the lock owner in
> + * audit_log_start() or similar.
> + */
> +static struct audit_ctl_mutex {
> + struct mutex lock;
> + void *owner;
> +} audit_cmd_mutex;
>
> /* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
> * audit records. Since printk uses a 1024 byte buffer, this buffer
> @@ -227,6 +239,36 @@ int auditd_test_task(struct task_struct *task)
> return rc;
> }
>
> +/**
> + * audit_ctl_lock - Take the audit control lock
> + */
> +void audit_ctl_lock(void)
> +{
> + mutex_lock(&audit_cmd_mutex.lock);
> + audit_cmd_mutex.owner = current;
> +}
> +
> +/**
> + * audit_ctl_unlock - Drop the audit control lock
> + */
> +void audit_ctl_unlock(void)
> +{
> + audit_cmd_mutex.owner = NULL;
> + mutex_unlock(&audit_cmd_mutex.lock);
> +}
> +
> +/**
> + * audit_ctl_owner_current - Test to see if the current task owns the lock
> + *
> + * Description:
> + * Return true if the current task owns the audit control lock, false if it
> + * doesn't own the lock.
> + */
> +static bool audit_ctl_owner_current(void)
> +{
> + return (current == audit_cmd_mutex.owner);
> +}
> +
> /**
> * auditd_pid_vnr - Return the auditd PID relative to the namespace
> *
> @@ -861,8 +903,8 @@ int audit_send_list(void *_dest)
> struct sock *sk = audit_get_sk(dest->net);
>
> /* wait for parent to finish and send an ACK */
> - mutex_lock(&audit_cmd_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> + audit_ctl_lock();
> + audit_ctl_unlock();
>
> while ((skb = __skb_dequeue(&dest->q)) != NULL)
> netlink_unicast(sk, skb, dest->portid, 0);
> @@ -903,8 +945,8 @@ static int audit_send_reply_thread(void *arg)
> struct audit_reply *reply = (struct audit_reply *)arg;
> struct sock *sk = audit_get_sk(reply->net);
>
> - mutex_lock(&audit_cmd_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> + audit_ctl_lock();
> + audit_ctl_unlock();
>
> /* Ignore failure. It'll only happen if the sender goes away,
> because our timeout is set to infinite. */
> @@ -1467,7 +1509,7 @@ static void audit_receive(struct sk_buff *skb)
> nlh = nlmsg_hdr(skb);
> len = skb->len;
>
> - mutex_lock(&audit_cmd_mutex);
> + audit_ctl_lock();
> while (nlmsg_ok(nlh, len)) {
> err = audit_receive_msg(skb, nlh);
> /* if err or if this message says it wants a response */
> @@ -1476,7 +1518,7 @@ static void audit_receive(struct sk_buff *skb)
>
> nlh = nlmsg_next(nlh, &len);
> }
> - mutex_unlock(&audit_cmd_mutex);
> + audit_ctl_unlock();
> }
>
> /* Run custom bind function on netlink socket group connect or bind requests. */
> @@ -1548,6 +1590,9 @@ static int __init audit_init(void)
> for (i = 0; i < AUDIT_INODE_BUCKETS; i++)
> INIT_LIST_HEAD(&audit_inode_hash[i]);
>
> + mutex_init(&audit_cmd_mutex.lock);
> + audit_cmd_mutex.owner = NULL;
> +
> pr_info("initializing netlink subsys (%s)\n",
> audit_default ? "enabled" : "disabled");
> register_pernet_subsys(&audit_net_ops);
> @@ -1711,8 +1756,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
> * using a PID anchored in the caller's namespace
> * 2. generator holding the audit_cmd_mutex - we don't want to block
> * while holding the mutex */
> - if (!(auditd_test_task(current) ||
> - (current == __mutex_owner(&audit_cmd_mutex)))) {
> + if (!(auditd_test_task(current) || audit_ctl_owner_current())) {
> long stime = audit_backlog_wait_time;
>
> while (audit_backlog_limit &&
> diff --git a/kernel/audit.h b/kernel/audit.h
> index af5bc59487ed..214e14948370 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -341,4 +341,5 @@ extern struct list_head *audit_killed_trees(void);
> #define audit_filter_inodes(t,c) AUDIT_DISABLED
> #endif
>
> -extern struct mutex audit_cmd_mutex;
> +extern void audit_ctl_lock(void);
> +extern void audit_ctl_unlock(void);
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index fd353120e0d9..67e6956c0b61 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -709,7 +709,7 @@ static int prune_tree_thread(void *unused)
> schedule();
> }
>
> - mutex_lock(&audit_cmd_mutex);
> + audit_ctl_lock();
> mutex_lock(&audit_filter_mutex);
>
> while (!list_empty(&prune_list)) {
> @@ -727,7 +727,7 @@ static int prune_tree_thread(void *unused)
> }
>
> mutex_unlock(&audit_filter_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> + audit_ctl_unlock();
> }
> return 0;
> }
> @@ -924,7 +924,7 @@ static void audit_schedule_prune(void)
> */
> void audit_kill_trees(struct list_head *list)
> {
> - mutex_lock(&audit_cmd_mutex);
> + audit_ctl_lock();
> mutex_lock(&audit_filter_mutex);
>
> while (!list_empty(list)) {
> @@ -942,7 +942,7 @@ void audit_kill_trees(struct list_head *list)
> }
>
> mutex_unlock(&audit_filter_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> + audit_ctl_unlock();
> }
>
> /*
>
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
More information about the Linux-audit
mailing list