[PATCH next] audit: printk before dropping logs in audit_log_end

[PATCH next] audit: printk before dropping logs in audit_log_end

[Ecronic]

If the log rate of audit exceeds audit_rate_limit, audit_log_end
will drop the audit logs. Printk before dropping them may be a
better choice.

Signed-off-by: Ecronic <ecronic@outlook.com>
---
 kernel/audit.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index a75978ae38ad..3f5be93447cb 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2415,10 +2415,10 @@ void audit_log_end(struct audit_buffer *ab)
 	if (!ab)
 		return;
 
-	if (audit_rate_check()) {
-		skb = ab->skb;
-		ab->skb = NULL;
+	skb = ab->skb;
+	ab->skb = NULL;
 
+	if (audit_rate_check()) {
 		/* setup the netlink header, see the comments in
 		 * kauditd_send_multicast_skb() for length quirks */
 		nlh = nlmsg_hdr(skb);
@@ -2427,8 +2427,11 @@ void audit_log_end(struct audit_buffer *ab)
 		/* queue the netlink packet and poke the kauditd thread */
 		skb_queue_tail(&audit_queue, skb);
 		wake_up_interruptible(&kauditd_wait);
-	} else
+	} else {
+		kauditd_printk_skb(skb);
+		kfree_skb(skb);
 		audit_log_lost("rate limit exceeded");
+	}
 
 	audit_buffer_free(ab);
 }
-- 
2.30.0

[PATCH next] audit: printk before dropping logs in audit_log_end

[Ecronic]

If the log rate of audit exceeds audit_rate_limit, audit_log_end
will drop the audit logs. Printk before dropping them may be a
better choice.

Signed-off-by: Ecronic <ecronic@outlook.com>
---
 kernel/audit.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index a75978ae38ad..3f5be93447cb 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2415,10 +2415,10 @@ void audit_log_end(struct audit_buffer *ab)
 	if (!ab)
 		return;
 
-	if (audit_rate_check()) {
-		skb = ab->skb;
-		ab->skb = NULL;
+	skb = ab->skb;
+	ab->skb = NULL;
 
+	if (audit_rate_check()) {
 		/* setup the netlink header, see the comments in
 		 * kauditd_send_multicast_skb() for length quirks */
 		nlh = nlmsg_hdr(skb);
@@ -2427,8 +2427,11 @@ void audit_log_end(struct audit_buffer *ab)
 		/* queue the netlink packet and poke the kauditd thread */
 		skb_queue_tail(&audit_queue, skb);
 		wake_up_interruptible(&kauditd_wait);
-	} else
+	} else {
+		kauditd_printk_skb(skb);
+		kfree_skb(skb);
 		audit_log_lost("rate limit exceeded");
+	}
 
 	audit_buffer_free(ab);
 }
-- 
2.30.0

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Paul Moore]

On Sun, Aug 21, 2022 at 10:22 AM Ecronic <ecronic@outlook.com> wrote:
>
> If the log rate of audit exceeds audit_rate_limit, audit_log_end
> will drop the audit logs. Printk before dropping them may be a
> better choice.
>
> Signed-off-by: Ecronic <ecronic@outlook.com>
> ---
>  kernel/audit.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Hi Ecronic,

I'm not sure this is the right approach; if we're hitting a rate
limit, printing the record to the console is likely the last thing we
want to do.  Are you currently hitting a problem with the rate
limiting, or is this simply something you found via code inspection?

> diff --git a/kernel/audit.c b/kernel/audit.c
> index a75978ae38ad..3f5be93447cb 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2415,10 +2415,10 @@ void audit_log_end(struct audit_buffer *ab)
>         if (!ab)
>                 return;
>
> -       if (audit_rate_check()) {
> -               skb = ab->skb;
> -               ab->skb = NULL;
> +       skb = ab->skb;
> +       ab->skb = NULL;
>
> +       if (audit_rate_check()) {
>                 /* setup the netlink header, see the comments in
>                  * kauditd_send_multicast_skb() for length quirks */
>                 nlh = nlmsg_hdr(skb);
> @@ -2427,8 +2427,11 @@ void audit_log_end(struct audit_buffer *ab)
>                 /* queue the netlink packet and poke the kauditd thread */
>                 skb_queue_tail(&audit_queue, skb);
>                 wake_up_interruptible(&kauditd_wait);
> -       } else
> +       } else {
> +               kauditd_printk_skb(skb);
> +               kfree_skb(skb);
>                 audit_log_lost("rate limit exceeded");
> +       }
>
>         audit_buffer_free(ab);
>  }
> --
> 2.30.0

