[PATCH] audit: ensure userspace is penalized the same as the kernel when under pressure

[PATCH] audit: ensure userspace is penalized the same as the kernel when under pressure

[Paul Moore]

Due to the audit control mutex necessary for serializing audit
userspace messages we haven't been able to block/penalize userspace
processes that attempt to send audit records while the system is
under audit pressure.  The result is that privileged userspace
applications have a priority boost with respect to audit as they are
not bound by the same audit queue throttling as the other tasks on
the system.

This patch attempts to restore some balance to the system when under
audit pressure by blocking these privileged userspace tasks after
they have finished their audit processing, and dropped the audit
control mutex, but before they return to userspace.

Reported-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 4cebadb5f30d..eab7282668ab 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1540,6 +1540,20 @@ static void audit_receive(struct sk_buff  *skb)
 		nlh = nlmsg_next(nlh, &len);
 	}
 	audit_ctl_unlock();
+
+	/* can't block with the ctrl lock, so penalize the sender now */
+	if (audit_backlog_limit &&
+	    (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
+		DECLARE_WAITQUEUE(wait, current);
+
+		/* wake kauditd to try and flush the queue */
+		wake_up_interruptible(&kauditd_wait);
+
+		add_wait_queue_exclusive(&audit_backlog_wait, &wait);
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(audit_backlog_wait_time);
+		remove_wait_queue(&audit_backlog_wait, &wait);
+	}
 }
 
 /* Log information about who is connecting to the audit multicast socket */
@@ -1824,7 +1838,9 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 	 *    task_tgid_vnr() since auditd_pid is set in audit_receive_msg()
 	 *    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 */
+	 *    while holding the mutex, although we do penalize the sender
+	 *    later in audit_receive() when it is safe to block
+	 */
 	if (!(auditd_test_task(current) || audit_ctl_owner_current())) {
 		long stime = audit_backlog_wait_time;
 

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

Re: [PATCH] audit: ensure userspace is penalized the same as the kernel when under pressure

[cuigaosheng]

在 2021/12/15 0:16, Paul Moore 写道:
> Due to the audit control mutex necessary for serializing audit
> userspace messages we haven't been able to block/penalize userspace
> processes that attempt to send audit records while the system is
> under audit pressure.  The result is that privileged userspace
> applications have a priority boost with respect to audit as they are
> not bound by the same audit queue throttling as the other tasks on
> the system.
>
> This patch attempts to restore some balance to the system when under
> audit pressure by blocking these privileged userspace tasks after
> they have finished their audit processing, and dropped the audit
> control mutex, but before they return to userspace.
>
> Reported-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>   kernel/audit.c |   18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 4cebadb5f30d..eab7282668ab 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1540,6 +1540,20 @@ static void audit_receive(struct sk_buff  *skb)
>   		nlh = nlmsg_next(nlh, &len);
>   	}
>   	audit_ctl_unlock();
> +
> +	/* can't block with the ctrl lock, so penalize the sender now */
> +	if (audit_backlog_limit &&
> +	    (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
> +		DECLARE_WAITQUEUE(wait, current);
> +
> +		/* wake kauditd to try and flush the queue */
> +		wake_up_interruptible(&kauditd_wait);
> +
> +		add_wait_queue_exclusive(&audit_backlog_wait, &wait);
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(audit_backlog_wait_time);
> +		remove_wait_queue(&audit_backlog_wait, &wait);
> +	}
>   }
>   
>   /* Log information about who is connecting to the audit multicast socket */
> @@ -1824,7 +1838,9 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
>   	 *    task_tgid_vnr() since auditd_pid is set in audit_receive_msg()
>   	 *    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 */
> +	 *    while holding the mutex, although we do penalize the sender
> +	 *    later in audit_receive() when it is safe to block
> +	 */
>   	if (!(auditd_test_task(current) || audit_ctl_owner_current())) {
>   		long stime = audit_backlog_wait_time;
>   

Tested-by: Gaosheng Cui<cuigaosheng1@huawei.com>


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

Re: [PATCH] audit: ensure userspace is penalized the same as the kernel when under pressure

[Richard Guy Briggs]

On 2021-12-14 11:16, Paul Moore wrote:
> Due to the audit control mutex necessary for serializing audit
> userspace messages we haven't been able to block/penalize userspace
> processes that attempt to send audit records while the system is
> under audit pressure.  The result is that privileged userspace
> applications have a priority boost with respect to audit as they are
> not bound by the same audit queue throttling as the other tasks on
> the system.
> 
> This patch attempts to restore some balance to the system when under
> audit pressure by blocking these privileged userspace tasks after
> they have finished their audit processing, and dropped the audit
> control mutex, but before they return to userspace.

I can't speak to the exact wait queue mechanism here, but this seems
like a reasonable action in this location.  FWIW: Reviewed-by

> Reported-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  kernel/audit.c |   18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 4cebadb5f30d..eab7282668ab 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1540,6 +1540,20 @@ static void audit_receive(struct sk_buff  *skb)
>  		nlh = nlmsg_next(nlh, &len);
>  	}
>  	audit_ctl_unlock();
> +
> +	/* can't block with the ctrl lock, so penalize the sender now */
> +	if (audit_backlog_limit &&
> +	    (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
> +		DECLARE_WAITQUEUE(wait, current);
> +
> +		/* wake kauditd to try and flush the queue */
> +		wake_up_interruptible(&kauditd_wait);
> +
> +		add_wait_queue_exclusive(&audit_backlog_wait, &wait);
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(audit_backlog_wait_time);
> +		remove_wait_queue(&audit_backlog_wait, &wait);
> +	}
>  }
>  
>  /* Log information about who is connecting to the audit multicast socket */
> @@ -1824,7 +1838,9 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
>  	 *    task_tgid_vnr() since auditd_pid is set in audit_receive_msg()
>  	 *    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 */
> +	 *    while holding the mutex, although we do penalize the sender
> +	 *    later in audit_receive() when it is safe to block
> +	 */
>  	if (!(auditd_test_task(current) || audit_ctl_owner_current())) {
>  		long stime = audit_backlog_wait_time;
>  
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit
> 

- RGB

--
Richard Guy Briggs <rgb@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

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

Re: [PATCH] audit: ensure userspace is penalized the same as the kernel when under pressure

[Paul Moore]

On Tue, Dec 14, 2021 at 11:16 AM Paul Moore <paul@paul-moore.com> wrote:
>
> Due to the audit control mutex necessary for serializing audit
> userspace messages we haven't been able to block/penalize userspace
> processes that attempt to send audit records while the system is
> under audit pressure.  The result is that privileged userspace
> applications have a priority boost with respect to audit as they are
> not bound by the same audit queue throttling as the other tasks on
> the system.
>
> This patch attempts to restore some balance to the system when under
> audit pressure by blocking these privileged userspace tasks after
> they have finished their audit processing, and dropped the audit
> control mutex, but before they return to userspace.
>
> Reported-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  kernel/audit.c |   18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

FYI, I just merged this into audit/next; thanks for the testing/review
help everyone!

-- 
paul moore
www.paul-moore.com

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