2023-04-18 11:10:57

by Eiichi Tsukata

[permalink] [raw]
Subject: [PATCH v2 0/2] audit: syscall audit optimization (> +6% faster)

This patchset is an optimization of auditd_test_task() function.
As is described in Patch 1, it improves the performance of syscall
auditing.

Benchmarks
==========

Run the following micro benchmarks:

(1) dd:
dd if=/dev/zero of=/dev/null bs=1 count=5M

(2) UnixBench syscall:
./Run syscall -i 10 -c 1

With rule:

-a never,exit -F arch=b64 -S uname

Results:

(1) dd
Base line : 2.572 sec
/w this patch: 2.418 sec (6.3% faster)

(2) UnixBench syscall Index Score
Base line : 860
/w this patch: 953 (10.8% faster)


This patchset consists of the following parts:

Patch 1: add global auditd_pid to make auditd_test_task() faster
Patch 2: cleanup: replace auditd_conn.pid with auditd_pid

v1 -> v2:
- Use global auditd_pid intead of pid.is_auditd
- Add UnixBench syscall benchmark

v1: https://lore.kernel.org/audit/[email protected]/

Eiichi Tsukata (2):
audit: add global auditd_pid to make auditd_test_task() faster
audit: replace auditd_conn.pid with auditd_pid

kernel/audit.c | 61 +++++++++++++++++---------------------------------
kernel/audit.h | 4 +++-
2 files changed, 24 insertions(+), 41 deletions(-)

--
2.39.2


2023-04-18 11:11:03

by Eiichi Tsukata

[permalink] [raw]
Subject: [PATCH v2 1/2] audit: add global auditd_pid to make auditd_test_task() faster

auditd_test_task() is a hot path of system call auditing. This patch
introduces a global auditd_pid pid struct which can be used for faster
check of registered audit daemon.

Benchmarks
==========

Run the following micro benchmarks:

(1) dd:
dd if=/dev/zero of=/dev/null bs=1 count=5M

(2) UnixBench syscall:
./Run syscall -i 10 -c 1

With rule:

-a never,exit -F arch=b64 -S uname

Results:

(1) dd
Base line : 2.572 sec
/w this patch: 2.418 sec (6.3% faster)

(2) UnixBench syscall Index Score
Base line : 860
/w this patch: 953 (10.8% faster)

Signed-off-by: Eiichi Tsukata <[email protected]>
---
kernel/audit.c | 39 +++++++++++----------------------------
kernel/audit.h | 4 +++-
2 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 9bc0b0301198..9426980368e4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -71,6 +71,7 @@ static int audit_initialized = AUDIT_UNINITIALIZED;

u32 audit_enabled = AUDIT_OFF;
bool audit_ever_enabled = !!AUDIT_OFF;
+struct pid *auditd_pid;

EXPORT_SYMBOL_GPL(audit_enabled);

@@ -208,26 +209,6 @@ struct audit_reply {
struct sk_buff *skb;
};

-/**
- * auditd_test_task - Check to see if a given task is an audit daemon
- * @task: the task to check
- *
- * Description:
- * Return 1 if the task is a registered audit daemon, 0 otherwise.
- */
-int auditd_test_task(struct task_struct *task)
-{
- int rc;
- struct auditd_connection *ac;
-
- rcu_read_lock();
- ac = rcu_dereference(auditd_conn);
- rc = (ac && ac->pid == task_tgid(task) ? 1 : 0);
- rcu_read_unlock();
-
- return rc;
-}
-
/**
* audit_ctl_lock - Take the audit control lock
*/
@@ -512,6 +493,7 @@ static int auditd_set(struct pid *pid, u32 portid, struct net *net)
ac_old = rcu_dereference_protected(auditd_conn,
lockdep_is_held(&auditd_conn_lock));
rcu_assign_pointer(auditd_conn, ac_new);
+ WRITE_ONCE(auditd_pid, ac_new->pid);
spin_unlock_irqrestore(&auditd_conn_lock, flags);

