diff mbox series

[-next] audit: refactor audit_log_cap to improve performance

Message ID 20230307122524.2341659-1-cuigaosheng1@huawei.com (mailing list archive)
State Rejected
Delegated to: Paul Moore
Headers show
Series [-next] audit: refactor audit_log_cap to improve performance | expand

Commit Message

cuigaosheng March 7, 2023, 12:25 p.m. UTC 
By testing the performance of snprintf on qemu, I found that
snprintf("%s", str) performed much better than snprintf("xx%s", str),
the former takes 2ns while the latter takes 60ns.

Based on the test results, using audit_log_format(ab, "%s", prefix)
instead of audit_log_format(ab, " %s=0", prefix) will get better
performance, so refactor the audit_log_cap() to improve performance.

Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 kernel/auditsc.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

Comments

Paul Moore March 8, 2023, 5:14 p.m. UTC | #1 
On Tue, Mar 7, 2023 at 7:25 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote:
>
> By testing the performance of snprintf on qemu, I found that
> snprintf("%s", str) performed much better than snprintf("xx%s", str),
> the former takes 2ns while the latter takes 60ns.
>
> Based on the test results, using audit_log_format(ab, "%s", prefix)
> instead of audit_log_format(ab, " %s=0", prefix) will get better
> performance, so refactor the audit_log_cap() to improve performance.
>
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>  kernel/auditsc.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index addeed3df15d..d04d0a57a603 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1295,11 +1295,12 @@ static void audit_log_execve_info(struct audit_context *context,
>  static void audit_log_cap(struct audit_buffer *ab, char *prefix,
>                           kernel_cap_t *cap)
>  {
> +       audit_log_format(ab, "%s", prefix);
>         if (cap_isclear(*cap)) {
> -               audit_log_format(ab, " %s=0", prefix);
> +               audit_log_format(ab, "0");
>                 return;
>         }
> -       audit_log_format(ab, " %s=%016llx", prefix, cap->val);
> +       audit_log_format(ab, "%016llx", cap->val);
>  }

Have you specifically measured the impact to audit?  I worry that any
improvements gained by using snprintf() differently will be lost by
the additional audit_log_format() call for every capability logged.
diff mbox series

Patch

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index addeed3df15d..d04d0a57a603 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1295,11 +1295,12 @@  static void audit_log_execve_info(struct audit_context *context,
 static void audit_log_cap(struct audit_buffer *ab, char *prefix,
 			  kernel_cap_t *cap)
 {
+	audit_log_format(ab, "%s", prefix);
 	if (cap_isclear(*cap)) {
-		audit_log_format(ab, " %s=0", prefix);
+		audit_log_format(ab, "0");
 		return;
 	}
-	audit_log_format(ab, " %s=%016llx", prefix, cap->val);
+	audit_log_format(ab, "%016llx", cap->val);
 }
 
 static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
@@ -1308,8 +1309,8 @@  static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
 		audit_log_format(ab, " cap_fe=? cap_fver=? cap_fp=? cap_fi=?");
 		return;
 	}
-	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
-	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
+	audit_log_cap(ab, " cap_fp=", &name->fcap.permitted);
+	audit_log_cap(ab, " cap_fi=", &name->fcap.inheritable);
 	audit_log_format(ab, " cap_fe=%d cap_fver=%x cap_frootid=%d",
 			 name->fcap.fE, name->fcap_ver,
 			 from_kuid(&init_user_ns, name->fcap.rootid));
@@ -1450,10 +1451,10 @@  static void show_special(struct audit_context *context, int *call_panic)
 		break; }
 	case AUDIT_CAPSET:
 		audit_log_format(ab, "pid=%d", context->capset.pid);
-		audit_log_cap(ab, "cap_pi", &context->capset.cap.inheritable);
-		audit_log_cap(ab, "cap_pp", &context->capset.cap.permitted);
-		audit_log_cap(ab, "cap_pe", &context->capset.cap.effective);
-		audit_log_cap(ab, "cap_pa", &context->capset.cap.ambient);
+		audit_log_cap(ab, " cap_pi=", &context->capset.cap.inheritable);
+		audit_log_cap(ab, " cap_pp=", &context->capset.cap.permitted);
+		audit_log_cap(ab, " cap_pe=", &context->capset.cap.effective);
+		audit_log_cap(ab, " cap_pa=", &context->capset.cap.ambient);
 		break;
 	case AUDIT_MMAP:
 		audit_log_format(ab, "fd=%d flags=0x%x", context->mmap.fd,
@@ -1726,17 +1727,17 @@  static void audit_log_exit(void)
 			struct audit_aux_data_bprm_fcaps *axs = (void *)aux;
 
 			audit_log_format(ab, "fver=%x", axs->fcap_ver);
-			audit_log_cap(ab, "fp", &axs->fcap.permitted);
-			audit_log_cap(ab, "fi", &axs->fcap.inheritable);
+			audit_log_cap(ab, " fp=", &axs->fcap.permitted);
+			audit_log_cap(ab, " fi=", &axs->fcap.inheritable);
 			audit_log_format(ab, " fe=%d", axs->fcap.fE);
-			audit_log_cap(ab, "old_pp", &axs->old_pcap.permitted);
-			audit_log_cap(ab, "old_pi", &axs->old_pcap.inheritable);
-			audit_log_cap(ab, "old_pe", &axs->old_pcap.effective);
-			audit_log_cap(ab, "old_pa", &axs->old_pcap.ambient);
-			audit_log_cap(ab, "pp", &axs->new_pcap.permitted);
-			audit_log_cap(ab, "pi", &axs->new_pcap.inheritable);
-			audit_log_cap(ab, "pe", &axs->new_pcap.effective);
-			audit_log_cap(ab, "pa", &axs->new_pcap.ambient);
+			audit_log_cap(ab, " old_pp=", &axs->old_pcap.permitted);
+			audit_log_cap(ab, " old_pi=", &axs->old_pcap.inheritable);
+			audit_log_cap(ab, " old_pe=", &axs->old_pcap.effective);
+			audit_log_cap(ab, " old_pa=", &axs->old_pcap.ambient);
+			audit_log_cap(ab, " pp=", &axs->new_pcap.permitted);
+			audit_log_cap(ab, " pi=", &axs->new_pcap.inheritable);
+			audit_log_cap(ab, " pe=", &axs->new_pcap.effective);
+			audit_log_cap(ab, " pa=", &axs->new_pcap.ambient);
 			audit_log_format(ab, " frootid=%d",
 					 from_kuid(&init_user_ns,
 						   axs->fcap.rootid));