-- 
paul-moore.com

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Paul Moore]

On Sun, Aug 21, 2022 at 10:22 AM Ecronic <ecronic@outlook.com> wrote:
>
> If the log rate of audit exceeds audit_rate_limit, audit_log_end
> will drop the audit logs. Printk before dropping them may be a
> better choice.
>
> Signed-off-by: Ecronic <ecronic@outlook.com>
> ---
>  kernel/audit.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Hi Ecronic,

I'm not sure this is the right approach; if we're hitting a rate
limit, printing the record to the console is likely the last thing we
want to do.  Are you currently hitting a problem with the rate
limiting, or is this simply something you found via code inspection?

> diff --git a/kernel/audit.c b/kernel/audit.c
> index a75978ae38ad..3f5be93447cb 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2415,10 +2415,10 @@ void audit_log_end(struct audit_buffer *ab)
>         if (!ab)
>                 return;
>
> -       if (audit_rate_check()) {
> -               skb = ab->skb;
> -               ab->skb = NULL;
> +       skb = ab->skb;
> +       ab->skb = NULL;
>
> +       if (audit_rate_check()) {
>                 /* setup the netlink header, see the comments in
>                  * kauditd_send_multicast_skb() for length quirks */
>                 nlh = nlmsg_hdr(skb);
> @@ -2427,8 +2427,11 @@ void audit_log_end(struct audit_buffer *ab)
>                 /* queue the netlink packet and poke the kauditd thread */
>                 skb_queue_tail(&audit_queue, skb);
>                 wake_up_interruptible(&kauditd_wait);
> -       } else
> +       } else {
> +               kauditd_printk_skb(skb);
> +               kfree_skb(skb);
>                 audit_log_lost("rate limit exceeded");
> +       }
>
>         audit_buffer_free(ab);
>  }
> --
> 2.30.0

-- 
paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Gaosheng Cui]

[-- Attachment #1: Type: text/html, Size: 3125 bytes --]

[-- Attachment #2: Type: text/plain, Size: 107 bytes --]

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Paul Moore]

On Mon, Aug 22, 2022 at 10:33 PM Gaosheng Cui <ecronic@outlook.com> wrote:
>
> Thanks for your reply.
>
> This is a personal idea of mine，in the process of using audit，I find that if the audit rules are configured too much，or the server hard-disk performance is too poor，hitting a rate limit will be easy to occur，then some logs would be dropped directly.
> I think we should print the record to the console，just likely the last thing we want to do，better play the role of audit，and improve kernel security.
>
> I hope that will be helpful，thanks.

Yes, thank you for the additional information on your environment and
use case.  As I'm sure you already know, the audit rate limit, backlog
queue depth, and other related tunables can all be configured at boot
or runtime to help ensure that the system remains responsive in the
face of higher audit loads.

-- 
paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Paul Moore]

On Mon, Aug 22, 2022 at 10:33 PM Gaosheng Cui <ecronic@outlook.com> wrote:
>
> Thanks for your reply.
>
> This is a personal idea of mine，in the process of using audit，I find that if the audit rules are configured too much，or the server hard-disk performance is too poor，hitting a rate limit will be easy to occur，then some logs would be dropped directly.
> I think we should print the record to the console，just likely the last thing we want to do，better play the role of audit，and improve kernel security.
>
> I hope that will be helpful，thanks.

Yes, thank you for the additional information on your environment and
use case.  As I'm sure you already know, the audit rate limit, backlog
queue depth, and other related tunables can all be configured at boot
or runtime to help ensure that the system remains responsive in the
face of higher audit loads.

-- 
paul-moore.com

Re: [PATCH next] audit: printk before dropping logs in audit_log_end

[Gaosheng Cui]

[-- Attachment #1: Type: text/html, Size: 1606 bytes --]

[-- Attachment #2: Type: text/plain, Size: 107 bytes --]

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit