performance questions

performance questions

[LC Bruzenak]

I was looking at some strace results from a process using the
audit_log_user_message call and I think I see how I can eliminate some
ioctls and /proc/self lookups by setting the hostname/tty parameters to
non-NULL pointers pointing to NULL values.

But the exename is another story. It does a lookup each time. We have
persistent processes each of which submit 100Ks (on the way to 1Ms) of
audit_log_user_message events daily, so it would make a difference.

I was thinking about a patch to store off the exename statically if one
isn't already in the pipeline. Let me know; I'll submit something if
not.

The other question is on the auditd side. IIUC on each event the
write_to_log function is checking the logfile size. Seems to me that we
could limit the fstat checks to say one every ten events or so. Any
problems there?

Thx,
LCB 

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com

Re: performance questions

[Steve Grubb]

On Thursday, September 29, 2011 11:33:09 AM LC Bruzenak wrote:
> I was looking at some strace results from a process using the
> audit_log_user_message call and I think I see how I can eliminate some
> ioctls and /proc/self lookups by setting the hostname/tty parameters to
> non-NULL pointers pointing to NULL values.
> 
> But the exename is another story. It does a lookup each time. We have
> persistent processes each of which submit 100Ks (on the way to 1Ms) of
> audit_log_user_message events daily, so it would make a difference.
> 
> I was thinking about a patch to store off the exename statically if one
> isn't already in the pipeline. Let me know; I'll submit something if
> not.

You might try this:

diff -urp audit-2.1.4.orig/lib/audit_logging.c audit-2.1.4/lib/audit_logging.c
--- audit-2.1.4.orig/lib/audit_logging.c	2011-09-06 14:17:06.000000000 -0400
+++ audit-2.1.4/lib/audit_logging.c	2011-09-30 09:08:50.000000000 -0400
@@ -240,7 +240,7 @@ int audit_log_user_message(int audit_fd,
 {
 	char buf[MAX_AUDIT_MESSAGE_LENGTH];
 	char addrbuf[INET6_ADDRSTRLEN];
-	char exename[PATH_MAX*2];
+	static char exename[PATH_MAX*2]="";
 	char ttyname[TTY_PATH];
 	const char *success;
 	int ret;
@@ -262,7 +262,8 @@ int audit_log_user_message(int audit_fd,
 	else
 		strncat(addrbuf, addr, sizeof(addrbuf)-1);
 
-	_get_exename(exename, sizeof(exename));
+	if (exename[0] == 0)
+		_get_exename(exename, sizeof(exename));
 	if (tty == NULL) 
 		tty = _get_tty(ttyname, TTY_PATH);
 	else if (*tty == 0)


> The other question is on the auditd side. IIUC on each event the
> write_to_log function is checking the logfile size. Seems to me that we
> could limit the fstat checks to say one every ten events or so. Any
> problems there?

We can probably use the return value of fprintf() +1 (for the NULL byte) and 
just keep the running total in memory.

-Steve

Re: performance questions

[LC Bruzenak]

On Fri, 2011-09-30 at 09:20 -0400, Steve Grubb wrote:
> On Thursday, September 29, 2011 11:33:09 AM LC Bruzenak wrote:
...
> 
> You might try this:
...
>  
> -	_get_exename(exename, sizeof(exename));
> +	if (exename[0] == 0)
> +		_get_exename(exename, sizeof(exename));
>  	if (tty == NULL) 
>  		tty = _get_tty(ttyname, TTY_PATH);
>  	else if (*tty == 0)

Well, we could (and then it would work like the others) but we really
want to store the exename I think. Isn't that what becomes
"exe=<EXEPATH>" in the event?

> 
> We can probably use the return value of fprintf() +1 (for the NULL byte) and 
> just keep the running total in memory.

Oh, right. That would be more precise. Good idea!

Since we're looking, what about the fstatfs in check_disk_space? Any
thoughts on that one?

Thanks Steve!
LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com

Re: performance questions

[Steve Grubb]

On Friday, September 30, 2011 10:20:43 AM LC Bruzenak wrote:
> On Fri, 2011-09-30 at 09:20 -0400, Steve Grubb wrote:
> > On Thursday, September 29, 2011 11:33:09 AM LC Bruzenak wrote:
> ...
> 
> > You might try this:
> ...
> 
> > -	_get_exename(exename, sizeof(exename));
> > +	if (exename[0] == 0)
> > +		_get_exename(exename, sizeof(exename));
> > 
> >  	if (tty == NULL)
> >  	
> >  		tty = _get_tty(ttyname, TTY_PATH);
> >  	
> >  	else if (*tty == 0)
> 
> Well, we could (and then it would work like the others) but we really
> want to store the exename I think. Isn't that what becomes
> "exe=<EXEPATH>" in the event?

It does. You can strace it. :)

 
> > We can probably use the return value of fprintf() +1 (for the NULL byte)
> > and just keep the running total in memory.
> 
> Oh, right. That would be more precise. Good idea!
> 
> Since we're looking, what about the fstatfs in check_disk_space? Any
> thoughts on that one?

Probably can't get rid of that one. Many times people don't separate the audit 
directory to its own partition. So, we wind up sharing space with /var/log/messages 
which anyone can write to. Even if we had it exclusively, sometimes there is a cron 
job that might come and grab files for archiving in which case an internal count would 
be off.

-Steve