if (ac_old)
@@ -652,6 +634,7 @@ static void auditd_reset(const struct auditd_connection *ac)
return;
}
rcu_assign_pointer(auditd_conn, NULL);
+ WRITE_ONCE(auditd_pid, NULL);
spin_unlock_irqrestore(&auditd_conn_lock, flags);

if (ac_old)
@@ -1263,7 +1246,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
* run auditd from the initial pid namespace, but
* something to keep in mind if this changes */
pid_t new_pid = s.pid;
- pid_t auditd_pid;
+ pid_t auditd_pid_nr;
struct pid *req_pid = task_tgid(current);

/* Sanity check - PID values must match. Setting
@@ -1274,18 +1257,18 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
/* test the auditd connection */
audit_replace(req_pid);

- auditd_pid = auditd_pid_vnr();
- if (auditd_pid) {
+ auditd_pid_nr = auditd_pid_vnr();
+ if (auditd_pid_nr) {
/* replacing a healthy auditd is not allowed */
if (new_pid) {
audit_log_config_change("audit_pid",
- new_pid, auditd_pid, 0);
+ new_pid, auditd_pid_nr, 0);
return -EEXIST;
}
/* only current auditd can unregister itself */
- if (pid_vnr(req_pid) != auditd_pid) {
+ if (pid_vnr(req_pid) != auditd_pid_nr) {
audit_log_config_change("audit_pid",
- new_pid, auditd_pid, 0);
+ new_pid, auditd_pid_nr, 0);
return -EACCES;
}
}
@@ -1298,7 +1281,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (audit_enabled != AUDIT_OFF)
audit_log_config_change("audit_pid",
new_pid,
- auditd_pid,
+ auditd_pid_nr,
err ? 0 : 1);
if (err)
return err;
@@ -1309,7 +1292,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (audit_enabled != AUDIT_OFF)
audit_log_config_change("audit_pid",
new_pid,
- auditd_pid, 1);
+ auditd_pid_nr, 1);

/* unregister the auditd connection */
auditd_reset(NULL);
diff --git a/kernel/audit.h b/kernel/audit.h
index c57b008b9914..93074c2d4346 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -214,7 +214,9 @@ extern bool audit_ever_enabled;

extern void audit_log_session_info(struct audit_buffer *ab);

-extern int auditd_test_task(struct task_struct *task);
+extern struct pid *auditd_pid;
+/* Check to see if a given task is an audit daemon */
+#define auditd_test_task(tsk) (READ_ONCE(auditd_pid) == task_tgid(tsk))

#define AUDIT_INODE_BUCKETS 32
extern struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];
--
2.39.2

2023-04-18 11:11:39

by Eiichi Tsukata

[permalink] [raw]
Subject: [PATCH v2 2/2] audit: replace auditd_conn.pid with auditd_pid

auditd_conn.pid is redundant. Replace it with auditd_pid.

