linux-kernel - Re: [PATCH] audit: wait_for_auditd() should use TASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130524174127.GA17186@redhat.com>
Date:	Fri, 24 May 2013 19:41:27 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Guy Streeter <streeter@...hat.com>, Eric Paris <eparis@...hat.com>,
	David Woodhouse <dwmw2@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] audit: wait_for_auditd() should use
	TASK_UNINTERRUPTIBLE

On 05/24, Oleg Nesterov wrote:
>
> audit_log_start() does wait_for_auditd() in a loop until
> audit_backlog_wait_time passes or audit_skb_queue has a room.
>
> If signal_pending() is true this becomes a busy-wait loop,
> schedule() in TASK_INTERRUPTIBLE won't block.

And the code looks strange imho. I think it should be cleanuped,
something like below. But I do not know how can I test this change.

Oleg.

--- x/kernel/audit.c
+++ x/kernel/audit.c
@@ -1053,18 +1053,19 @@ static inline void audit_get_stamp(struc
 /*
  * Wait for auditd to drain the queue a little
  */
-static void wait_for_auditd(unsigned long sleep_time)
+static bool wait_for_auditd(gfp_t gfp_mask)
 {
-	DECLARE_WAITQUEUE(wait, current);
-	set_current_state(TASK_INTERRUPTIBLE);
-	add_wait_queue(&audit_backlog_wait, &wait);
-
-	if (audit_backlog_limit &&
-	    skb_queue_len(&audit_skb_queue) > audit_backlog_limit)
-		schedule_timeout(sleep_time);
+	bool can_wait = gfp_mask & __GFP_WAIT;
+	int reserve = can_wait ? 0 : 5;
 
-	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(&audit_backlog_wait, &wait);
+	if (!audit_backlog_limit ||
+	    skb_queue_len(&audit_skb_queue) <= audit_backlog_limit + reserve)
+		return true;
+
+	return can_wait && wait_event_timeout(audit_backlog_wait,
+				!audit_backlog_limit ||
+					skb_queue_len(&audit_skb_queue) <= audit_backlog_limit,
+				audit_backlog_wait_time);
 }
 
 /* Obtain an audit buffer.  This routine does locking to obtain the
@@ -1095,8 +1096,6 @@ struct audit_buffer *audit_log_start(str
 	struct audit_buffer	*ab	= NULL;
 	struct timespec		t;
 	unsigned int		uninitialized_var(serial);
-	int reserve;
-	unsigned long timeout_start = jiffies;
 
 	if (audit_initialized != AUDIT_INITIALIZED)
 		return NULL;
@@ -1104,23 +1103,7 @@ struct audit_buffer *audit_log_start(str
 	if (unlikely(audit_filter_type(type)))
 		return NULL;
 
-	if (gfp_mask & __GFP_WAIT)
-		reserve = 0;
-	else
-		reserve = 5; /* Allow atomic callers to go up to five
-				entries over the normal backlog limit */
-
-	while (audit_backlog_limit
-	       && skb_queue_len(&audit_skb_queue) > audit_backlog_limit + reserve) {
-		if (gfp_mask & __GFP_WAIT && audit_backlog_wait_time) {
-			unsigned long sleep_time;
-
-			sleep_time = timeout_start + audit_backlog_wait_time -
-					jiffies;
-			if ((long)sleep_time > 0)
-				wait_for_auditd(sleep_time);
-			continue;
-		}
+	if (!wait_for_auditd(gfp_mask)) {
 		if (audit_rate_check() && printk_ratelimit())
 			printk(KERN_WARNING
 			       "audit: audit_backlog=%d > "

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/