Skip to content

Commit

Permalink
audit: track the owner of the command mutex ourselves
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Richard Guy Briggs <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
pcmoore committed Feb 23, 2018
1 parent 23138ea commit ce42363
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 16 deletions.
66 changes: 55 additions & 11 deletions kernel/audit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -1469,7 +1511,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 */
Expand All @@ -1478,7 +1520,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. */
Expand Down Expand Up @@ -1550,6 +1592,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);
Expand Down Expand Up @@ -1713,8 +1758,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 &&
Expand Down
3 changes: 2 additions & 1 deletion kernel/audit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
8 changes: 4 additions & 4 deletions kernel/audit_tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -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();
}

/*
Expand Down

0 comments on commit ce42363

Please sign in to comment.