Signed-off-by: Eiichi Tsukata <[email protected]>
---
kernel/audit.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 9426980368e4..72a7397eaa89 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -94,7 +94,6 @@ struct audit_net {

/**
* struct auditd_connection - kernel/auditd connection state
- * @pid: auditd PID
* @portid: netlink portid
* @net: the associated network namespace
* @rcu: RCU head
@@ -104,7 +103,6 @@ struct audit_net {
* or the associated spinlock for writing.
*/
struct auditd_connection {
- struct pid *pid;
u32 portid;
struct net *net;
struct rcu_head rcu;
@@ -248,15 +246,11 @@ static bool audit_ctl_owner_current(void)
static pid_t auditd_pid_vnr(void)
{
pid_t pid;
- const struct auditd_connection *ac;
+ unsigned long flags;

- rcu_read_lock();
- ac = rcu_dereference(auditd_conn);
- if (!ac || !ac->pid)
- pid = 0;
- else
- pid = pid_vnr(ac->pid);
- rcu_read_unlock();
+ spin_lock_irqsave(&auditd_conn_lock, flags);
+ pid = auditd_pid ? pid_vnr(auditd_pid) : 0;
+ spin_unlock_irqrestore(&auditd_conn_lock, flags);

return pid;
}
@@ -459,7 +453,6 @@ static void auditd_conn_free(struct rcu_head *rcu)
struct auditd_connection *ac;

ac = container_of(rcu, struct auditd_connection, rcu);
- put_pid(ac->pid);
put_net(ac->net);
kfree(ac);
}
@@ -478,6 +471,7 @@ static int auditd_set(struct pid *pid, u32 portid, struct net *net)
{
unsigned long flags;
struct auditd_connection *ac_old, *ac_new;
+ struct pid *auditd_pid_old;

if (!pid || !net)
return -EINVAL;
@@ -485,7 +479,6 @@ static int auditd_set(struct pid *pid, u32 portid, struct net *net)
ac_new = kzalloc(sizeof(*ac_new), GFP_KERNEL);
if (!ac_new)
return -ENOMEM;
- ac_new->pid = get_pid(pid);
ac_new->portid = portid;
ac_new->net = get_net(net);

@@ -493,9 +486,11 @@ static int auditd_set(struct pid *pid, u32 portid, struct net *net)
ac_old = rcu_dereference_protected(auditd_conn,
lockdep_is_held(&auditd_conn_lock));
rcu_assign_pointer(auditd_conn, ac_new);
- WRITE_ONCE(auditd_pid, ac_new->pid);
+ auditd_pid_old = auditd_pid;
+ WRITE_ONCE(auditd_pid, get_pid(pid));
spin_unlock_irqrestore(&auditd_conn_lock, flags);

+ put_pid(auditd_pid_old);
if (ac_old)
call_rcu(&ac_old->rcu, auditd_conn_free);

@@ -623,6 +618,7 @@ static void auditd_reset(const struct auditd_connection *ac)
unsigned long flags;
struct sk_buff *skb;
struct auditd_connection *ac_old;
+ struct pid *auditd_pid_old;

/* if it isn't already broken, break the connection */
spin_lock_irqsave(&auditd_conn_lock, flags);
@@ -634,9 +630,11 @@ static void auditd_reset(const struct auditd_connection *ac)
return;
}
rcu_assign_pointer(auditd_conn, NULL);
+ auditd_pid_old = auditd_pid;
WRITE_ONCE(auditd_pid, NULL);
spin_unlock_irqrestore(&auditd_conn_lock, flags);

+ put_pid(auditd_pid_old);
if (ac_old)
call_rcu(&ac_old->rcu, auditd_conn_free);

--
2.39.2

2023-04-18 13:36:11

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] audit: add global auditd_pid to make auditd_test_task() faster

On Tue, Apr 18, 2023 at 7:10 AM Eiichi Tsukata
<[email protected]> wrote:
>
> auditd_test_task() is a hot path of system call auditing. This patch
> introduces a global auditd_pid pid struct which can be used for faster
> check of registered audit daemon.
>
> Benchmarks
> ==========
>
> Run the following micro benchmarks:
>
> (1) dd:
> dd if=/dev/zero of=/dev/null bs=1 count=5M
>
> (2) UnixBench syscall:
> ./Run syscall -i 10 -c 1
>
> With rule:
>
> -a never,exit -F arch=b64 -S uname
>
> Results:
>
> (1) dd
> Base line : 2.572 sec
> /w this patch: 2.418 sec (6.3% faster)
>
> (2) UnixBench syscall Index Score
> Base line : 860
> /w this patch: 953 (10.8% faster)
>
> Signed-off-by: Eiichi Tsukata <[email protected]>
> ---
> kernel/audit.c | 39 +++++++++++----------------------------
> kernel/audit.h | 4 +++-
> 2 files changed, 14 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 9bc0b0301198..9426980368e4 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -71,6 +71,7 @@ static int audit_initialized = AUDIT_UNINITIALIZED;
>
> u32 audit_enabled = AUDIT_OFF;
> bool audit_ever_enabled = !!AUDIT_OFF;
> +struct pid *auditd_pid;

As discussed previously, I want to keep the auditd tracking PID in the
auditd_connection struct.

--
paul-moore.com